From 7f9cecb62314bea4eaaab3b77b8037c0da1eb3f4 Mon Sep 17 00:00:00 2001 From: Lei Zhang <27994433+SWJTU-ZhangLei@users.noreply.github.com> Date: Tue, 12 Dec 2023 21:08:38 +0800 Subject: [PATCH] [fix](fe) Key column varchar length change should't light schema change (#28235) --- .../doris/alter/SchemaChangeHandler.java | 8 ++- .../doris/datasource/InternalCatalog.java | 2 +- .../test_multi_column_partition.groovy | 2 +- ...har_schema_change_with_distribution.groovy | 52 +++++++++++++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 regression-test/suites/schema_change_p0/test_varchar_schema_change_with_distribution.groovy 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 d6ab6a972e..88f6b7718c 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 @@ -588,7 +588,13 @@ public class SchemaChangeHandler extends AlterHandler { if (columnPos == null && col.getDataType() == PrimitiveType.VARCHAR && modColumn.getDataType() == PrimitiveType.VARCHAR) { col.checkSchemaChangeAllowed(modColumn); - lightSchemaChange = olapTable.getEnableLightSchemaChange(); + // If col and modColumn is not key, it allow light schema change, + // of course, olapTable has been enable light schema change + if (modColumn.isKey() || col.isKey()) { + lightSchemaChange = false; + } else { + lightSchemaChange = olapTable.getEnableLightSchemaChange(); + } } if (col.isClusterKey()) { throw new DdlException("Can not modify cluster key column: " + col.getName()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 3a080d2b28..d4943e6cb8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -1475,7 +1475,7 @@ public class InternalCatalog implements CatalogIf { if (!hashDistributionInfo.sameDistributionColumns((HashDistributionInfo) defaultDistributionInfo)) { throw new DdlException("Cannot assign hash distribution with different distribution cols. " + "new is: " + hashDistributionInfo.getDistributionColumns() + " default is: " - + ((HashDistributionInfo) distributionInfo).getDistributionColumns()); + + ((HashDistributionInfo) defaultDistributionInfo).getDistributionColumns()); } } else if (distributionInfo.getType() == DistributionInfoType.RANDOM) { RandomDistributionInfo randomDistributionInfo = (RandomDistributionInfo) distributionInfo; diff --git a/regression-test/suites/partition_p0/multi_partition/test_multi_column_partition.groovy b/regression-test/suites/partition_p0/multi_partition/test_multi_column_partition.groovy index fb4d78935c..f86c375784 100644 --- a/regression-test/suites/partition_p0/multi_partition/test_multi_column_partition.groovy +++ b/regression-test/suites/partition_p0/multi_partition/test_multi_column_partition.groovy @@ -221,7 +221,7 @@ suite("test_multi_partition_key", "p0") { test { sql "ALTER TABLE test_multi_col_test_partition_add ADD PARTITION partition_add VALUES LESS THAN ('30', '1000') " + "DISTRIBUTED BY hash(k1) BUCKETS 5" - exception "Cannot assign hash distribution with different distribution cols. new is: [`k1` TINYINT NOT NULL] default is: [`k1` TINYINT NOT NULL]" + exception "Cannot assign hash distribution with different distribution cols. new is: [`k1` TINYINT NOT NULL] default is: [`k1` TINYINT NOT NULL, `k2` SMALLINT NOT NULL, `k3` INT NOT NULL]" } sql "ALTER TABLE test_multi_col_test_partition_add ADD PARTITION partition_add VALUES LESS THAN ('30', '1000') " diff --git a/regression-test/suites/schema_change_p0/test_varchar_schema_change_with_distribution.groovy b/regression-test/suites/schema_change_p0/test_varchar_schema_change_with_distribution.groovy new file mode 100644 index 0000000000..5068f0aec4 --- /dev/null +++ b/regression-test/suites/schema_change_p0/test_varchar_schema_change_with_distribution.groovy @@ -0,0 +1,52 @@ +// 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_varchar_schema_change_with_distribution") { + def tableName = "test_varchar_schema_change_with_distribution" + sql """ DROP TABLE IF EXISTS ${tableName} FORCE;""" + + sql """ + CREATE TABLE ${tableName} + ( + dt datetime NOT NULL COMMENT 'εˆ†εŒΊζ—₯期', + citycode SMALLINT, + watts_range VARCHAR(20), + pv BIGINT SUM DEFAULT '0' + ) + AGGREGATE KEY(dt, citycode, watts_range) + PARTITION BY RANGE(dt)() + DISTRIBUTED BY HASH(dt, watts_range) BUCKETS 1 + PROPERTIES ( + "dynamic_partition.enable"="true", + "dynamic_partition.end"="3", + "dynamic_partition.buckets"="1", + "dynamic_partition.start"="-3", + "dynamic_partition.prefix"="p", + "dynamic_partition.time_unit"="HOUR", + "dynamic_partition.create_history_partition"="true", + "dynamic_partition.replication_allocation" = "tag.location.default: 1" + ); + """ + + test { + sql """ alter table ${tableName} modify column watts_range varchar(30) """ + exception "Can not modify distribution column" + } + + sql """ DROP TABLE IF EXISTS ${tableName} FORCE;""" + +} \ No newline at end of file