[BugFix] Fix some issue of add/drop field for struct column (backport #…

…48126) (#48446)

Signed-off-by: sevev <[email protected]>
Signed-off-by: zhangqiang <[email protected]>
Co-authored-by: zhangqiang <[email protected]>

fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeHandler.java

@@ -600,6 +600,10 @@ private void processDropField(DropFieldClause alterClause, OlapTable olapTable,
             }
             fields.add(field);
         }
+        if (fields.isEmpty()) {
+            throw new DdlException("Field[" + dropFieldName + "] is the last field of column[" + modifyColumnName +
+                                   "], can not drop any more.");
+        }
         oriFieldType.updateFields(fields);
 
         // update the modifyColumn int index schema.
@@ -1782,7 +1786,7 @@ public int getAsInt() {
                 AddFieldClause addFieldClause = (AddFieldClause) alterClause;
                 modifyFieldColumns = ImmutableSet.of(addFieldClause.getColName());
                 checkModifiedColumWithMaterializedViews(olapTable, modifyFieldColumns);
-  
+
                 int id = olapTable.incAndGetMaxColUniqueId();
                 processAddField((AddFieldClause) alterClause, olapTable, indexSchemaMap, id, newIndexes);
             } else if (alterClause instanceof DropFieldClause) {
@@ -2658,15 +2662,6 @@ public void modifyTableAddOrDrop(Database db, OlapTable olapTable,
             olapTable.setIndexes(indexes);
             olapTable.rebuildFullSchema();
 
-            // update max column unique id
-            int maxColUniqueId = olapTable.getMaxColUniqueId();
-            for (Column column : indexSchemaMap.get(olapTable.getBaseIndexId())) {
-                if (column.getUniqueId() > maxColUniqueId) {
-                    maxColUniqueId = column.getUniqueId();
-                }
-            }
-            olapTable.setMaxColUniqueId(maxColUniqueId);
-
             // If modified columns are already done, inactive related mv
             inactiveRelatedMaterializedViews(db, olapTable, modifiedColumns);
 

fe/fe-core/src/main/java/com/starrocks/catalog/ArrayType.java

@@ -170,6 +170,10 @@ public String toMysqlDataTypeString() {
     public String toMysqlColumnTypeString() {
         return toSql();
     }
+    @Override
+    public int getMaxUniqueId() {
+        return itemType.getMaxUniqueId();
+    }
 }
 
 

fe/fe-core/src/main/java/com/starrocks/catalog/Column.java

@@ -816,4 +816,9 @@ public void setIndexFlag(TColumn tColumn, List<Index> indexes, Set<String> bfCol
             tColumn.setIs_bloom_filter_column(true);
         }
     }
+
+    // return max unique id of all fields
+    public int getMaxUniqueId() {
+        return Math.max(this.uniqueId, type.getMaxUniqueId());
+    }
 }

fe/fe-core/src/main/java/com/starrocks/catalog/MapType.java

@@ -217,5 +217,10 @@ public String toMysqlDataTypeString() {
     public String toMysqlColumnTypeString() {
         return toSql();
     }
+
+    @Override
+    public int getMaxUniqueId() {
+        return Math.max(keyType.getMaxUniqueId(), valueType.getMaxUniqueId());
+    }
 }
 

fe/fe-core/src/main/java/com/starrocks/catalog/OlapTable.java

@@ -594,6 +594,12 @@ public void rebuildFullSchema() {
             }
         }
         fullSchema = newFullSchema;
+        // update max column unique id
+        int maxColUniqueId = getMaxColUniqueId();
+        for (Column column : fullSchema) {
+            maxColUniqueId = Math.max(maxColUniqueId, column.getMaxUniqueId());
+        }
+        setMaxColUniqueId(maxColUniqueId);
         LOG.debug("after rebuild full schema. table {}, schema: {}", id, fullSchema);
     }
 

fe/fe-core/src/main/java/com/starrocks/catalog/StructField.java

@@ -141,6 +141,10 @@ public boolean equals(Object other) {
     public StructField clone() {
         return new StructField(name, fieldId, type.clone(), comment);
     }
+
+    public int getMaxUniqueId() {
+        return Math.max(fieldId, type.getMaxUniqueId());
+    }
 }
 
 

fe/fe-core/src/main/java/com/starrocks/catalog/StructType.java

@@ -350,5 +350,14 @@ public String toMysqlDataTypeString() {
     public String toMysqlColumnTypeString() {
         return toSql();
     }
+
+    @Override
+    public int getMaxUniqueId() {
+        int maxUniqueId = -1;
+        for (StructField f : fields) {
+            maxUniqueId = Math.max(maxUniqueId, f.getMaxUniqueId());
+        }
+        return maxUniqueId;
+    }
 }
 

fe/fe-core/src/main/java/com/starrocks/catalog/Type.java

