This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 7cd97571424 [fix](schema-change) Complete check for string type length
change (#48607)
7cd97571424 is described below
commit 7cd97571424cca3a0d9d7a95c7a40c2a67bcaa68
Author: Siyang Tang <[email protected]>
AuthorDate: Thu Mar 6 21:51:34 2025 +0800
[fix](schema-change) Complete check for string type length change (#48607)
### What problem does this PR solve?
Related PR: introduce #46639
Problem Summary:
After the change of #46639, shorten length for CHAR type escapes from
checking of schema change. Fix in this PR and add some regression test
cases to verify it.
---
.../apache/doris/alter/SchemaChangeHandler.java | 2 +-
.../main/java/org/apache/doris/catalog/Column.java | 4 ++
.../java/org/apache/doris/catalog/ColumnType.java | 30 +++++---
.../schema_change_p0/test_type_length_change.out | Bin 0 -> 352 bytes
.../test_type_length_change.groovy | 77 +++++++++++++++++++++
.../test_varchar_sc_in_complex.groovy | 6 +-
.../test_varchar_schema_change.groovy | 2 +-
7 files changed, 107 insertions(+), 14 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
index 1831fa27235..0d43b1a6afd 100644
--- 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
@@ -630,7 +630,7 @@ public class SchemaChangeHandler extends AlterHandler {
// TODO:the case where columnPos is not empty has not been
considered
if (columnPos == null && col.getDataType() ==
PrimitiveType.VARCHAR
&& modColumn.getDataType() ==
PrimitiveType.VARCHAR) {
-
ColumnType.checkSupportSchemaChangeForCharType(col.getType(),
modColumn.getType());
+ ColumnType.checkForTypeLengthChange(col.getType(),
modColumn.getType());
lightSchemaChange =
olapTable.getEnableLightSchemaChange();
}
if (columnPos == null && col.getDataType().isComplexType()
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
index f7d4065b00c..be1716b1c7d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
@@ -883,6 +883,10 @@ public class Column implements GsonPostProcessable {
}
}
+ if (type.isStringType() && other.type.isStringType()) {
+ ColumnType.checkForTypeLengthChange(type, other.type);
+ }
+
// Nested types only support changing the order and increasing the
length of the nested char type
// Char-type only support length growing
ColumnType.checkSupportSchemaChangeForComplexType(type, other.type,
false);
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 234c0d4eef0..302e53ec457 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
@@ -169,19 +169,31 @@ public abstract class ColumnType {
return
schemaChangeMatrix[lhs.getPrimitiveType().ordinal()][rhs.getPrimitiveType().ordinal()];
}
+ /**
+ * Used for checking type length changing.
+ * Currently, type length is only meaningful for string types{@link
Type#isStringType()},
+ * see {@link ScalarType#len}.
+ */
+ public static void checkForTypeLengthChange(Type src, Type dst) throws
DdlException {
+ final int srcTypeLen = src.getLength();
+ final int dstTypeLen = dst.getLength();
+ if (srcTypeLen < 0 || dstTypeLen < 0) {
+ // type length is negative means that it is meaningless, just
return
+ return;
+ }
+ if (srcTypeLen > dstTypeLen) {
+ throw new DdlException(
+ String.format("Shorten type length is prohibited, srcType=%s,
dstType=%s", src.toSql(), dst.toSql()));
+ }
+ }
+
// This method defines the char type
// to support the schema-change behavior of length growth.
// return true if the checkType and other are both char-type otherwise
return false,
// which used in checkSupportSchemaChangeForComplexType
- public static boolean checkSupportSchemaChangeForCharType(Type checkType,
Type other) throws DdlException {
- if ((checkType.getPrimitiveType() == PrimitiveType.VARCHAR &&
other.getPrimitiveType() == PrimitiveType.VARCHAR)
- || (checkType.getPrimitiveType() == PrimitiveType.CHAR
- && other.getPrimitiveType() == PrimitiveType.VARCHAR)
- || (checkType.getPrimitiveType() == PrimitiveType.CHAR
- && other.getPrimitiveType() == PrimitiveType.CHAR)) {
- if (checkType.getLength() > other.getLength()) {
- throw new DdlException("Cannot shorten string length");
- }
+ private static boolean checkSupportSchemaChangeForCharType(Type checkType,
Type other) throws DdlException {
+ if (checkType.isStringType() && other.isStringType()) {
+ checkForTypeLengthChange(checkType, other);
return true;
} else {
// types equal can return true
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
new file mode 100644
index 00000000000..cfdc79c47c6
Binary files /dev/null and
b/regression-test/data/schema_change_p0/test_type_length_change.out differ
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
new file mode 100644
index 00000000000..6da10d30166
--- /dev/null
+++ b/regression-test/suites/schema_change_p0/test_type_length_change.groovy
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_type_length_change", "p0") {
+ def tableName = "test_type_length_change";
+ sql """ DROP TABLE IF EXISTS ${tableName} """
+ sql """
+ CREATE TABLE ${tableName}
+ (
+ k INT,
+ c0 CHAR(5),
+ c1 VARCHAR(5)
+ )
+ PROPERTIES ( "replication_allocation" = "tag.location.default: 1");
+ """
+
+ def getTableStatusSql = " SHOW ALTER TABLE COLUMN WHERE
IndexName='${tableName}' ORDER BY createtime DESC LIMIT 1 "
+
+ sql """ INSERT INTO ${tableName} VALUES(1, "abc", "abc") """
+
+ test {
+ sql """ ALTER TABLE ${tableName} MODIFY COLUMN c0 CHAR(3) """
+ exception "Shorten type length is prohibited"
+ }
+
+ sql """ ALTER TABLE ${tableName} MODIFY COLUMN c0 CHAR(6) """
+ waitForSchemaChangeDone {
+ sql getTableStatusSql
+ time 600
+ }
+
+ sql """ INSERT INTO ${tableName} VALUES(2, "abcde", "abc") """
+ qt_master_sql """ SELECT * FROM ${tableName} ORDER BY k"""
+
+ test {
+ sql """ ALTER TABLE ${tableName} MODIFY COLUMN c1 VARCHAR(3) """
+ exception "Shorten type length is prohibited"
+ }
+
+ sql """ ALTER TABLE ${tableName} MODIFY COLUMN c1 VARCHAR(6) """
+ waitForSchemaChangeDone {
+ sql getTableStatusSql
+ time 600
+ }
+
+ sql """ INSERT INTO ${tableName} VALUES(3, "abcde", "abcde") """
+ qt_master_sql """ SELECT * FROM ${tableName} ORDER BY k"""
+
+ test {
+ sql """ ALTER TABLE ${tableName} MODIFY COLUMN c0 VARCHAR(3) """
+ exception "Shorten type length is prohibited"
+ }
+
+ sql """ ALTER TABLE ${tableName} MODIFY COLUMN c0 VARCHAR(6) """
+ waitForSchemaChangeDone {
+ sql getTableStatusSql
+ time 600
+ }
+
+ sql """ INSERT INTO ${tableName} VALUES(4, "abcde", "abcde") """
+ qt_master_sql """ SELECT * FROM ${tableName} ORDER BY k"""
+ qt_master_sql """ DESC ${tableName} """
+}
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 d3d2554025f..172913bd8aa 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
@@ -62,19 +62,19 @@ suite ("test_varchar_sc_in_complex") {
test {
sql """ alter table ${tableName} modify column c_a
array<varchar(3)>
"""
- exception "Cannot shorten string length"
+ exception "Shorten type length is prohibited"
}
test {
sql """ alter table ${tableName} modify column c_m
map<varchar(3),varchar(3)>
"""
- exception "Cannot shorten string length"
+ exception "Shorten type length is prohibited"
}
test {
sql """ alter table ${tableName} modify column c_s
struct<col:varchar(3)>
"""
- exception "Cannot shorten string length"
+ exception "Shorten type length is prohibited"
}
// add case alter modify array/map/struct to other type
diff --git
a/regression-test/suites/schema_change_p0/test_varchar_schema_change.groovy
b/regression-test/suites/schema_change_p0/test_varchar_schema_change.groovy
index 7b5d7897114..9387cdaed06 100644
--- a/regression-test/suites/schema_change_p0/test_varchar_schema_change.groovy
+++ b/regression-test/suites/schema_change_p0/test_varchar_schema_change.groovy
@@ -55,7 +55,7 @@ suite ("test_varchar_schema_change") {
test {
sql """ alter table ${tableName} modify column c2 varchar(10)
"""
- exception "Cannot shorten string length"
+ exception "Shorten type length is prohibited"
}
// test {//为什么第一次改没发生Nothing is changed错误？查看branch-1.2-lts代码
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
