Skip to content

Commit

Permalink
[fix](schema-change) Complete check for string type length change (#4…
Browse files Browse the repository at this point in the history
…8607)

### 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.
  • Loading branch information
TangSiyang2001 authored Mar 6, 2025
1 parent a99eba0 commit 7cd9757
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ private boolean processModifyColumn(ModifyColumnClause alterClause, OlapTable ol
// 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()
Expand Down
4 changes: 4 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,10 @@ public void checkSchemaChangeAllowed(Column other) throws DdlException {
}
}

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);
Expand Down
30 changes: 21 additions & 9 deletions fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java
Original file line number Diff line number Diff line change
Expand Up @@ -169,19 +169,31 @@ static boolean isSchemaChangeAllowed(Type lhs, Type rhs) {
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
Expand Down
21 changes: 21 additions & 0 deletions regression-test/data/schema_change_p0/test_type_length_change.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !master_sql --
1 abc abc
2 abcde abc

-- !master_sql --
1 abc abc
2 abcde abc
3 abcde abcde

-- !master_sql --
1 abc abc
2 abcde abc
3 abcde abcde
4 abcde abcde

-- !master_sql --
k int Yes true \N
c0 varchar(6) Yes true \N
c1 varchar(6) Yes true \N

Original file line number Diff line number Diff line change
@@ -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} """
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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代码
Expand Down

0 comments on commit 7cd9757

Please sign in to comment.