From dd15da4e12c40d26d749a08a056075f93eefdbe0 Mon Sep 17 00:00:00 2001 From: Mingyu Chen Date: Mon, 12 Jul 2021 15:31:33 +0800 Subject: [PATCH] [DynamicPartition] Fix UT and add more tests for dynamic partition (#6198) --- .../org/apache/doris/catalog/Catalog.java | 5 +- .../catalog/DynamicPartitionProperty.java | 1 + .../apache/doris/catalog/TableProperty.java | 10 ++ .../common/util/DynamicPartitionUtil.java | 36 +++--- .../catalog/DynamicPartitionTableTest.java | 113 ++++++++++++++++++ 5 files changed, 149 insertions(+), 16 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java index 1bf5a80a3e..6cd57aefb8 100755 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java @@ -5452,8 +5452,11 @@ public class Catalog { if (tableProperty == null) { DynamicPartitionUtil.checkAndSetDynamicPartitionProperty(table, properties); } else { + // Merge the new properties with origin properties, and then analyze them + Map origDynamicProperties = tableProperty.getOriginDynamicPartitionProperty(); + origDynamicProperties.putAll(properties); Map analyzedDynamicPartition = DynamicPartitionUtil. - analyzeDynamicPartition(properties, table.getPartitionInfo()); + analyzeDynamicPartition(origDynamicProperties, table.getPartitionInfo()); tableProperty.modifyTableProperties(analyzedDynamicPartition); tableProperty.buildDynamicProperty(); } 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 1837c63212..42e62abeba 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 @@ -26,6 +26,7 @@ import java.util.Map; import java.util.TimeZone; public class DynamicPartitionProperty { + public static final String DYNAMIC_PARTITION_PROPERTY_PREFIX = "dynamic_partition."; public static final String TIME_UNIT = "dynamic_partition.time_unit"; public static final String START = "dynamic_partition.start"; public static final String END = "dynamic_partition.end"; 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 bf94346dcb..e89d2d9490 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 @@ -149,6 +149,16 @@ public class TableProperty implements Writable { return dynamicPartitionProperty; } + public Map getOriginDynamicPartitionProperty() { + Map origProp = Maps.newHashMap(); + for (Map.Entry entry : properties.entrySet()) { + if (entry.getKey().startsWith(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX)) { + origProp.put(entry.getKey(), entry.getValue()); + } + } + return origProp; + } + public Short getReplicationNum() { return replicationNum; } 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 bf9e2c5b90..c142e880a2 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 @@ -158,7 +158,7 @@ public class DynamicPartitionUtil { } try { int historyPartitionNum = Integer.parseInt(val); - if (historyPartitionNum <= 0) { + if (historyPartitionNum < 0 && historyPartitionNum != DynamicPartitionProperty.NOT_SET_HISTORY_PARTITION_NUM) { ErrorReport.reportDdlException(ErrorCode.ERROR_DYNAMIC_PARTITION_HISTORY_PARTITION_NUM_ZERO); } } catch (NumberFormatException e) { @@ -226,19 +226,17 @@ public class DynamicPartitionUtil { if (properties == null) { return false; } - return properties.containsKey(DynamicPartitionProperty.TIME_UNIT) || - properties.containsKey(DynamicPartitionProperty.TIME_ZONE) || - properties.containsKey(DynamicPartitionProperty.START) || - properties.containsKey(DynamicPartitionProperty.END) || - properties.containsKey(DynamicPartitionProperty.PREFIX) || - properties.containsKey(DynamicPartitionProperty.BUCKETS) || - properties.containsKey(DynamicPartitionProperty.REPLICATION_NUM) || - properties.containsKey(DynamicPartitionProperty.ENABLE) || - properties.containsKey(DynamicPartitionProperty.START_DAY_OF_WEEK) || - properties.containsKey(DynamicPartitionProperty.START_DAY_OF_MONTH) || - properties.containsKey(DynamicPartitionProperty.HOT_PARTITION_NUM); + + for (String key : properties.keySet()) { + if (key.startsWith(DynamicPartitionProperty.DYNAMIC_PARTITION_PROPERTY_PREFIX)) { + return true; + } + } + return false; } + // 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 properties, PartitionInfo partitionInfo) throws DdlException { if (properties == null || properties.isEmpty()) { return false; @@ -254,6 +252,7 @@ public class DynamicPartitionUtil { String buckets = properties.get(DynamicPartitionProperty.BUCKETS); String enable = properties.get(DynamicPartitionProperty.ENABLE); String createHistoryPartition = properties.get(DynamicPartitionProperty.CREATE_HISTORY_PARTITION); + String historyPartitionNum = properties.get(DynamicPartitionProperty.HISTORY_PARTITION_NUM); if (!(Strings.isNullOrEmpty(enable) && Strings.isNullOrEmpty(timeUnit) && Strings.isNullOrEmpty(timeZone) && @@ -261,7 +260,8 @@ public class DynamicPartitionUtil { Strings.isNullOrEmpty(start) && Strings.isNullOrEmpty(end) && Strings.isNullOrEmpty(buckets) && - Strings.isNullOrEmpty(createHistoryPartition))) { + Strings.isNullOrEmpty(createHistoryPartition) && + Strings.isNullOrEmpty(historyPartitionNum))) { if (Strings.isNullOrEmpty(enable)) { properties.put(DynamicPartitionProperty.ENABLE, "true"); } @@ -286,6 +286,10 @@ public class DynamicPartitionUtil { if (Strings.isNullOrEmpty(createHistoryPartition)) { properties.put(DynamicPartitionProperty.CREATE_HISTORY_PARTITION, "false"); } + if (Strings.isNullOrEmpty(historyPartitionNum)) { + properties.put(DynamicPartitionProperty.HISTORY_PARTITION_NUM, + String.valueOf(DynamicPartitionProperty.NOT_SET_HISTORY_PARTITION_NUM)); + } } return true; } @@ -306,6 +310,7 @@ public class DynamicPartitionUtil { } } + // Analyze all properties to check their validation public static Map analyzeDynamicPartition(Map properties, PartitionInfo partitionInfo) throws DdlException { // properties should not be empty, check properties before call this function Map analyzedProperties = new HashMap<>(); @@ -373,7 +378,8 @@ public class DynamicPartitionUtil { // If create_history_partition is false, history partition is not considered. // If create_history_partition is true, will pre-create history partition according the valid value from // start and history_partition_num. - int expectCreatePartitionNum; + // + int expectCreatePartitionNum = 0; if (!createHistoryPartition) { start = 0; expectCreatePartitionNum = end - start; @@ -384,7 +390,7 @@ public class DynamicPartitionUtil { expectCreatePartitionNum = end - Math.max(start, -historyPartitionNum); } else { if (start == Integer.MIN_VALUE) { - throw new DdlException("Provide start or create_history_partition property when creating history partition"); + throw new DdlException("Provide start or history_partition_num property when creating history partition"); } expectCreatePartitionNum = end - start; } diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java index f46c149f1a..72723e6638 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java @@ -17,22 +17,27 @@ package org.apache.doris.catalog; +import org.apache.doris.analysis.AlterTableStmt; import org.apache.doris.analysis.CreateDbStmt; import org.apache.doris.analysis.CreateTableStmt; import org.apache.doris.clone.DynamicPartitionScheduler; import org.apache.doris.common.Config; import org.apache.doris.common.DdlException; +import org.apache.doris.common.ExceptionChecker; import org.apache.doris.common.FeConstants; import org.apache.doris.qe.ConnectContext; import org.apache.doris.thrift.TStorageMedium; import org.apache.doris.utframe.UtFrameUtils; + import com.clearspring.analytics.util.Lists; + import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; + import java.text.SimpleDateFormat; import java.util.Calendar; import java.util.Collections; @@ -77,6 +82,11 @@ public class DynamicPartitionTableTest { Catalog.getCurrentCatalog().createTable(createTableStmt); } + private static void alterTable(String sql) throws Exception { + AlterTableStmt alterTableStmt = (AlterTableStmt) UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext); + Catalog.getCurrentCatalog().alterTable(alterTableStmt); + } + @Test public void testNormal() throws Exception { String createOlapTblStmt = "CREATE TABLE test.`dynamic_partition_normal` (\n" + @@ -586,6 +596,109 @@ public class DynamicPartitionTableTest { createTable(createOlapTblStmt); } + @Test + public void testFillHistoryDynamicPartition3() throws Exception { + String createOlapTblStmt = "CREATE TABLE test.`dynamic_partition3` (\n" + + " `k1` date NULL COMMENT \"\"\n" + + ")\n" + + "PARTITION BY RANGE (k1)\n" + + "()\n" + + "DISTRIBUTED BY HASH(`k1`) BUCKETS 1\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"dynamic_partition.enable\" = \"true\",\n" + + "\"dynamic_partition.end\" = \"3\",\n" + + "\"dynamic_partition.time_unit\" = \"day\",\n" + + "\"dynamic_partition.prefix\" = \"p\",\n" + + "\"dynamic_partition.buckets\" = \"1\",\n" + + "\"dynamic_partition.create_history_partition\" = \"true\"\n" + + ");"; + // start and history_partition_num are not set, can not create history partition + ExceptionChecker.expectThrowsWithMsg(DdlException.class, + "Provide start or history_partition_num property when creating history partition", + () -> createTable(createOlapTblStmt)); + + String createOlapTblStmt2 = "CREATE TABLE test.`dynamic_partition3` (\n" + + " `k1` date NULL COMMENT \"\"\n" + + ")\n" + + "PARTITION BY RANGE (k1)\n" + + "()\n" + + "DISTRIBUTED BY HASH(`k1`) BUCKETS 1\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"dynamic_partition.enable\" = \"true\",\n" + + "\"dynamic_partition.end\" = \"3\",\n" + + "\"dynamic_partition.time_unit\" = \"day\",\n" + + "\"dynamic_partition.prefix\" = \"p\",\n" + + "\"dynamic_partition.buckets\" = \"1\",\n" + + "\"dynamic_partition.history_partition_num\" = \"1000\",\n" + + "\"dynamic_partition.create_history_partition\" = \"true\"\n" + + ");"; + // start is not set, but history_partition_num is set too large, can not create history partition + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Too many dynamic partitions", () -> createTable(createOlapTblStmt2)); + + String createOlapTblStmt3 = "CREATE TABLE test.`dynamic_partition3` (\n" + + " `k1` date NULL COMMENT \"\"\n" + + ")\n" + + "PARTITION BY RANGE (k1)\n" + + "()\n" + + "DISTRIBUTED BY HASH(`k1`) BUCKETS 1\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"dynamic_partition.enable\" = \"true\",\n" + + "\"dynamic_partition.end\" = \"3\",\n" + + "\"dynamic_partition.time_unit\" = \"day\",\n" + + "\"dynamic_partition.prefix\" = \"p\",\n" + + "\"dynamic_partition.buckets\" = \"1\",\n" + + "\"dynamic_partition.start\" = \"-1000\",\n" + + "\"dynamic_partition.create_history_partition\" = \"true\"\n" + + ");"; + // start is set but too small,history_partition_num is not set, can not create history partition + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Too many dynamic partitions", () -> createTable(createOlapTblStmt3)); + + String createOlapTblStmt4 = "CREATE TABLE test.`dynamic_partition3` (\n" + + " `k1` date NULL COMMENT \"\"\n" + + ")\n" + + "PARTITION BY RANGE (k1)\n" + + "()\n" + + "DISTRIBUTED BY HASH(`k1`) BUCKETS 1\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"dynamic_partition.enable\" = \"true\",\n" + + "\"dynamic_partition.end\" = \"3\",\n" + + "\"dynamic_partition.time_unit\" = \"day\",\n" + + "\"dynamic_partition.prefix\" = \"p\",\n" + + "\"dynamic_partition.buckets\" = \"1\",\n" + + "\"dynamic_partition.start\" = \"-10\",\n" + + "\"dynamic_partition.history_partition_num\" = \"5\",\n" + + "\"dynamic_partition.create_history_partition\" = \"true\"\n" + + ");"; + // start and history_partition_num are set, create ok + ExceptionChecker.expectThrowsNoException(() -> createTable(createOlapTblStmt4)); + Database db = Catalog.getCurrentCatalog().getDb("default_cluster:test"); + OlapTable tbl = (OlapTable) db.getTable("dynamic_partition3"); + Assert.assertEquals(9, tbl.getPartitionNames().size()); + + // alter dynamic partition property of table dynamic_partition3 + // start too small + String alter1 = "alter table test.dynamic_partition3 set ('dynamic_partition.start' = '-1000', 'dynamic_partition.history_partition_num' = '1000')"; + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Too many dynamic partitions", () -> alterTable(alter1)); + + // end too large + String alter2 = "alter table test.dynamic_partition3 set ('dynamic_partition.end' = '1000')"; + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Too many dynamic partitions", () -> alterTable(alter2)); + + // history_partition_num too large, but because start is -10, so modify ok + String alter3 = "alter table test.dynamic_partition3 set ('dynamic_partition.history_partition_num' = '1000')"; + ExceptionChecker.expectThrowsNoException(() -> alterTable(alter3)); + Assert.assertEquals(14, tbl.getPartitionNames().size()); + + // set start and history_partition_num properly. + String alter4 = "alter table test.dynamic_partition3 set ('dynamic_partition.history_partition_num' = '100', 'dynamic_partition.start' = '-20')"; + ExceptionChecker.expectThrowsNoException(() -> alterTable(alter4)); + Assert.assertEquals(24, tbl.getPartitionNames().size()); + } + @Test public void testFillHistoryDynamicPartitionWithHistoryPartitionNum() throws Exception { String createOlapTblStmt = "CREATE TABLE test.`history_dynamic_partition_day` (\n" +