@@ -1786,4 +1786,13 @@ public String toMysqlDataTypeString() {
     public String toMysqlColumnTypeString() {
         return "unknown";
     }
+
+    // This function is called by Column::getMaxUniqueId()
+    // If type is a scalar type, it does not have field Id because scalar type does not have sub fields
+    // If type is struct type, it will return the max field id(default value of field id is -1)
+    // If type is array type, it will return the max field id of item type
+    // if type is map type, it will return the max unique id between key type and value type
+    public int getMaxUniqueId() {
+        return -1;
+    }
 }

test/sql/test_add_drop_field/R/test_add_drop_field

@@ -62,9 +62,9 @@ insert into tab1 values (3, row(3, row(3,3), 3));
 -- !result
 select * from tab1;
 -- result:
+3	{"v1":3,"v2":{"v3":3,"v4":3},"val1":3}
 1	{"v1":1,"v2":{"v3":1,"v4":1},"val1":null}
 2	{"v1":2,"v2":{"v3":2,"v4":2},"val1":null}
-3	{"v1":3,"v2":{"v3":3,"v4":3},"val1":3}
 -- !result
 alter table tab1 modify column c1 drop field v2.v5;
 -- result:
@@ -80,9 +80,9 @@ None
 -- !result
 select * from tab1;
 -- result:
+3	{"v2":{"v3":3,"v4":3},"val1":3}
 1	{"v2":{"v3":1,"v4":1},"val1":null}
 2	{"v2":{"v3":2,"v4":2},"val1":null}
-3	{"v2":{"v3":3,"v4":3},"val1":3}
 -- !result
 alter table tab1 modify column c1 add field v1 int AFTER v2;
 -- result:
@@ -106,8 +106,8 @@ select * from tab1;
 -- result:
 1	{"v2":{"v3":1,"v4":1},"v1":null,"val1":null}
 2	{"v2":{"v3":2,"v4":2},"v1":null,"val1":null}
-3	{"v2":{"v3":3,"v4":3},"v1":null,"val1":3}
 4	{"v2":{"v3":4,"v4":4},"v1":4,"val1":4}
+3	{"v2":{"v3":3,"v4":3},"v1":null,"val1":3}
 -- !result
 drop table tab1;
 -- result:
@@ -195,9 +195,9 @@ None
 -- !result
 select * from tab1;
 -- result:
+3	[{"v2":1,"val1":1},{"v2":2,"val1":1}]
 1	[{"v2":1,"val1":null},{"v2":2,"val1":null}]
 2	[{"v2":1,"val1":null},{"v2":2,"val1":null}]
-3	[{"v2":1,"val1":1},{"v2":2,"val1":1}]
 -- !result
 insert into tab1 values (4, [row(4,4), row(4,5)]);
 -- result:
@@ -220,10 +220,10 @@ None
 -- !result
 select * from tab1;
 -- result:
-1	[{"v2":1,"val1":null,"v1":null},{"v2":2,"val1":null,"v1":null}]
-2	[{"v2":1,"val1":null,"v1":null},{"v2":2,"val1":null,"v1":null}]
 3	[{"v2":1,"val1":1,"v1":null},{"v2":2,"val1":1,"v1":null}]
 4	[{"v2":4,"val1":4,"v1":null},{"v2":4,"val1":5,"v1":null}]
+1	[{"v2":1,"val1":null,"v1":null},{"v2":2,"val1":null,"v1":null}]
+2	[{"v2":1,"val1":null,"v1":null},{"v2":2,"val1":null,"v1":null}]
 -- !result
 insert into tab1 values (5, [row(5,5,5), row(5,6,6)]);
 -- result:
@@ -233,9 +233,9 @@ select * from tab1;
 -- result:
 1	[{"v2":1,"val1":null,"v1":null},{"v2":2,"val1":null,"v1":null}]
 2	[{"v2":1,"val1":null,"v1":null},{"v2":2,"val1":null,"v1":null}]
+5	[{"v2":5,"val1":5,"v1":5},{"v2":5,"val1":6,"v1":6}]
 3	[{"v2":1,"val1":1,"v1":null},{"v2":2,"val1":1,"v1":null}]
 4	[{"v2":4,"val1":4,"v1":null},{"v2":4,"val1":5,"v1":null}]
-5	[{"v2":5,"val1":5,"v1":5},{"v2":5,"val1":6,"v1":6}]
 -- !result
 drop table tab1;
 -- result:
@@ -284,4 +284,116 @@ drop table tab1;
 drop database test_add_drop_field_not_allowed;
 -- result:
 []
