fix case

apache · Dec 5, 2024 · ab7cdb7 · ab7cdb7
1 parent b5aea67
commit ab7cdb7
Show file tree

Hide file tree

Showing 14 changed files with 121 additions and 153 deletions.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
@@ -578,17 +578,9 @@ private boolean processModifyColumn(ModifyColumnClause alterClause, OlapTable ol
             }
             if (!modColumn.isKey()) {
                 if (olapTable.getEnableUniqueKeyMergeOnWrite()) {
-                    if (modColumn.getAggregationType() != AggregateType.NONE) {
-                        throw new DdlException(
-                                "Can not assign aggregation method" + " on column in Unique data model table: "
-                                        + modColumn.getName());
-                    }
+                    modColumn.setAggregationType(AggregateType.NONE, true);
                 } else {
-                    if (modColumn.getAggregationType() != AggregateType.REPLACE) {
-                        throw new DdlException(
-                                "Can not assign aggregation method" + " on column in Unique data model table: "
-                                        + modColumn.getName());
-                    }
+                    modColumn.setAggregationType(AggregateType.REPLACE, true);
                 }
             }
         } else {
@@ -598,11 +590,7 @@ private boolean processModifyColumn(ModifyColumnClause alterClause, OlapTable ol
                                 + modColumn.getName());
             }
             if (!modColumn.isKey()) {
-                if (modColumn.getAggregationType() != AggregateType.NONE) {
-                    throw new DdlException(
-                            "Can not assign aggregation method" + " on column in duplicate data model table: "
-                                    + modColumn.getName());
-                }
+                modColumn.setAggregationType(AggregateType.NONE, true);
             }
         }
         ColumnPosition columnPos = alterClause.getColPos();
@@ -967,8 +955,8 @@ private boolean addColumnInternal(OlapTable olapTable, Column newColumn, ColumnP
         if (KeysType.AGG_KEYS == olapTable.getKeysType()) {
             if (newColumn.isKey() && newColumn.getAggregationType() != null) {
                 throw new DdlException("Can not assign aggregation method on key column: " + newColName);
-            } else if (null == newColumn.getAggregationType() && !newColumn.isKey()) {
-                throw new DdlException("column having no aggregation method must set as key column: " + newColName);
+            } else if (null == newColumn.getAggregationType()) {
+                newColumn.setIsKey(true);
             } else if (newColumn.getAggregationType() == AggregateType.SUM && newColumn.getDefaultValue() != null
                     && !newColumn.getDefaultValue().equals("0")) {
                 throw new DdlException(
@@ -984,40 +972,27 @@ private boolean addColumnInternal(OlapTable olapTable, Column newColumn, ColumnP
         } else if (KeysType.UNIQUE_KEYS == olapTable.getKeysType()) {
             if (newColumn.getAggregationType() != null && newColumn.isKey()) {
                 throw new DdlException(
-                        "Can not assign aggregation method" + " on key column in Unique data model table: "
-                                + newColName);
+                        "Can not assign aggregation method" + " on column in Unique data model table: " + newColName);
             }
             if (!newColumn.isKey()) {
                 if (olapTable.getEnableUniqueKeyMergeOnWrite()) {
-                    if (newColumn.getAggregationType() != AggregateType.NONE) {
-                        throw new DdlException(
-                                "Can not assign aggregation method" + " on column in Unique data model table: "
-                                        + newColumn.getName());
-                    }
+                    newColumn.setAggregationType(AggregateType.NONE, false);
                 } else {
-                    if (newColumn.getAggregationType() != AggregateType.REPLACE) {
-                        throw new DdlException(
-                                "Can not assign aggregation method" + " on column in Unique data model table: "
-                                        + newColumn.getName());
-                    }
+                    newColumn.setAggregationType(AggregateType.REPLACE, true);
                 }
             }
         } else {
             if (newColumn.getAggregationType() != null && newColumn.isKey()) {
                 throw new DdlException(
-                        "Can not assign aggregation method" + " on key column in Duplicate data model table: "
+                        "Can not assign aggregation method" + " on column in Duplicate data model table: "
                                 + newColName);
             }
             if (!newColumn.isKey()) {
                 if (targetIndexId != -1L
                         && olapTable.getIndexMetaByIndexId(targetIndexId).getKeysType() == KeysType.AGG_KEYS) {
                     throw new DdlException("Please add non-key column on base table directly");
                 }
-                if (newColumn.getAggregationType() != AggregateType.NONE) {
-                    throw new DdlException(
-                            "Can not assign aggregation method" + " on column in duplicate data model table: "
-                                    + newColumn.getName());
-                }
+                newColumn.setAggregationType(AggregateType.NONE, true);
             } else if (olapTable.isDuplicateWithoutKey()) {
                 throw new DdlException("Duplicate table without keys do not support add key column!");
             }

diff --git a/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java b/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java
@@ -189,6 +189,7 @@ private void rewriteAlterOpForOlapTable(ConnectContext ctx, OlapTable table) thr
                         } else {
                             throw new AnalysisException("unknown feature : " + alterFeature);
                         }
+                        addColumnOp.setTableName(tbl);
                         addColumnOp.validate(ctx);
                         alterTableOps.add(addColumnOp);
                     }
@@ -209,6 +210,7 @@ private void rewriteAlterOpForOlapTable(ConnectContext ctx, OlapTable table) thr
                         String lastCol = fullSchema.get(fullSchema.size() - 1).getName();
                         addColumnOp = new AddColumnOp(skipBItmapCol, new ColumnPosition(lastCol), null, null);
                     }
+                    addColumnOp.setTableName(tbl);
                     addColumnOp.validate(ctx);
                     alterTableOps.add(addColumnOp);
                 }

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java
@@ -25,6 +25,7 @@
 import org.apache.doris.catalog.Column;
 import org.apache.doris.catalog.Env;
 import org.apache.doris.catalog.KeysType;
