morrySnow commented on code in PR #54025:
URL: https://github.com/apache/doris/pull/54025#discussion_r2250777600
##########
fe/fe-core/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java:
##########
@@ -256,6 +256,20 @@ public void
processCreateMaterializedView(CreateMaterializedViewCommand createMv
if (olapTable.existTempPartitions()) {
throw new DdlException("Can not alter table when there are
temp partitions in table");
}
+ // check no duplicate column name in full schema
+ Set<String> allColumnNames = new
TreeSet<>(String.CASE_INSENSITIVE_ORDER);
+ for (Column column : olapTable.getFullSchema()) {
+ // we don't check the duplicate name of historic mv for
backwards compatibility
+ allColumnNames.add(column.getName());
+ }
+
+ for (MVColumnItem mvColumnItem :
createMvCommand.getMVColumnItemList()) {
+ String colName = mvColumnItem.getName();
+ if (!allColumnNames.add(colName)) {
+ throw new AnalysisException(String.format("duplicate
column name %s in full schema, "
+ + "please use a new unique name xxx, like %s as
xxx in select list", colName, colName));
Review Comment:
let the error messsage same with
`org.apache.doris.common.ErrorCode#ERR_DUP_FIELDNAME`
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/MVColumnItem.java:
##########
@@ -78,22 +64,13 @@ public MVColumnItem(String name, Type type, AggregateType
aggregateType, boolean
}
}
- public MVColumnItem(String name, Type type) {
- this.name = name;
- this.type = type;
- baseColumnNames = new HashSet<>();
- baseColumnNames.add(name);
- }
-
public MVColumnItem(Expr defineExpr) throws AnalysisException {
- this.name =
CreateMaterializedViewStmt.mvColumnBuilder(defineExpr.toSqlWithoutTbl());
-
- if (this.name == null) {
- throw new AnalysisException("defineExpr.toSql() is null");
- }
-
- this.name = MaterializedIndexMeta.normalizeName(this.name);
+ this(defineExpr.toSqlWithoutTbl(), defineExpr);
+ }
+ public MVColumnItem(String name, Expr defineExpr) throws AnalysisException
{
+ this.name = MaterializedIndexMeta.normalizeName(name);
+ this.isAggregationTypeImplicit = false;
Review Comment:
always false？could we remove it?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/MVColumnItem.java:
##########
@@ -78,22 +64,13 @@ public MVColumnItem(String name, Type type, AggregateType
aggregateType, boolean
}
}
- public MVColumnItem(String name, Type type) {
- this.name = name;
- this.type = type;
- baseColumnNames = new HashSet<>();
- baseColumnNames.add(name);
- }
-
public MVColumnItem(Expr defineExpr) throws AnalysisException {
- this.name =
CreateMaterializedViewStmt.mvColumnBuilder(defineExpr.toSqlWithoutTbl());
-
- if (this.name == null) {
- throw new AnalysisException("defineExpr.toSql() is null");
- }
-
- this.name = MaterializedIndexMeta.normalizeName(this.name);
+ this(defineExpr.toSqlWithoutTbl(), defineExpr);
+ }
+ public MVColumnItem(String name, Expr defineExpr) throws AnalysisException
{
+ this.name = MaterializedIndexMeta.normalizeName(name);
Review Comment:
why still need to do normalize?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java:
##########
@@ -1195,6 +1186,17 @@ public boolean isMaterializedViewColumn() {
return defineExpr != null;
}
+ public String tryGetBaseColumnName() {
Review Comment:
base column could more than one? if defineExpr is s1 + s2, we do not return
anything? add ut for it
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/MVColumnItem.java:
##########
@@ -47,24 +45,12 @@ public class MVColumnItem {
private boolean isAggregationTypeImplicit;
private Expr defineExpr;
private Set<String> baseColumnNames;
- private String baseTableName;
- public MVColumnItem(String name, Type type, AggregateType aggregateType,
boolean isAggregationTypeImplicit,
- Expr defineExpr, String baseColumnName) {
- this(name, type, aggregateType, isAggregationTypeImplicit, defineExpr);
- }
-
- public MVColumnItem(Type type, AggregateType aggregateType, Expr
defineExpr, String baseColumnName) {
- this(CreateMaterializedViewStmt.mvColumnBuilder(aggregateType,
baseColumnName), type, aggregateType, false,
- defineExpr);
- }
-
- public MVColumnItem(String name, Type type, AggregateType aggregateType,
boolean isAggregationTypeImplicit,
- Expr defineExpr) {
- this.name = name;
+ public MVColumnItem(String name, Type type, AggregateType aggregateType,
Expr defineExpr) {
+ this.name = MaterializedIndexMeta.normalizeName(name);
Review Comment:
why do this normalize?
--
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]
