From fcec33fed859bdc7d384c9f45954aa3a18213339 Mon Sep 17 00:00:00 2001 From: yujun Date: Wed, 6 Mar 2024 00:16:20 +0800 Subject: [PATCH] [improvement](alter table) cann't rewrite force property (#31820) --- .../java/org/apache/doris/alter/Alter.java | 24 +++++++++ .../doris/alter/SchemaChangeHandler.java | 1 + .../catalog/InternalSchemaInitializer.java | 5 +- .../common/util/CloudPropertyAnalyzer.java | 2 + .../doris/common/util/PropertyAnalyzer.java | 8 +++ .../alter_p0/test_alter_force_property.groovy | 49 +++++++++++++++++++ 6 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 regression-test/suites/alter_p0/test_alter_force_property.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java index 28c9afb72222ec..8c8d0e105b40d0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java @@ -62,6 +62,7 @@ import org.apache.doris.common.util.DynamicPartitionUtil; import org.apache.doris.common.util.MetaLockUtils; import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.common.util.PropertyAnalyzer.RewriteProperty; import org.apache.doris.nereids.trees.plans.commands.info.TableNameInfo; import org.apache.doris.persist.AlterMTMV; import org.apache.doris.persist.AlterViewInfo; @@ -141,6 +142,19 @@ private boolean processAlterOlapTable(AlterTableStmt stmt, OlapTable olapTable, AlterOperations currentAlterOps = new AlterOperations(); currentAlterOps.checkConflict(alterClauses); + for (AlterClause clause : alterClauses) { + Map properties = null; + try { + properties = clause.getProperties(); + } catch (Exception e) { + continue; + } + + if (properties != null && !properties.isEmpty()) { + checkNoForceProperty(properties); + } + } + if (olapTable instanceof MTMV) { currentAlterOps.checkMTMVAllow(alterClauses); } @@ -726,6 +740,7 @@ public void modifyPartitionsProperty(Database db, Map properties, boolean isTempPartition) throws DdlException, AnalysisException { + checkNoForceProperty(properties); Preconditions.checkArgument(olapTable.isWriteLockHeldByCurrentThread()); List modifyPartitionInfos = Lists.newArrayList(); olapTable.checkNormalStateForAlter(); @@ -806,6 +821,15 @@ public void modifyPartitionsProperty(Database db, Env.getCurrentEnv().getEditLog().logBatchModifyPartition(info); } + public void checkNoForceProperty(Map properties) throws DdlException { + for (RewriteProperty property : PropertyAnalyzer.getInstance().getForceProperties()) { + if (properties.containsKey(property.key())) { + throw new DdlException("Cann't modify property '" + property.key() + "'" + + (Config.isCloudMode() ? " in cloud mode" : "") + "."); + } + } + } + public void replayModifyPartition(ModifyPartitionInfo info) throws MetaNotFoundException { Database db = Env.getCurrentInternalCatalog().getDbOrMetaException(info.getDbId()); OlapTable olapTable = (OlapTable) db.getTableOrMetaException(info.getTableId(), TableType.OLAP); 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 bab1a0c32b844a..eb116b765e8d72 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 @@ -2172,6 +2172,7 @@ static long storagePolicyNameToId(String storagePolicy) throws DdlException { */ public void updateTableProperties(Database db, String tableName, Map properties) throws UserException { + Env.getCurrentEnv().getAlterInstance().checkNoForceProperty(properties); List partitions = Lists.newArrayList(); OlapTable olapTable = (OlapTable) db.getTableOrMetaException(tableName, Table.TableType.OLAP); boolean enableUniqueKeyMergeOnWrite = false; diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java index 6ae761f2015b19..6aaaf4004fb791 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/InternalSchemaInitializer.java @@ -92,8 +92,9 @@ public void run() { @VisibleForTesting public static void modifyTblReplicaCount(Database database, String tblName) { - if (!(Config.min_replication_num_per_tablet < StatisticConstants.STATISTIC_INTERNAL_TABLE_REPLICA_NUM - && Config.max_replication_num_per_tablet >= StatisticConstants.STATISTIC_INTERNAL_TABLE_REPLICA_NUM)) { + if (Config.isCloudMode() + || Config.min_replication_num_per_tablet >= StatisticConstants.STATISTIC_INTERNAL_TABLE_REPLICA_NUM + || Config.max_replication_num_per_tablet < StatisticConstants.STATISTIC_INTERNAL_TABLE_REPLICA_NUM) { return; } while (true) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/cloud/common/util/CloudPropertyAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/cloud/common/util/CloudPropertyAnalyzer.java index 93cd5e32bb645f..34ecf33cf32766 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/cloud/common/util/CloudPropertyAnalyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/cloud/common/util/CloudPropertyAnalyzer.java @@ -40,6 +40,8 @@ public CloudPropertyAnalyzer() { String.valueOf(ReplicaAllocation.DEFAULT_ALLOCATION.getTotalReplicaNum())), RewriteProperty.replace(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION, ReplicaAllocation.DEFAULT_ALLOCATION.toCreateStmt()), + RewriteProperty.delete("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_NUM), + RewriteProperty.delete("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION), RewriteProperty.delete(DynamicPartitionProperty.STORAGE_MEDIUM), RewriteProperty.replace(DynamicPartitionProperty.REPLICATION_NUM, String.valueOf(ReplicaAllocation.DEFAULT_ALLOCATION.getTotalReplicaNum())), diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java index 5c666ec6e2ff54..1adea8d1b1eb2e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java @@ -214,6 +214,10 @@ private RewriteProperty(RewriteType rewriteType, String key, String value) { this.value = value; } + public String key() { + return this.key; + } + public static RewriteProperty put(String key, String value) { return new RewriteProperty(RewriteType.PUT, key, value); } @@ -1366,6 +1370,10 @@ public void rewriteForceProperties(Map properties) { forceProperties.forEach(property -> property.rewrite(properties)); } + public ImmutableList getForceProperties() { + return forceProperties; + } + private static Map rewriteReplicaAllocationProperties( String ctl, String db, Map properties) { if (Config.force_olap_table_replication_num <= 0) { diff --git a/regression-test/suites/alter_p0/test_alter_force_property.groovy b/regression-test/suites/alter_p0/test_alter_force_property.groovy new file mode 100644 index 00000000000000..1d1c8b9296aff9 --- /dev/null +++ b/regression-test/suites/alter_p0/test_alter_force_property.groovy @@ -0,0 +1,49 @@ +// 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_alter_force_property') { + def tbl = 'test_alter_force_property_tbl' + sql "DROP TABLE IF EXISTS ${tbl} FORCE" + sql """ + CREATE TABLE ${tbl} + ( + k1 int, + k2 int + ) + DISTRIBUTED BY HASH(k1) BUCKETS 6 + PROPERTIES + ( + "replication_num" = "1" + ); + """ + + if (isCloudCluster()) { + test { + sql "ALTER TABLE ${tbl} SET ('default.replication_num' = '1')" + exception "Cann't modify property 'default.replication_allocation' in cloud mode." + } + test { + sql "ALTER TABLE ${tbl} MODIFY PARTITION (*) SET ('replication_num' = '1')" + exception "Cann't modify property 'replication_num' in cloud mode." + } + } else { + sql "ALTER TABLE ${tbl} SET ('default.replication_num' = '1')" + sql "ALTER TABLE ${tbl} MODIFY PARTITION (*) SET ('replication_num' = '1')" + } + + sql "DROP TABLE IF EXISTS ${tbl} FORCE" +}