From 65fa0fbc4d7a5ad24cb8c25857df21a5f1dc2c3c Mon Sep 17 00:00:00 2001 From: Siyang Tang Date: Mon, 24 Mar 2025 13:51:25 +0800 Subject: [PATCH 1/5] [fix](schema-change) Nested type should only support enlarge varchar length with light schema change --- .../src/main/java/org/apache/doris/catalog/ColumnType.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java index a325361d65505b..4e9b00198f01a1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java @@ -196,7 +196,9 @@ public static void checkForTypeLengthChange(Type src, Type dst) throws DdlExcept // return true if the checkType and other are both char-type otherwise return false, // which used in checkSupportSchemaChangeForComplexType private static boolean checkSupportSchemaChangeForCharType(Type checkType, Type other) throws DdlException { - if (checkType.isStringType() && other.isStringType()) { + if (Type.VARCHAR.equals(checkType) && Type.VARCHAR.equals(other)) { + // currently nested type only support light schema change, for string types, + // only varchar can do light schema change checkForTypeLengthChange(checkType, other); return true; } else { From 692e174abb08adf72c8efb3df864e6c865816973 Mon Sep 17 00:00:00 2001 From: Siyang Tang Date: Tue, 25 Mar 2025 14:04:47 +0800 Subject: [PATCH 2/5] add test cases --- .../org/apache/doris/catalog/ColumnType.java | 7 ++- .../test_type_length_change.groovy | 5 ++ .../test_varchar_sc_in_complex.groovy | 55 +++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java index 4e9b00198f01a1..e98d0a3638d404 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java @@ -103,6 +103,7 @@ public abstract class ColumnType { schemaChangeMatrix[PrimitiveType.VARCHAR.ordinal()][PrimitiveType.DATEV2.ordinal()] = true; schemaChangeMatrix[PrimitiveType.VARCHAR.ordinal()][PrimitiveType.STRING.ordinal()] = true; schemaChangeMatrix[PrimitiveType.VARCHAR.ordinal()][PrimitiveType.JSONB.ordinal()] = true; + // could not change varchar to char cuz varchar max length is larger than char schemaChangeMatrix[PrimitiveType.STRING.ordinal()][PrimitiveType.JSONB.ordinal()] = true; @@ -196,7 +197,8 @@ public static void checkForTypeLengthChange(Type src, Type dst) throws DdlExcept // return true if the checkType and other are both char-type otherwise return false, // which used in checkSupportSchemaChangeForComplexType private static boolean checkSupportSchemaChangeForCharType(Type checkType, Type other) throws DdlException { - if (Type.VARCHAR.equals(checkType) && Type.VARCHAR.equals(other)) { + if (checkType.getPrimitiveType() == PrimitiveType.VARCHAR + && other.getPrimitiveType() == PrimitiveType.VARCHAR) { // currently nested type only support light schema change, for string types, // only varchar can do light schema change checkForTypeLengthChange(checkType, other); @@ -275,7 +277,8 @@ public static void checkSupportSchemaChangeForComplexType(Type checkType, Type o // only support char-type schema change behavior for nested complex type // if nested is false, we do not check return value. if (nested && !checkSupportSchemaChangeForCharType(checkType, other)) { - throw new DdlException("Cannot change " + checkType.toSql() + " to " + other.toSql()); + throw new DdlException( + "Cannot change " + checkType.toSql() + " to " + other.toSql() + " in nested types"); } } } diff --git a/regression-test/suites/schema_change_p0/test_type_length_change.groovy b/regression-test/suites/schema_change_p0/test_type_length_change.groovy index 6da10d30166520..bdc9bd51f95785 100644 --- a/regression-test/suites/schema_change_p0/test_type_length_change.groovy +++ b/regression-test/suites/schema_change_p0/test_type_length_change.groovy @@ -74,4 +74,9 @@ suite("test_type_length_change", "p0") { sql """ INSERT INTO ${tableName} VALUES(4, "abcde", "abcde") """ qt_master_sql """ SELECT * FROM ${tableName} ORDER BY k""" qt_master_sql """ DESC ${tableName} """ + + test { + sql """ ALTER TABLE ${tableName} MODIFY COLUMN c1 CHAR(10) """ + exception "Can not change VARCHAR to CHAR" + } } diff --git a/regression-test/suites/schema_change_p0/test_varchar_sc_in_complex.groovy b/regression-test/suites/schema_change_p0/test_varchar_sc_in_complex.groovy index 682ef2f1e20645..316b6f73d6df3f 100644 --- a/regression-test/suites/schema_change_p0/test_varchar_sc_in_complex.groovy +++ b/regression-test/suites/schema_change_p0/test_varchar_sc_in_complex.groovy @@ -477,6 +477,61 @@ suite ("test_varchar_sc_in_complex") { // """ // qt_sc_after " select * from there_level_nested_type order by c0; " + // case7. do not support enlarge char length in nested types + def tableName2 = "test_enlarge_char_length_nested" + sql """ DROP TABLE IF EXISTS ${tableName2} """ + sql """ + CREATE TABLE IF NOT EXISTS ${tableName2} + ( + k BIGINT NOT NULL, + c1 ARRAY, + c2 MAP, + c3 STRUCT, + c4 ARRAY, + c5 MAP, + c6 STRUCT + ) DISTRIBUTED BY HASH(k) BUCKETS 1 + PROPERTIES ( "replication_num" = "1", "light_schema_change" = "true" ); + """ + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c1 ARRAY """ + exception "Cannot change char(10) to char(20) in nested types" + } + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c2 MAP """ + exception "Cannot change char(10) to char(20) in nested types" + } + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c3 STRUCT """ + exception "Cannot change char(10) to char(20) in nested types" + } + + // case8. do not support convert from char to varchar and varchar to char in nested types + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c1 ARRAY """ + exception "Cannot change char(10) to varchar(20) in nested types" + } + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c2 MAP """ + exception "Cannot change char(10) to varchar(20) in nested types" + } + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c3 STRUCT """ + exception "Cannot change char(10) to varchar(20) in nested types" + } + + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c4 ARRAY """ + exception "Cannot change varchar(10) to char(20) in nested types" + } + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c5 MAP """ + exception "Cannot change varchar(10) to char(20) in nested types" + } + test { + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN c6 STRUCT """ + exception "Cannot change varchar(10) to char(20) in nested types" + } } finally { try_sql("DROP TABLE IF EXISTS ${tableName}") } From 8216a800a1dc558759a165be2037e214d74d30a4 Mon Sep 17 00:00:00 2001 From: Siyang Tang Date: Tue, 25 Mar 2025 15:44:10 +0800 Subject: [PATCH 3/5] Update ColumnType.java --- .../src/main/java/org/apache/doris/catalog/ColumnType.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java index e98d0a3638d404..70ce5982812608 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java @@ -199,7 +199,7 @@ public static void checkForTypeLengthChange(Type src, Type dst) throws DdlExcept private static boolean checkSupportSchemaChangeForCharType(Type checkType, Type other) throws DdlException { if (checkType.getPrimitiveType() == PrimitiveType.VARCHAR && other.getPrimitiveType() == PrimitiveType.VARCHAR) { - // currently nested type only support light schema change, for string types, + // currently nested types only support light schema change, for string types, // only varchar can do light schema change checkForTypeLengthChange(checkType, other); return true; From 2e4317a165c889e840235661ca6853f92d6548da Mon Sep 17 00:00:00 2001 From: Siyang Tang Date: Tue, 25 Mar 2025 15:46:19 +0800 Subject: [PATCH 4/5] Update ColumnType.java --- .../src/main/java/org/apache/doris/catalog/ColumnType.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java index 70ce5982812608..58eeab49a23888 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java @@ -199,7 +199,7 @@ public static void checkForTypeLengthChange(Type src, Type dst) throws DdlExcept private static boolean checkSupportSchemaChangeForCharType(Type checkType, Type other) throws DdlException { if (checkType.getPrimitiveType() == PrimitiveType.VARCHAR && other.getPrimitiveType() == PrimitiveType.VARCHAR) { - // currently nested types only support light schema change, for string types, + // currently nested types only support light schema change for internal fields, for string types, // only varchar can do light schema change checkForTypeLengthChange(checkType, other); return true; From 3c9986c18eab3de04363424bc4ca386ed4edb66e Mon Sep 17 00:00:00 2001 From: Siyang Tang Date: Wed, 26 Mar 2025 16:07:43 +0800 Subject: [PATCH 5/5] avoid test case conflict with 3.0 --- .../data/schema_change_p0/test_type_length_change.out | 4 ++-- .../suites/schema_change_p0/test_type_length_change.groovy | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/regression-test/data/schema_change_p0/test_type_length_change.out b/regression-test/data/schema_change_p0/test_type_length_change.out index cfdc79c47c697e..edda3be3ddaaf1 100644 --- a/regression-test/data/schema_change_p0/test_type_length_change.out +++ b/regression-test/data/schema_change_p0/test_type_length_change.out @@ -16,6 +16,6 @@ -- !master_sql -- k int Yes true \N -c0 varchar(6) Yes true \N -c1 varchar(6) Yes true \N +c0 varchar(6) Yes false \N NONE +c1 varchar(6) Yes false \N NONE diff --git a/regression-test/suites/schema_change_p0/test_type_length_change.groovy b/regression-test/suites/schema_change_p0/test_type_length_change.groovy index bdc9bd51f95785..3fa18c3fe4c249 100644 --- a/regression-test/suites/schema_change_p0/test_type_length_change.groovy +++ b/regression-test/suites/schema_change_p0/test_type_length_change.groovy @@ -24,7 +24,8 @@ suite("test_type_length_change", "p0") { k INT, c0 CHAR(5), c1 VARCHAR(5) - ) + ) DUPLICATE KEY (k) + DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES ( "replication_allocation" = "tag.location.default: 1"); """