This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
commit cfd34cebf348a0cf2dce0ea85e72f9b2a203bd94
Author: Lijia Liu <liutang...@yeah.net>
AuthorDate: Fri Jun 14 14:20:44 2024 +0800
[BUG](Meta) Fix fe exit by wrong dynamic properties (#28138)
## Proposed changes
Issue Number: close #xxx
`ALTER TABLE tbl SET ("dynamic_partition.some_words" = "");` will cause
fe down.
```
java.lang.NumberFormatException: null
at java.lang.Integer.parseInt(Integer.java:620) ~[?:?]
at java.lang.Integer.parseInt(Integer.java:776) ~[?:?]
at
org.apache.doris.catalog.DynamicPartitionProperty.<init>(DynamicPartitionProperty.java:75)
~[palo-fe.jar:3.4.0]
at
org.apache.doris.catalog.TableProperty.executeBuildDynamicProperty(TableProperty.java:115)
~[palo-fe.jar:3.4.0]
at
org.apache.doris.catalog.TableProperty.buildProperty(TableProperty.java:82)
~[palo-fe.jar:3.4.0]
at
org.apache.doris.catalog.Catalog.replayModifyTableProperty(Catalog.java:5646)
~[palo-fe.jar:3.4.0]
at org.apache.doris.persist.EditLog.loadJournal(EditLog.java:763)
[palo-fe.jar:3.4.0]
at
org.apache.doris.catalog.Catalog.replayJournal(Catalog.java:2516)
[palo-fe.jar:3.4.0]
at
org.apache.doris.catalog.Catalog$3.runOneCycle(Catalog.java:2299)
[palo-fe.jar:3.4.0]
at org.apache.doris.common.util.Daemon.run(Daemon.java:116)
[palo-fe.jar:3.4.0]
```
<!--Describe your changes.-->
Prevent setting incorrect table properties
## Further comments
If this is a relatively large or complex change, kick off the discussion
at [d...@doris.apache.org](mailto:d...@doris.apache.org) by explaining why
you chose the solution you did and what alternatives you considered,
etc...
---------
Co-authored-by: liutang123 <liuli...@gmail.com>
---
.../apache/doris/alter/SchemaChangeHandler.java | 1 +
.../analysis/ModifyTablePropertiesClause.java | 4 ++-
.../doris/catalog/DynamicPartitionProperty.java | 8 ++++++
.../org/apache/doris/catalog/TableProperty.java | 10 +++++---
.../doris/common/util/DynamicPartitionUtil.java | 20 ++++++++++++++-
.../apache/doris/datasource/InternalCatalog.java | 1 +
.../apache/doris/alter/SchemaChangeJobV2Test.java | 24 ++++++++++++++++++
.../test_dynamic_partition_failed.groovy | 29 ++++++++++++++++++++++
.../test_dynamic_partition_with_alter.groovy | 5 ++++
9 files changed, 96 insertions(+), 6 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 b214afe387e..ab77e242a8a 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
@@ -1883,6 +1883,7 @@ public class SchemaChangeHandler extends AlterHandler {
sendClearAlterTask(db, olapTable);
return;
} else if
(DynamicPartitionUtil.checkDynamicPartitionPropertiesExist(properties)) {
+
DynamicPartitionUtil.checkDynamicPartitionPropertyKeysValid(properties);
if (!olapTable.dynamicPartitionExists()) {
try {
DynamicPartitionUtil.checkInputDynamicPartitionProperties(properties,
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java
index fd85ad0978e..0895890533a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java
@@ -18,6 +18,7 @@
package org.apache.doris.analysis;
import org.apache.doris.alter.AlterOpType;
+import org.apache.doris.catalog.DynamicPartitionProperty;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.MTMV;
import org.apache.doris.catalog.ReplicaAllocation;
@@ -71,7 +72,8 @@ public class ModifyTablePropertiesClause extends
AlterTableClause {
}
if (properties.size() != 1
- && !TableProperty.isSamePrefixProperties(properties,
TableProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX)
+ && !TableProperty.isSamePrefixProperties(
+ properties,
DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX)
&& !TableProperty.isSamePrefixProperties(properties,
PropertyAnalyzer.PROPERTIES_BINLOG_PREFIX)) {
throw new AnalysisException(
"Can only set one table property(without dynamic partition
&& binlog) at a time");
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/DynamicPartitionProperty.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/DynamicPartitionProperty.java
index 816bb64e0ad..918b0dc9d55 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/catalog/DynamicPartitionProperty.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/catalog/DynamicPartitionProperty.java
@@ -28,7 +28,10 @@ import org.apache.doris.common.util.TimeUtils;
import com.google.common.base.Strings;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import java.util.TimeZone;
import java.util.function.BiConsumer;
@@ -52,6 +55,11 @@ public class DynamicPartitionProperty {
public static final String STORAGE_POLICY =
"dynamic_partition.storage_policy";
public static final String STORAGE_MEDIUM =
"dynamic_partition.storage_medium";
+ public static final Set<String> DYNAMIC_PARTITION_PROPERTIES = new
HashSet<>(
+ Arrays.asList(TIME_UNIT, START, END, PREFIX, BUCKETS, ENABLE,
START_DAY_OF_WEEK, START_DAY_OF_MONTH,
+ TIME_ZONE, REPLICATION_NUM, REPLICATION_ALLOCATION,
CREATE_HISTORY_PARTITION, HISTORY_PARTITION_NUM,
+ HOT_PARTITION_NUM, RESERVED_HISTORY_PERIODS,
STORAGE_POLICY, STORAGE_MEDIUM));
+
public static final int MIN_START_OFFSET = Integer.MIN_VALUE;
public static final int MAX_END_OFFSET = Integer.MAX_VALUE;
public static final int NOT_SET_REPLICATION_NUM = -1;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
index 8f010eead54..f9625a7506e 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java
@@ -53,8 +53,6 @@ import java.util.Map;
public class TableProperty implements Writable {
private static final Logger LOG =
LogManager.getLogger(TableProperty.class);
- public static final String DYNAMIC_PARTITION_PROPERTY_PREFIX =
"dynamic_partition";
-
@SerializedName(value = "properties")
private Map<String, String> properties;
@@ -187,10 +185,14 @@ public class TableProperty implements Writable {
private TableProperty executeBuildDynamicProperty() {
HashMap<String, String> dynamicPartitionProperties = new HashMap<>();
for (Map.Entry<String, String> entry : properties.entrySet()) {
- if (entry.getKey().startsWith(DYNAMIC_PARTITION_PROPERTY_PREFIX)) {
+ if
(entry.getKey().startsWith(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX))
{
+ if
(!DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTIES.contains(entry.getKey()))
{
+ LOG.warn("Ignore invalid dynamic property key: {}: value:
{}", entry.getKey(), entry.getValue());
+ }
dynamicPartitionProperties.put(entry.getKey(),
entry.getValue());
}
}
+
dynamicPartitionProperty =
EnvFactory.getInstance().createDynamicPartitionProperty(dynamicPartitionProperties);
return this;
}
@@ -491,7 +493,7 @@ public class TableProperty implements Writable {
public Map<String, String> getOriginDynamicPartitionProperty() {
Map<String, String> origProp = Maps.newHashMap();
for (Map.Entry<String, String> entry : properties.entrySet()) {
- if
(entry.getKey().startsWith(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX))
{
+ if
(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTIES.contains(entry.getKey()))
{
origProp.put(entry.getKey(), entry.getValue());
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java
b/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java
index 6f001ddb2d4..c03ef1b2017 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java
@@ -64,6 +64,7 @@ import java.util.Calendar;
import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.TimeZone;
@@ -419,10 +420,27 @@ public class DynamicPartitionUtil {
return false;
}
+ public static void checkDynamicPartitionPropertyKeysValid(Map<String,
String> properties) throws DdlException {
+ if (properties == null) {
+ return;
+ }
+ List<String> invalidDynamicPartitionProperties = new LinkedList<>();
+ for (String key : properties.keySet()) {
+ if
(key.startsWith(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX)
+ &&
!DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTIES.contains(key)) {
+ invalidDynamicPartitionProperties.add(key);
+ }
+ }
+ if (!invalidDynamicPartitionProperties.isEmpty()) {
+ throw new DdlException("Invalid dynamic partition properties: "
+ + String.join(", ", invalidDynamicPartitionProperties));
+ }
+ }
+
// Check if all requried properties has been set.
// And also check all optional properties, if not set, set them to default
value.
public static boolean checkInputDynamicPartitionProperties(Map<String,
String> properties,
- OlapTable olapTable) throws DdlException {
+ OlapTable
olapTable) throws DdlException {
if (properties == null || properties.isEmpty()) {
return false;
}
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 4bc21b27347..5a2a8c48793 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
@@ -2813,6 +2813,7 @@ public class InternalCatalog implements
CatalogIf<Database> {
// and then check if there still has unknown properties
olapTable.setStorageMedium(dataProperty.getStorageMedium());
if (partitionInfo.getType() == PartitionType.RANGE) {
+
DynamicPartitionUtil.checkDynamicPartitionPropertyKeysValid(properties);
DynamicPartitionUtil.checkAndSetDynamicPartitionProperty(olapTable, properties,
db);
} else if (partitionInfo.getType() == PartitionType.LIST) {
if
(DynamicPartitionUtil.checkDynamicPartitionPropertiesExist(properties)) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
index 6db68473bdb..4ae7d2775e3 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
@@ -389,6 +389,30 @@ public class SchemaChangeJobV2Test {
modifyDynamicPartitionWithoutTableProperty(DynamicPartitionProperty.BUCKETS,
"30");
}
+ @Test
+ public void testModifyDynamicPartitionWithInvalidProperty() throws
UserException {
+ fakeEnv = new FakeEnv();
+ fakeEditLog = new FakeEditLog();
+ FakeEnv.setEnv(masterEnv);
+ SchemaChangeHandler schemaChangeHandler =
Env.getCurrentEnv().getSchemaChangeHandler();
+ ArrayList<AlterClause> alterClauses = new ArrayList<>();
+ Map<String, String> properties = new HashMap<>();
+ properties.put(DynamicPartitionProperty.ENABLE, "true");
+
properties.put(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX +
"time_uint", "day");
+
properties.put(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX +
"edn", "3");
+ properties.put(DynamicPartitionProperty.PREFIX, "p");
+ properties.put(DynamicPartitionProperty.BUCKETS, "30");
+ properties.put("invalid_property", "invalid_value");
+ alterClauses.add(new ModifyTablePropertiesClause(properties));
+
+ Database db = CatalogMocker.mockDb();
+ OlapTable olapTable = (OlapTable)
db.getTableOrDdlException(CatalogMocker.TEST_TBL2_ID);
+ expectedEx.expect(DdlException.class);
+ expectedEx.expectMessage("errCode = 2,"
+ + " detailMessage = Invalid dynamic partition properties:
dynamic_partition.time_uint, dynamic_partition.edn");
+ schemaChangeHandler.process(alterClauses, db, olapTable);
+ }
+
@Test
public void testSerializeOfSchemaChangeJob() throws IOException {
// prepare file
diff --git
a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
index 7da145a36bc..a19efadf740 100644
---
a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
+++
b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
@@ -65,10 +65,39 @@ suite('test_dynamic_partition_failed', 'nonConcurrent') {
// 'date/datetime literal [+271768-09-11 00:00:00] is invalid'
assertTrue(msg.contains('date/datetime literal') &&
msg.contains('is invalid'))
}
+
+ }
+
+ sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_3'
+ test {
+ sql '''CREATE TABLE test_dynamic_partition_failed_3
+ ( `k1` datetime NULL )
+ PARTITION BY RANGE (k1)()
+ DISTRIBUTED BY HASH(`k1`) BUCKETS 1
+ PROPERTIES
+ (
+ "replication_num" = "1",
+ "dynamic_partition.enable" = "true",
+ "dynamic_partition.end" = "3",
+ "dynamic_partition.time_uint" = "day",
+ "dynamic_partition.prefix" = "p",
+ "dynamic_partition.buckets" = "1",
+ "dynamic_partition.start" = "2024-06-11",
+ "dynamic_partition.edn" = "2024-06-13",
+ "dynamic_partition.create_history_partition" = "true"
+ )'''
+ check { result, exception, startTime, endTime ->
+ assertNotNull(exception)
+ def msg = exception.toString()
+ logger.info("exception: " + msg)
+ // 'Invalid dynamic partition properties:
dynamic_partition.time_uint, dynamic_partition.edn'
+ assertTrue(msg.contains('Invalid dynamic partition properties:
dynamic_partition.time_uint, dynamic_partition.edn'))
+ }
}
} finally {
setFeConfig('max_dynamic_partition_num', old_max_dynamic_partition_num)
sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_1'
sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_2'
+ sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_3'
}
}
diff --git
a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_with_alter.groovy
b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_with_alter.groovy
index d488a53812f..fe27fdbd77a 100644
---
a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_with_alter.groovy
+++
b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_with_alter.groovy
@@ -48,6 +48,11 @@ suite("test_dynamic_partition_with_alter") {
sql "alter table ${tbl} set ('dynamic_partition.start' =
'-2147483647')"
exception "Too many dynamic partitions"
}
+
+ test {
+ sql "alter table ${tbl} set ('dynamic_partition.time_uint' =
'day')"
+ exception "Invalid dynamic partition properties:
dynamic_partition.time_uint"
+ }
} catch (Exception e) {
sql "drop table if exists ${tbl}"
throw e
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
