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

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

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

@@ -610,6 +610,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.

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

@@ -178,6 +178,11 @@ protected String toTypeString(int depth) {
         }
         return String.format("array<%s>", itemType.toTypeString(depth + 1));
     }
+
+    @Override
+    public int getMaxUniqueId() {
+        return itemType.getMaxUniqueId();
+    }
 }
 
 

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

@@ -843,6 +843,11 @@ public int getUniqueId() {
         return this.uniqueId;
     }
 
+    // return max unique id of all fields
+    public int getMaxUniqueId() {
+        return Math.max(this.uniqueId, type.getMaxUniqueId());
+    }
+
     public void setIndexFlag(TColumn tColumn, List<Index> indexes, Set<ColumnId> bfColumns) {
         for (Index index : indexes) {
             if (index.getIndexType() == IndexDef.IndexType.BITMAP) {

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

@@ -226,5 +226,10 @@ protected String toTypeString(int depth) {
         return String.format("map<%s,%s>",
                 keyType.toTypeString(depth + 1), valueType.toTypeString(depth + 1));
     }
+
+    @Override
+    public int getMaxUniqueId() {
+        return Math.max(keyType.getMaxUniqueId(), valueType.getMaxUniqueId());
+    }
 }
 

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

@@ -644,7 +644,7 @@ public void rebuildFullSchema() {
         // update max column unique id
         int maxColUniqueId = getMaxColUniqueId();
         for (Column column : fullSchema) {
-            maxColUniqueId = Math.max(maxColUniqueId, column.getUniqueId());
+            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

@@ -149,6 +149,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

@@ -362,5 +362,14 @@ protected String toTypeString(int depth) {
         }
         return String.format("struct<%s>", Joiner.on(", ").join(fieldsSql));
     }
+
+    @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

@@ -1724,4 +1724,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;