morrySnow commented on code in PR #61918: URL: https://github.com/apache/doris/pull/61918#discussion_r3019530522
########## fe/fe-core/src/main/java/org/apache/doris/analysis/ExprToExprNameVisitor.java: ########## @@ -0,0 +1,65 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.analysis; + +import org.apache.doris.common.NameFormatUtils; + +/** + * Visitor that computes the "expr name" for an {@link Expr} tree. + * + * <p>The expr name is a short, normalized identifier used for column labels + * and other metadata — it is <b>not</b> a full SQL representation. + * + * <ul> + * <li>{@link SlotRef} → delegates to {@link ExprToColumnLabelVisitor} (raw column name)</li> + * <li>{@link FunctionCallExpr} → normalized function name</li> + * <li>{@link ColumnRefExpr} → normalized column name</li> + * <li>{@link LiteralExpr} (all subtypes) → {@code "literal"}</li> + * <li>Everything else → normalized simple class name</li> + * </ul> + */ +public class ExprToExprNameVisitor extends ExprVisitor<String, Void> { + + public static final ExprToExprNameVisitor INSTANCE = new ExprToExprNameVisitor(); + + private ExprToExprNameVisitor() { + } + + @Override + public String visit(Expr expr, Void context) { + if (expr instanceof LiteralExpr) { + return "literal"; + } + return NameFormatUtils.normalizeName(expr.getClass().getSimpleName(), Expr.DEFAULT_EXPR_NAME); + } + + @Override + public String visitSlotRef(SlotRef expr, Void context) { + return expr.accept(ExprToColumnLabelVisitor.INSTANCE, null); Review Comment: ExprToColumnLabelVisitor only used in this class, so we could remove ExprToColumnLabelVisitor ########## fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java: ########## @@ -756,8 +757,12 @@ private void replaceTableInternal(Database db, OlapTable origTable, OlapTable ne String newTblName = newTbl.getName(); // Handle constraints for table replacement - TableNameInfo origTableInfo = new TableNameInfo(origTable); - TableNameInfo newTableInfo = new TableNameInfo(newTbl); + String catalogName = db.getCatalog() != null ? db.getCatalog().getName() + : NameSpaceContext.INTERNAL_CATALOG_NAME; + TableNameInfo origTableInfo = new TableNameInfo( + catalogName, db.getFullName(), origTable.getName()); + TableNameInfo newTableInfo = new TableNameInfo( + catalogName, db.getFullName(), newTbl.getName()); Review Comment: Abstract it into a utility function and reuse it in similar places ########## fe/fe-common/src/main/java/org/apache/doris/qe/GlobalVariable.java: ########## @@ -92,60 +91,74 @@ public final class GlobalVariable { public static final String ENABLE_NEW_TYPE_COERCION_BEHAVIOR = "enable_new_type_coercion_behavior"; - @VariableMgr.VarAttr(name = VARIABLE_VERSION, flag = VariableMgr.INVISIBLE - | VariableMgr.READ_ONLY | VariableMgr.GLOBAL) + @VarAttrDef.VarAttr(name = VARIABLE_VERSION, flag = VarAttrDef.INVISIBLE + | VarAttrDef.READ_ONLY | VarAttrDef.GLOBAL) public static int variableVersion = CURRENT_VARIABLE_VERSION; - @VariableMgr.VarAttr(name = VERSION_COMMENT, flag = VariableMgr.READ_ONLY) + @VarAttrDef.VarAttr(name = VERSION_COMMENT, flag = VarAttrDef.READ_ONLY) public static String versionComment = Version.DORIS_BUILD_VERSION_PREFIX + " version " + Version.DORIS_BUILD_VERSION + "-" + Version.DORIS_BUILD_SHORT_HASH + (Config.isCloudMode() ? " (Cloud Mode)" : ""); - @VariableMgr.VarAttr(name = VERSION) + @VarAttrDef.VarAttr(name = VERSION) public static String version = DEFAULT_SERVER_VERSION; // 0: table names are stored as specified and comparisons are case sensitive. // 1: table names are stored in lowercase on disk and comparisons are not case sensitive. // 2: table names are stored as given but compared in lowercase. - @VariableMgr.VarAttr(name = LOWER_CASE_TABLE_NAMES, flag = VariableMgr.READ_ONLY | VariableMgr.GLOBAL) + @VarAttrDef.VarAttr(name = LOWER_CASE_TABLE_NAMES, flag = VarAttrDef.READ_ONLY | VarAttrDef.GLOBAL) public static int lowerCaseTableNames = 0; - @VariableMgr.VarAttr(name = LICENSE, flag = VariableMgr.READ_ONLY) + @VarAttrDef.VarAttr(name = LICENSE, flag = VarAttrDef.READ_ONLY) public static String license = "Apache License, Version 2.0"; - @VariableMgr.VarAttr(name = LANGUAGE, flag = VariableMgr.READ_ONLY) + @VarAttrDef.VarAttr(name = LANGUAGE, flag = VarAttrDef.READ_ONLY) public static String language = "/palo/share/english/"; // A string to be executed by the server for each client that connects - @VariableMgr.VarAttr(name = INIT_CONNECT, flag = VariableMgr.GLOBAL) + @VarAttrDef.VarAttr(name = INIT_CONNECT, flag = VarAttrDef.GLOBAL) public static volatile String initConnect = ""; // A string to be executed by the server for each client that connects - @VariableMgr.VarAttr(name = SYSTEM_TIME_ZONE, flag = VariableMgr.READ_ONLY) - public static String systemTimeZone = TimeUtils.getSystemTimeZone().getID(); + @VarAttrDef.VarAttr(name = SYSTEM_TIME_ZONE, flag = VarAttrDef.READ_ONLY) + public static String systemTimeZone = resolveSystemTimeZone(); + + private static String resolveSystemTimeZone() { Review Comment: use a static map directly ########## fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java: ########## @@ -592,137 +588,6 @@ public String getOnUpdateDefaultValueSql() { return onUpdateDefaultValueExprDef.getSql(); } - public TColumn toThrift() { - TColumn tColumn = new TColumn(); - tColumn.setColumnName(removeNamePrefix(this.name)); - - TColumnType tColumnType = new TColumnType(); - tColumnType.setType(this.getDataType().toThrift()); - tColumnType.setLen(this.getStrLen()); - tColumnType.setPrecision(this.getPrecision()); - tColumnType.setScale(this.getScale()); - tColumnType.setVariantMaxSubcolumnsCount(this.getVariantMaxSubcolumnsCount()); - tColumnType.setVariantEnableDocMode(this.getVariantEnableDocMode()); - - tColumnType.setIndexLen(this.getOlapColumnIndexSize()); - - tColumn.setColumnType(tColumnType); - if (null != this.aggregationType) { - tColumn.setAggregationType(this.aggregationType.toThrift()); - } else { - tColumn.setAggregationType(TAggregationType.NONE); - } - - tColumn.setIsKey(this.isKey); - tColumn.setIsAllowNull(this.isAllowNull); - tColumn.setIsAutoIncrement(this.isAutoInc); - tColumn.setIsOnUpdateCurrentTimestamp(this.hasOnUpdateDefaultValue); - // keep compatibility - tColumn.setDefaultValue(this.realDefaultValue == null ? this.defaultValue : this.realDefaultValue); - tColumn.setVisible(visible); - toChildrenThrift(this, tColumn); - - tColumn.setColUniqueId(uniqueId); - - if (type.isAggStateType()) { - AggStateType aggState = (AggStateType) type; - tColumn.setAggregation(aggState.getFunctionName()); - tColumn.setResultIsNullable(aggState.getResultIsNullable()); - if (children != null) { - for (Column column : children) { - tColumn.addToChildrenColumn(column.toThrift()); - } - } - tColumn.setBeExecVersion(Config.be_exec_version); - } - tColumn.setClusterKeyId(this.clusterKeyId); - tColumn.setVariantEnableTypedPathsToSparse(this.getVariantEnableTypedPathsToSparse()); - tColumn.setVariantMaxSparseColumnStatisticsSize(this.getVariantMaxSparseColumnStatisticsSize()); - tColumn.setVariantSparseHashShardCount(this.getVariantSparseHashShardCount()); - tColumn.setVariantEnableDocMode(this.getVariantEnableDocMode()); - tColumn.setVariantDocMaterializationMinRows(this.getvariantDocMaterializationMinRows()); - tColumn.setVariantDocHashShardCount(this.getVariantDocShardCount()); - tColumn.setVariantEnableNestedGroup(this.getVariantEnableNestedGroup()); - // ATTN: - // Currently, this `toThrift()` method is only used from CreateReplicaTask. - // And CreateReplicaTask does not need `defineExpr` field. - // The `defineExpr` is only used when creating `TAlterMaterializedViewParam`, which is in `AlterReplicaTask`. - // And when creating `TAlterMaterializedViewParam`, the `defineExpr` is certainly analyzed. - // If we need to use `defineExpr` and call defineExpr.treeToThrift(), - // make sure it is analyzed, or NPE will thrown. - return tColumn; - } - - private void setChildrenTColumn(Column children, TColumn tColumn) { - TColumn childrenTColumn = new TColumn(); - childrenTColumn.setColumnName(children.name); - - TColumnType childrenTColumnType = new TColumnType(); - childrenTColumnType.setType(children.getDataType().toThrift()); - childrenTColumnType.setLen(children.getStrLen()); - childrenTColumnType.setPrecision(children.getPrecision()); - childrenTColumnType.setScale(children.getScale()); - childrenTColumnType.setIndexLen(children.getOlapColumnIndexSize()); - - childrenTColumn.setColumnType(childrenTColumnType); - childrenTColumn.setIsAllowNull(children.isAllowNull()); - // TODO: If we don't set the aggregate type for children, the type will be - // considered as TAggregationType::SUM after deserializing in BE. - // For now, we make children inherit the aggregate type from their parent. - if (tColumn.getAggregationType() != null) { - childrenTColumn.setAggregationType(tColumn.getAggregationType()); - } - if (children.fieldPatternType != null) { - childrenTColumn.setPatternType(children.fieldPatternType); - } - childrenTColumn.setClusterKeyId(children.clusterKeyId); - - tColumn.children_column.add(childrenTColumn); - toChildrenThrift(children, childrenTColumn); - } - - private void addChildren(Column column, TColumn tColumn) { - if (column.getChildren() != null) { - List<Column> childrenColumns = column.getChildren(); - tColumn.setChildrenColumn(new ArrayList<>()); - for (Column c : childrenColumns) { - setChildrenTColumn(c, tColumn); - } - } - } - - private void addChildren(OlapFile.ColumnPB.Builder builder) throws DdlException { - if (this.getChildren() != null) { - List<Column> childrenColumns = this.getChildren(); - for (Column c : childrenColumns) { - builder.addChildrenColumns(c.toPb(Sets.newHashSet(), Lists.newArrayList())); - } - } - } - - private void toChildrenThrift(Column column, TColumn tColumn) { - if (column.type.isArrayType()) { - Column children = column.getChildren().get(0); - tColumn.setChildrenColumn(new ArrayList<>()); - setChildrenTColumn(children, tColumn); - } else if (column.type.isMapType()) { - Column k = column.getChildren().get(0); - Column v = column.getChildren().get(1); - tColumn.setChildrenColumn(new ArrayList<>()); - setChildrenTColumn(k, tColumn); - setChildrenTColumn(v, tColumn); - } else if (column.type.isStructType()) { - List<Column> childrenColumns = column.getChildren(); - tColumn.setChildrenColumn(new ArrayList<>()); - for (Column children : childrenColumns) { - setChildrenTColumn(children, tColumn); - } - } else if (column.type.isVariantType()) { - // variant may contain predefined structured fields - addChildren(column, tColumn); - } - } - // CLOUD_CODE_BEGIN public int getFieldLengthByType(TPrimitiveType type, int stringLength) throws DdlException { Review Comment: this function should move to ColumnToProtobuf ########## fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnToProtobuf.java: ########## @@ -0,0 +1,136 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog; + +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.proto.OlapFile; +import org.apache.doris.proto.OlapFile.PatternTypePB; +import org.apache.doris.thrift.TPrimitiveType; + +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.google.protobuf.ByteString; + +import java.util.List; +import java.util.Set; + +public class ColumnToProtobuf { + public static OlapFile.ColumnPB toPb(Column column, Set<String> bfColumns, List<Index> indexes) + throws DdlException { + OlapFile.ColumnPB.Builder builder = OlapFile.ColumnPB.newBuilder(); + + // when doing schema change, some modified column has a prefix in name. + // this prefix is only used in FE, not visible to BE, so we should remove this prefix. + String name = column.getName(); + builder.setName(name.startsWith(Column.SHADOW_NAME_PREFIX) + ? name.substring(Column.SHADOW_NAME_PREFIX.length()) : name); + + builder.setUniqueId(column.getUniqueId()); + builder.setType(column.getDataType().toThrift().name()); + builder.setIsKey(column.isKey()); + if (column.getFieldPatternType() != null) { + switch (column.getFieldPatternType()) { + case MATCH_NAME: + builder.setPatternType(PatternTypePB.MATCH_NAME); + break; + case MATCH_NAME_GLOB: + builder.setPatternType(PatternTypePB.MATCH_NAME_GLOB); + break; + default: + throw new IllegalArgumentException("Unknown PatternType: " + column.getFieldPatternType()); + } + } + if (column.getAggregationType() != null) { + if (column.getType().isAggStateType()) { + AggStateType aggState = (AggStateType) column.getType(); + builder.setAggregation(aggState.getFunctionName()); + builder.setResultIsNullable(aggState.getResultIsNullable()); + builder.setBeExecVersion(Config.be_exec_version); + if (column.getChildren() != null) { + for (Column child : column.getChildren()) { + builder.addChildrenColumns(toPb(child, Sets.newHashSet(), Lists.newArrayList())); + } + } + } else { + builder.setAggregation(column.getAggregationType().toString()); + } + } else { + builder.setAggregation("NONE"); + } + builder.setIsNullable(column.isAllowNull()); + if (column.getDefaultValue() != null) { + builder.setDefaultValue(ByteString.copyFrom(column.getDefaultValue().getBytes())); + } + builder.setPrecision(column.getPrecision()); + builder.setFrac(column.getScale()); + + int length = column.getFieldLengthByType(column.getDataType().toThrift(), column.getStrLen()); + + builder.setLength(length); + builder.setIndexLength(length); + if (column.getDataType().toThrift() == TPrimitiveType.VARCHAR + || column.getDataType().toThrift() == TPrimitiveType.STRING) { Review Comment: use PrimitiveType to compare, do not call toThrift() -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
