Skip to content

Commit 795a4d0

Browse files
authored
[refactor](fe) Extract toThrift from descriptor classes into DescriptorToThriftConverter (#62312)
Problem Summary: The `toThrift()` methods on `SlotDescriptor`, `TupleDescriptor`, and `DescriptorTable` mix serialization logic with domain model classes. Following the same pattern as `FunctionToThriftConverter`, this PR extracts all three `toThrift()` methods into a new `DescriptorToThriftConverter` utility class, keeping the domain classes focused on their core responsibilities. **Changes:** - Created `DescriptorToThriftConverter` as a `final` utility class with private constructor and 3 static `toThrift` overloads - Created `DescriptorToThriftConverterTest` with 17 unit tests covering all code paths - Updated all 11 call sites across 7 files to use `DescriptorToThriftConverter.toThrift()` - Removed `toThrift()` from `SlotDescriptor`, `TupleDescriptor`, and `DescriptorTable` - Added necessary getters: `getMaterializedColumnName()`, `getThriftDescTable()`, `getTupleDescs()` - Cleaned up unused imports (`TSlotDescriptor`, `TTupleDescriptor`, `TableIf`, `Logger/LogManager`)
1 parent b74f32f commit 795a4d0

22 files changed

Lines changed: 803 additions & 300 deletions
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.doris.analysis;
19+
20+
import java.util.ArrayList;
21+
import java.util.Collections;
22+
import java.util.List;
23+
import java.util.Objects;
24+
25+
/**
26+
* Java equivalent of the Thrift {@code TColumnAccessPath} struct.
27+
* Merges {@code TDataAccessPath} and {@code TMetaAccessPath} into a single class
28+
* since both are structurally identical ({@code List<String> path}).
29+
* The {@link ColumnAccessPathType} discriminates between data and metadata access.
30+
*
31+
* <p>This class is immutable and implements {@link Comparable} so it can be used
32+
* in sorted collections such as {@code TreeSet}.
33+
*/
34+
public final class ColumnAccessPath implements Comparable<ColumnAccessPath> {
35+
private final ColumnAccessPathType type;
36+
private final List<String> path;
37+
38+
public ColumnAccessPath(ColumnAccessPathType type, List<String> path) {
39+
this.type = Objects.requireNonNull(type, "type must not be null");
40+
this.path = Collections.unmodifiableList(new ArrayList<>(
41+
Objects.requireNonNull(path, "path must not be null")));
42+
}
43+
44+
/** Creates a DATA access path. */
45+
public static ColumnAccessPath data(List<String> path) {
46+
return new ColumnAccessPath(ColumnAccessPathType.DATA, path);
47+
}
48+
49+
/** Creates a META access path. */
50+
public static ColumnAccessPath meta(List<String> path) {
51+
return new ColumnAccessPath(ColumnAccessPathType.META, path);
52+
}
53+
54+
public ColumnAccessPathType getType() {
55+
return type;
56+
}
57+
58+
public List<String> getPath() {
59+
return path;
60+
}
61+
62+
@Override
63+
public boolean equals(Object o) {
64+
if (this == o) {
65+
return true;
66+
}
67+
if (!(o instanceof ColumnAccessPath)) {
68+
return false;
69+
}
70+
ColumnAccessPath that = (ColumnAccessPath) o;
71+
return type == that.type && Objects.equals(path, that.path);
72+
}
73+
74+
@Override
75+
public int hashCode() {
76+
return Objects.hash(type, path);
77+
}
78+
79+
@Override
80+
public int compareTo(ColumnAccessPath other) {
81+
int cmp = type.compareTo(other.type);
82+
if (cmp != 0) {
83+
return cmp;
84+
}
85+
int minLen = Math.min(path.size(), other.path.size());
86+
for (int i = 0; i < minLen; i++) {
87+
cmp = path.get(i).compareTo(other.path.get(i));
88+
if (cmp != 0) {
89+
return cmp;
90+
}
91+
}
92+
return Integer.compare(path.size(), other.path.size());
93+
}
94+
95+
@Override
96+
public String toString() {
97+
return type + ":" + String.join(".", path);
98+
}
99+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.doris.analysis;
19+
20+
/**
21+
* Java equivalent of the Thrift {@code TAccessPathType} enum.
22+
* Represents whether a column access path reads data or metadata.
23+
*/
24+
public enum ColumnAccessPathType {
25+
DATA,
26+
META
27+
}

fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorTable.java

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,12 @@
2020

2121
package org.apache.doris.analysis;
2222

23-
import org.apache.doris.catalog.TableIf;
2423
import org.apache.doris.common.IdGenerator;
25-
import org.apache.doris.thrift.TDescriptorTable;
2624

2725
import com.google.common.collect.Maps;
2826

27+
import java.util.Collection;
2928
import java.util.HashMap;
30-
import java.util.Map;
3129

3230
/**
3331
* Repository for tuple (and slot) descriptors.
@@ -39,7 +37,6 @@ public class DescriptorTable {
3937
private final IdGenerator<TupleId> tupleIdGenerator = TupleId.createGenerator();
4038
private final IdGenerator<SlotId> slotIdGenerator = SlotId.createGenerator();
4139
private final HashMap<SlotId, SlotDescriptor> slotDescs = Maps.newHashMap();
42-
private TDescriptorTable thriftDescTable = null; // serialized version of this
4340

4441
public DescriptorTable() {
4542
}
@@ -61,33 +58,8 @@ public TupleDescriptor getTupleDesc(TupleId id) {
6158
return tupleDescs.get(id);
6259
}
6360

64-
public TDescriptorTable toThrift() {
65-
if (thriftDescTable != null) {
66-
return thriftDescTable;
67-
}
68-
69-
TDescriptorTable result = new TDescriptorTable();
70-
Map<Long, TableIf> referencedTbls = Maps.newHashMap();
71-
for (TupleDescriptor tupleD : tupleDescs.values()) {
72-
// inline view of a non-constant select has a non-materialized tuple descriptor
73-
// in the descriptor table just for type checking, which we need to skip
74-
result.addToTupleDescriptors(tupleD.toThrift());
75-
// an inline view of a constant select has a materialized tuple
76-
// but its table has no id
77-
if (tupleD.getTable() != null
78-
&& tupleD.getTable().getId() >= 0) {
79-
referencedTbls.put(tupleD.getTable().getId(), tupleD.getTable());
80-
}
81-
for (SlotDescriptor slotD : tupleD.getSlots()) {
82-
result.addToSlotDescriptors(slotD.toThrift());
83-
}
84-
}
85-
86-
for (TableIf tbl : referencedTbls.values()) {
87-
result.addToTableDescriptors(tbl.toThrift());
88-
}
89-
thriftDescTable = result;
90-
return result;
61+
public Collection<TupleDescriptor> getTupleDescs() {
62+
return tupleDescs.values();
9163
}
9264

9365
public String getExplainString() {
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.doris.analysis;
19+
20+
import org.apache.doris.catalog.Column;
21+
import org.apache.doris.catalog.TableIf;
22+
import org.apache.doris.thrift.TAccessPathType;
23+
import org.apache.doris.thrift.TColumnAccessPath;
24+
import org.apache.doris.thrift.TDataAccessPath;
25+
import org.apache.doris.thrift.TDescriptorTable;
26+
import org.apache.doris.thrift.TMetaAccessPath;
27+
import org.apache.doris.thrift.TSlotDescriptor;
28+
import org.apache.doris.thrift.TTupleDescriptor;
29+
30+
import com.google.common.collect.Maps;
31+
import org.apache.logging.log4j.LogManager;
32+
import org.apache.logging.log4j.Logger;
33+
34+
import java.util.ArrayList;
35+
import java.util.List;
36+
import java.util.Map;
37+
38+
/**
39+
* Converts {@link SlotDescriptor}, {@link TupleDescriptor}, and {@link DescriptorTable}
40+
* to their Thrift representations.
41+
*/
42+
public final class DescriptorToThriftConverter {
43+
private static final Logger LOG = LogManager.getLogger(DescriptorToThriftConverter.class);
44+
45+
private DescriptorToThriftConverter() {
46+
}
47+
48+
/**
49+
* Converts a {@link SlotDescriptor} to its Thrift representation.
50+
*/
51+
public static TSlotDescriptor toThrift(SlotDescriptor slotDesc) {
52+
String materializedColumnName = slotDesc.getMaterializedColumnName();
53+
Column column = slotDesc.getColumn();
54+
String colName = materializedColumnName != null ? materializedColumnName :
55+
((column != null) ? column.getNonShadowName() : "");
56+
TSlotDescriptor tSlotDescriptor = new TSlotDescriptor(slotDesc.getId().asInt(),
57+
slotDesc.getParentId().asInt(), slotDesc.getType().toThrift(), -1,
58+
0, 0, slotDesc.getIsNullable() ? 0 : -1, colName, -1,
59+
true);
60+
tSlotDescriptor.setIsAutoIncrement(slotDesc.isAutoInc());
61+
if (column != null) {
62+
if (LOG.isDebugEnabled()) {
63+
LOG.debug("column name:{}, column unique id:{}", column.getNonShadowName(), column.getUniqueId());
64+
}
65+
tSlotDescriptor.setColUniqueId(column.getUniqueId());
66+
tSlotDescriptor.setPrimitiveType(column.getDataType().toThrift());
67+
tSlotDescriptor.setIsKey(column.isKey());
68+
tSlotDescriptor.setColDefaultValue(column.getDefaultValue());
69+
}
70+
if (slotDesc.getSubColLables() != null) {
71+
tSlotDescriptor.setColumnPaths(slotDesc.getSubColLables());
72+
}
73+
if (slotDesc.getVirtualColumn() != null) {
74+
tSlotDescriptor.setVirtualColumnExpr(ExprToThriftVisitor.treeToThrift(slotDesc.getVirtualColumn()));
75+
}
76+
if (slotDesc.getAllAccessPaths() != null) {
77+
tSlotDescriptor.setAllAccessPaths(toThrift(slotDesc.getAllAccessPaths()));
78+
}
79+
if (slotDesc.getPredicateAccessPaths() != null) {
80+
tSlotDescriptor.setPredicateAccessPaths(toThrift(slotDesc.getPredicateAccessPaths()));
81+
}
82+
return tSlotDescriptor;
83+
}
84+
85+
/**
86+
* Converts a {@link ColumnAccessPath} to its Thrift representation.
87+
*/
88+
public static TColumnAccessPath toThrift(ColumnAccessPath accessPath) {
89+
TColumnAccessPath result = new TColumnAccessPath(
90+
accessPath.getType() == ColumnAccessPathType.DATA ? TAccessPathType.DATA : TAccessPathType.META);
91+
if (accessPath.getType() == ColumnAccessPathType.DATA) {
92+
result.setDataAccessPath(new TDataAccessPath(accessPath.getPath()));
93+
} else {
94+
result.setMetaAccessPath(new TMetaAccessPath(accessPath.getPath()));
95+
}
96+
return result;
97+
}
98+
99+
/**
100+
* Converts a list of {@link ColumnAccessPath} to Thrift representations.
101+
*/
102+
public static List<TColumnAccessPath> toThrift(List<ColumnAccessPath> accessPaths) {
103+
List<TColumnAccessPath> result = new ArrayList<>(accessPaths.size());
104+
for (ColumnAccessPath accessPath : accessPaths) {
105+
result.add(toThrift(accessPath));
106+
}
107+
return result;
108+
}
109+
110+
/**
111+
* Converts a {@link TupleDescriptor} to its Thrift representation.
112+
*/
113+
public static TTupleDescriptor toThrift(TupleDescriptor tupleDesc) {
114+
TTupleDescriptor tTupleDesc = new TTupleDescriptor(tupleDesc.getId().asInt(), 0, 0);
115+
if (tupleDesc.getTable() != null && tupleDesc.getTable().getId() >= 0) {
116+
tTupleDesc.setTableId((int) tupleDesc.getTable().getId());
117+
}
118+
return tTupleDesc;
119+
}
120+
121+
/**
122+
* Converts a {@link DescriptorTable} to its Thrift representation.
123+
*/
124+
public static TDescriptorTable toThrift(DescriptorTable descTable) {
125+
TDescriptorTable result = new TDescriptorTable();
126+
Map<Long, TableIf> referencedTbls = Maps.newHashMap();
127+
for (TupleDescriptor tupleD : descTable.getTupleDescs()) {
128+
result.addToTupleDescriptors(toThrift(tupleD));
129+
if (tupleD.getTable() != null && tupleD.getTable().getId() >= 0) {
130+
referencedTbls.put(tupleD.getTable().getId(), tupleD.getTable());
131+
}
132+
for (SlotDescriptor slotD : tupleD.getSlots()) {
133+
result.addToSlotDescriptors(toThrift(slotD));
134+
}
135+
}
136+
137+
for (TableIf tbl : referencedTbls.values()) {
138+
result.addToTableDescriptors(tbl.toThrift());
139+
}
140+
return result;
141+
}
142+
}

0 commit comments

Comments
 (0)