+import org.apache.doris.catalog.MaterializedIndexMeta;
 import org.apache.doris.catalog.OlapTable;
 import org.apache.doris.catalog.Table;
 import org.apache.doris.common.AnalysisException;
@@ -33,7 +34,6 @@
 import org.apache.doris.common.UserException;
 import org.apache.doris.qe.ConnectContext;
 
-import com.google.common.base.Strings;
 import com.google.common.collect.Sets;
 
 import java.util.Map;
@@ -77,13 +77,10 @@ public String getRollupName() {
 
     @Override
     public void validate(ConnectContext ctx) throws UserException {
-        if (!Strings.isNullOrEmpty(rollupName)) {
-            throw new AnalysisException("Cannot add column in rollup " + rollupName);
-        }
         if (colPos != null) {
             colPos.analyze();
         }
-        validateColumnDef(tableName, columnDef, colPos);
+        validateColumnDef(tableName, columnDef, colPos, rollupName);
         column = columnDef.translateToCatalogStyle();
     }
 
@@ -128,7 +125,8 @@ public String toString() {
     /**
      * validateColumnDef
      */
-    public static void validateColumnDef(TableNameInfo tableName, ColumnDefinition columnDef, ColumnPosition colPos)
+    public static void validateColumnDef(TableNameInfo tableName, ColumnDefinition columnDef, ColumnPosition colPos,
+            String rollupName)
             throws UserException {
         if (columnDef == null) {
             throw new AnalysisException("No column definition in add column clause.");
@@ -146,6 +144,13 @@ public static void validateColumnDef(TableNameInfo tableName, ColumnDefinition c
             olapTable = (OlapTable) table;
             keysType = olapTable.getKeysType();
             AggregateType aggregateType = columnDef.getAggType();
+            Long indexId = olapTable.getIndexIdByName(rollupName);
+            if (indexId != null) {
+                MaterializedIndexMeta indexMeta = olapTable.getIndexMetaByIndexId(indexId);
+                if (indexMeta.getDefineStmt() != null) {
+                    throw new AnalysisException("Cannot add column in rollup " + rollupName);
+                }
+            }
             if (keysType == KeysType.AGG_KEYS) {
                 if (aggregateType == null) {
                     columnDef.setIsKey(true);
@@ -157,7 +162,7 @@ public static void validateColumnDef(TableNameInfo tableName, ColumnDefinition c
                     }
                 }
             } else if (keysType == KeysType.UNIQUE_KEYS) {
-                if (aggregateType != null) {
+                if (aggregateType != null && columnDef.isVisible()) {
                     throw new AnalysisException(
                             String.format("Can not assign aggregation method on column in Unique data model table: %s",
                                     columnDef.getName()));

diff --git a/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnsOp.java b/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnsOp.java
@@ -28,7 +28,6 @@
 import org.apache.doris.common.UserException;
 import org.apache.doris.qe.ConnectContext;
 
-import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 
 import java.util.List;
@@ -65,11 +64,8 @@ public void validate(ConnectContext ctx) throws UserException {
         if (columnDefs == null || columnDefs.isEmpty()) {
             throw new AnalysisException("Columns is empty in add columns clause.");
         }
-        if (!Strings.isNullOrEmpty(rollupName)) {
-            throw new AnalysisException("Cannot add column in rollup " + rollupName);
-        }
         for (ColumnDefinition colDef : columnDefs) {
-            AddColumnOp.validateColumnDef(tableName, colDef, null);
+            AddColumnOp.validateColumnDef(tableName, colDef, null, rollupName);
         }
 
         Table table = Env.getCurrentInternalCatalog().getDbOrDdlException(tableName.getDb())

diff --git a/...re/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java b/...re/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java
@@ -180,8 +180,8 @@ public boolean hasDefaultValue() {
         return defaultValue.isPresent();
     }
 
-    public void setAggregationTypeImplicit(boolean aggTypeImplicit) {
-        this.aggTypeImplicit = aggTypeImplicit;
+    public boolean isVisible() {
+        return isVisible;
     }
 
     /**

diff --git a/...core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnOp.java b/...core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnOp.java
@@ -188,13 +188,7 @@ public void validate(ConnectContext ctx) throws UserException {
         }
         column = columnDef.translateToCatalogStyle();
         if (originalColumn != null) {
-            String errorMsg = originalColumn.verifyCanChangeTo(column);
-            if (errorMsg != null) {
-                throw new AnalysisException(errorMsg);
-            }
-            if (originalColumn.isAllowNull() && !column.isAllowNull()) {
-                throw new AnalysisException("Can not modify column " + colName + " from NULL to NOT NULL");
-            }
+            originalColumn.checkSchemaChangeAllowed(column);
         }
     }
 

diff --git a/...re/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ReorderColumnsOp.java b/...re/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ReorderColumnsOp.java
@@ -58,9 +58,6 @@ public String getRollupName() {
 
     @Override
     public void validate(ConnectContext ctx) throws UserException {
-        if (!Strings.isNullOrEmpty(rollupName)) {
-            throw new AnalysisException("Cannot reorder columns in rollup " + rollupName);
-        }
         if (columnsByPos == null || columnsByPos.isEmpty()) {
             throw new AnalysisException("No column in reorder columns clause.");
         }
@@ -81,8 +78,8 @@ public void validate(ConnectContext ctx) throws UserException {
                     }
                     seeValueColumn = seeValueColumn || !column.isKey();
                 } else {
-                    throw new AnalysisException(
-                            String.format("no column %s exists in table %s", col, tableName.getTbl()));
+                    // throw new AnalysisException(
+                    //         String.format("no column %s exists in table %s", col, tableName.getTbl()));
                 }
             }
         }

diff --git a/...est/suites/ddl_p0/test_create_table_generated_column/test_generated_column_nereids.groovy b/...est/suites/ddl_p0/test_create_table_generated_column/test_generated_column_nereids.groovy
@@ -176,8 +176,8 @@ suite("test_generated_column") {
     """
     sql "insert into gencol_refer_gencol(a,b) values(3,4)"
     test {
-        sql "alter table gencol_refer_gencol drop column a"
-        exception "Column 'a' has a generated column dependency on :[c]"
+        sql "alter table gencol_refer_gencol drop column b"
+        exception "Column 'b' has a generated column dependency on :[c]"
     }
     test {
         sql "alter table gencol_refer_gencol drop column c"

diff --git a/regression-test/suites/partition_p0/auto_partition/test_col_data_type_boundary.groovy b/regression-test/suites/partition_p0/auto_partition/test_col_data_type_boundary.groovy
@@ -71,7 +71,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -104,7 +104,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -137,7 +137,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -170,7 +170,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -244,7 +244,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -272,7 +272,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -301,7 +301,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -330,7 +330,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -358,7 +358,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 
@@ -386,7 +386,7 @@ suite("test_col_data_type_boundary") {
         if (isClusterKeyEnabled()) {
             assertTrue(e.getMessage().contains("Can not modify "))
         } else {
-            assertTrue(e.getMessage().contains("Can not modify partition column"))
+            assertTrue(e.getMessage().contains("Can not modify partition or distribution column"))
         }
     }
 

diff --git a/.../suites/partition_p0/dynamic_partition/test_dynamic_partition_mod_distribution_key.groovy b/.../suites/partition_p0/dynamic_partition/test_dynamic_partition_mod_distribution_key.groovy
@@ -49,7 +49,6 @@ suite("test_dynamic_partition_mod_distribution_key") {
         """
 
         sql """ alter table ${tableName} modify column k1 comment 'new_comment_for_k1' """
-        sql """ alter table ${tableName} modify column k2 varchar(255) """
 
         sql """ ADMIN SET FRONTEND CONFIG ('dynamic_partition_check_interval_seconds' = '1') """
         sql """ alter table ${tableName} set('dynamic_partition.end'='5') """

diff --git a/regression-test/suites/partition_p0/multi_partition/test_multi_column_partition.groovy b/regression-test/suites/partition_p0/multi_partition/test_multi_column_partition.groovy
@@ -321,15 +321,15 @@ suite("test_multi_partition_key", "p0") {
             false
     )
     test {
-        sql "ALTER TABLE test_multi_column_drop_partition_column DROP COLUMN k1"
-        exception "Can not drop key column when table has value column with REPLACE aggregation method"
+        sql "ALTER TABLE test_multi_column_drop_partition_column DROP COLUMN k4"
+        exception "Can not drop key column k4 in aggregation key table having REPLACE or REPLACE_IF_NOT_NULL column"
     }
     // drop replace value
     sql "ALTER TABLE test_multi_column_drop_partition_column DROP COLUMN v1, DROP COLUMN v2, DROP COLUMN v3"
     assertEquals("FINISHED", getAlterColumnFinalState("test_multi_column_drop_partition_column"))
     test {
         sql "ALTER TABLE test_multi_column_drop_partition_column DROP COLUMN k1"
-        exception "Partition column[k1] cannot be dropped"
+        exception "Can not drop partition or distribution column : k1"
     }
     sql "insert into test_multi_column_drop_partition_column " +
             "values(100, 0, 0, 0, 0, '2000-01-01 00:00:00', 0.001, -0.001, 0.001)"