+-- !result
+-- name: test_drop_last_field
+create database test_drop_last_field;
+-- result:
+[]
+-- !result
+use test_drop_last_field;
+-- result:
+[]
+-- !result
+CREATE TABLE `tab1` (
+  `c0` int(11) NULL COMMENT "",
+  `c1` Struct<v1 int>
+) ENGINE=OLAP
+DUPLICATE KEY(`c0`)
+DISTRIBUTED BY HASH(`c0`) BUCKETS 1
+PROPERTIES (
+"compression" = "LZ4",
+"fast_schema_evolution" = "true",
+"replicated_storage" = "true",
+"replication_num" = "1"
+);
+-- result:
+[]
+-- !result
+alter table tab1 modify column c1 drop field v1;
+-- result:
+E: (1064, 'Field[v1] is the last field of column[c1], can not drop any more.')
+-- !result
+drop table tab1;
+-- result:
+[]
+-- !result
+drop database test_drop_last_field;
+-- result:
+[]
+-- !result
+-- name: test_drop_add_same_name_field
+create database test_drop_add_same_name_field;
+-- result:
+[]
+-- !result
+use test_drop_add_same_name_field;
+-- result:
+[]
+-- !result
+CREATE TABLE `t` (
+  `c1` int(11) NULL COMMENT "",
+  `c2` struct<v2_1 int(11)> NULL COMMENT ""
+) ENGINE=OLAP
+DUPLICATE KEY(`c1`)
+DISTRIBUTED BY HASH(`c1`) BUCKETS 1
+PROPERTIES (
+"compression" = "LZ4",
+"fast_schema_evolution" = "true",
+"replicated_storage" = "true",
+"replication_num" = "1"
+);
+-- result:
+[]
+-- !result
+insert into t values(1, row(1)),(2,row(2));
+-- result:
+[]
+-- !result
+alter table t modify column c2 add field v2_2 string;
+-- result:
+[]
+-- !result
+function: wait_alter_table_finish()
+-- result:
+None
+-- !result
+select * from t;
+-- result:
+1	{"v2_1":1,"v2_2":null}
+2	{"v2_1":2,"v2_2":null}
+-- !result
+insert into t values(3, row(3, 'Beijing')),(4,row(4,'Shanghai'));
+-- result:
+[]
+-- !result
+alter table t modify column c2 drop field v2_2;
+-- result:
+[]
+-- !result
+function: wait_alter_table_finish()
+-- result:
+None
+-- !result
+alter table t modify column c2 add field v2_2 date;
+-- result:
+[]
+-- !result
+function: wait_alter_table_finish()
+-- result:
+None
+-- !result
+select * from t;
+-- result:
+1	{"v2_1":1,"v2_2":null}
+2	{"v2_1":2,"v2_2":null}
+3	{"v2_1":3,"v2_2":null}
+4	{"v2_1":4,"v2_2":null}
+-- !result
+drop table t;
+-- result:
+[]
+-- !result
+drop database test_drop_add_same_name_field;
+-- result:
+[]
 -- !result

test/sql/test_add_drop_field/T/test_add_drop_field

@@ -113,4 +113,58 @@ alter table tab1 modify column c1 add field [*].val1 int;
 alter table tab1 modify column c1 drop field [*].v1;
 
 drop table tab1;
-drop database test_add_drop_field_not_allowed;
+drop database test_add_drop_field_not_allowed;
+
+
+-- name: test_drop_last_field
+create database test_drop_last_field;
+use test_drop_last_field;
+
+CREATE TABLE `tab1` (
+  `c0` int(11) NULL COMMENT "",
+  `c1` Struct<v1 int>
+) ENGINE=OLAP
+DUPLICATE KEY(`c0`)
+DISTRIBUTED BY HASH(`c0`) BUCKETS 1
+PROPERTIES (
+"compression" = "LZ4",
+"fast_schema_evolution" = "true",
+"replicated_storage" = "true",
+"replication_num" = "1"
+);
+
+alter table tab1 modify column c1 drop field v1;
+
+drop table tab1;
+drop database test_drop_last_field;
+
+-- name: test_drop_add_same_name_field
+create database test_drop_add_same_name_field;
+use test_drop_add_same_name_field;
+
+CREATE TABLE `t` (
+  `c1` int(11) NULL COMMENT "",
+  `c2` struct<v2_1 int(11)> NULL COMMENT ""
+) ENGINE=OLAP
+DUPLICATE KEY(`c1`)
+DISTRIBUTED BY HASH(`c1`) BUCKETS 1
+PROPERTIES (
+"compression" = "LZ4",
+"fast_schema_evolution" = "true",
+"replicated_storage" = "true",
+"replication_num" = "1"
+);
+
+insert into t values(1, row(1)),(2,row(2));
+alter table t modify column c2 add field v2_2 string;
+function: wait_alter_table_finish()
+select * from t;
+insert into t values(3, row(3, 'Beijing')),(4,row(4,'Shanghai'));
+alter table t modify column c2 drop field v2_2;
+function: wait_alter_table_finish()
+alter table t modify column c2 add field v2_2 date;
+function: wait_alter_table_finish()
+select * from t;
+
+drop table t;
+drop database test_drop_add_same_name_field;