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 fcec33fed85 [improvement](alter table) cann't rewrite force property
(#31820)
fcec33fed85 is described below
commit fcec33fed859bdc7d384c9f45954aa3a18213339
Author: yujun <[email protected]>
AuthorDate: Wed Mar 6 00:16:20 2024 +0800
[improvement](alter table) cann't rewrite force property (#31820)
---
.../main/java/org/apache/doris/alter/Alter.java | 24 +++++++++++
.../apache/doris/alter/SchemaChangeHandler.java | 1 +
.../doris/catalog/InternalSchemaInitializer.java | 5 ++-
.../cloud/common/util/CloudPropertyAnalyzer.java | 2 +
.../apache/doris/common/util/PropertyAnalyzer.java | 8 ++++
.../alter_p0/test_alter_force_property.groovy | 49 ++++++++++++++++++++++
6 files changed, 87 insertions(+), 2 deletions(-)
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 28c9afb7222..8c8d0e105b4 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.UserException;
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 @@ public class Alter {
AlterOperations currentAlterOps = new AlterOperations();
currentAlterOps.checkConflict(alterClauses);
+ for (AlterClause clause : alterClauses) {
+ Map<String, String> 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 class Alter {
Map<String, String> properties,
boolean isTempPartition)
throws DdlException, AnalysisException {
+ checkNoForceProperty(properties);
Preconditions.checkArgument(olapTable.isWriteLockHeldByCurrentThread());
List<ModifyPartitionInfo> modifyPartitionInfos = Lists.newArrayList();
olapTable.checkNormalStateForAlter();
@@ -806,6 +821,15 @@ public class Alter {
Env.getCurrentEnv().getEditLog().logBatchModifyPartition(info);
}
+ public void checkNoForceProperty(Map<String, String> 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 bab1a0c32b8..eb116b765e8 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 @@ public class SchemaChangeHandler extends AlterHandler {
*/
public void updateTableProperties(Database db, String tableName,
Map<String, String> properties)
throws UserException {
+
Env.getCurrentEnv().getAlterInstance().checkNoForceProperty(properties);
List<Partition> 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 6ae761f2015..6aaaf4004fb 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 class InternalSchemaInitializer extends Thread {
@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 93cd5e32bb6..34ecf33cf32 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 class CloudPropertyAnalyzer extends PropertyAnalyzer {
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 5c666ec6e2f..1adea8d1b1e 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 @@ public class PropertyAnalyzer {
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 class PropertyAnalyzer {
forceProperties.forEach(property -> property.rewrite(properties));
}
+ public ImmutableList<RewriteProperty> getForceProperties() {
+ return forceProperties;
+ }
+
private static Map<String, String> rewriteReplicaAllocationProperties(
String ctl, String db, Map<String, String> 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 00000000000..1d1c8b9296a
--- /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"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
