From 7d49d9cf99e231adf09c77bf2bfb89b08b52cbc8 Mon Sep 17 00:00:00 2001 From: Jack Drogon Date: Sun, 2 Apr 2023 15:51:21 +0800 Subject: [PATCH] [improvement](dynamic partition) Fix dynamic partition no bucket (#18300) Signed-off-by: Jack Drogon --- .../doris/alter/SchemaChangeHandler.java | 2 +- .../common/util/DynamicPartitionUtil.java | 12 ++- .../catalog/DynamicPartitionTableTest.java | 6 +- .../suites/autobucket/test_autobucket.groovy | 3 +- .../test_autobucket_dynamic_partition.groovy | 48 ++++++++++++ .../test_dynamic_partition.groovy | 73 ++++++++++++------- 6 files changed, 110 insertions(+), 34 deletions(-) create mode 100644 regression-test/suites/autobucket/test_autobucket_dynamic_partition.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 3a38db241d..bcd3614a47 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 @@ -1755,7 +1755,7 @@ public class SchemaChangeHandler extends AlterHandler { if (!olapTable.dynamicPartitionExists()) { try { DynamicPartitionUtil.checkInputDynamicPartitionProperties(properties, - olapTable.getPartitionInfo()); + olapTable); } catch (DdlException e) { // This table is not a dynamic partition table // and didn't supply all dynamic partition properties 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 2ceb2c30a9..d196d92135 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 @@ -20,6 +20,7 @@ package org.apache.doris.common.util; import org.apache.doris.analysis.TimestampArithmeticExpr.TimeUnit; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.DistributionInfo; import org.apache.doris.catalog.DynamicPartitionProperty; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.OlapTable; @@ -399,10 +400,12 @@ public class DynamicPartitionUtil { // 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 { + OlapTable olapTable) throws DdlException { if (properties == null || properties.isEmpty()) { return false; } + + PartitionInfo partitionInfo = olapTable.getPartitionInfo(); if (partitionInfo.getType() != PartitionType.RANGE || partitionInfo.isMultiColumnPartition()) { throw new DdlException("Dynamic partition only support single-column range partition"); } @@ -443,7 +446,9 @@ public class DynamicPartitionUtil { throw new DdlException("Must assign dynamic_partition.end properties"); } if (Strings.isNullOrEmpty(buckets)) { - throw new DdlException("Must assign dynamic_partition.buckets properties"); + DistributionInfo distributionInfo = olapTable.getDefaultDistributionInfo(); + buckets = String.valueOf(distributionInfo.getBucketNum()); + properties.put(DynamicPartitionProperty.BUCKETS, buckets); } if (Strings.isNullOrEmpty(timeZone)) { properties.put(DynamicPartitionProperty.TIME_ZONE, TimeUtils.getSystemTimeZone().getID()); @@ -506,6 +511,7 @@ public class DynamicPartitionUtil { properties.remove(DynamicPartitionProperty.BUCKETS); analyzedProperties.put(DynamicPartitionProperty.BUCKETS, bucketsValue); } + if (properties.containsKey(DynamicPartitionProperty.ENABLE)) { String enableValue = properties.get(DynamicPartitionProperty.ENABLE); checkEnable(enableValue); @@ -675,7 +681,7 @@ public class DynamicPartitionUtil { */ public static void checkAndSetDynamicPartitionProperty(OlapTable olapTable, Map properties, Database db) throws UserException { - if (DynamicPartitionUtil.checkInputDynamicPartitionProperties(properties, olapTable.getPartitionInfo())) { + if (DynamicPartitionUtil.checkInputDynamicPartitionProperties(properties, olapTable)) { Map dynamicPartitionProperties = DynamicPartitionUtil.analyzeDynamicPartition(properties, olapTable, db); TableProperty tableProperty = olapTable.getTableProperty(); 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 3a522ad3a6..ff6d1f687d 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 @@ -258,7 +258,7 @@ public class DynamicPartitionTableTest { @Test public void testMissBuckets() throws Exception { - String createOlapTblStmt = "CREATE TABLE test.`dynamic_partition_buckets` (\n" + String createOlapTblStmt = "CREATE TABLE test.`dynamic_partition_miss_buckets` (\n" + " `k1` date NULL COMMENT \"\",\n" + " `k2` int NULL COMMENT \"\",\n" + " `k3` smallint NULL COMMENT \"\",\n" @@ -282,14 +282,12 @@ public class DynamicPartitionTableTest { + "\"dynamic_partition.time_unit\" = \"day\",\n" + "\"dynamic_partition.prefix\" = \"p\"\n" + ");"; - expectedException.expect(DdlException.class); - expectedException.expectMessage("errCode = 2, detailMessage = Must assign dynamic_partition.buckets properties"); createTable(createOlapTblStmt); } @Test public void testNotAllowed() throws Exception { - String createOlapTblStmt = "CREATE TABLE test.`dynamic_partition_buckets` (\n" + String createOlapTblStmt = "CREATE TABLE test.`dynamic_partition_not_allowed` (\n" + " `k1` date NULL COMMENT \"\",\n" + " `k2` int NULL COMMENT \"\",\n" + " `k3` smallint NULL COMMENT \"\",\n" diff --git a/regression-test/suites/autobucket/test_autobucket.groovy b/regression-test/suites/autobucket/test_autobucket.groovy index 29945e0f9a..ab0ae99658 100644 --- a/regression-test/suites/autobucket/test_autobucket.groovy +++ b/regression-test/suites/autobucket/test_autobucket.groovy @@ -25,7 +25,7 @@ suite("test_autobucket") { COMMENT 'OLAP' DISTRIBUTED BY HASH(`user_id`) BUCKETS AUTO PROPERTIES ( - "replication_allocation" = "tag.location.default: 1" + "replication_allocation" = "tag.location.default: 1" ) """ @@ -35,6 +35,7 @@ suite("test_autobucket") { result = sql "show partitions from autobucket_test" logger.info("${result}") // XXX: buckets at pos(8), next maybe impl by sql meta + // 10 is the default buckets without partition size assertEquals(Integer.valueOf(result.get(0).get(8)), 10) sql "drop table if exists autobucket_test" diff --git a/regression-test/suites/autobucket/test_autobucket_dynamic_partition.groovy b/regression-test/suites/autobucket/test_autobucket_dynamic_partition.groovy new file mode 100644 index 0000000000..80679cb07d --- /dev/null +++ b/regression-test/suites/autobucket/test_autobucket_dynamic_partition.groovy @@ -0,0 +1,48 @@ +// 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_autobucket_dynamic_partition") { + sql "drop table if exists test_autobucket_dynamic_partition" + result = sql """ + CREATE TABLE + test_autobucket_dynamic_partition (k1 DATETIME) + PARTITION BY + RANGE (k1) () DISTRIBUTED BY HASH (k1) BUCKETS AUTO + PROPERTIES ( + "dynamic_partition.enable" = "true", + "dynamic_partition.time_unit" = "WEEK", + "dynamic_partition.start" = "-2", + "dynamic_partition.end" = "2", + "dynamic_partition.prefix" = "p", + "replication_allocation" = "tag.location.default: 1" + ) + """ + + result = sql "show create table test_autobucket_dynamic_partition" + assertTrue(result.toString().containsIgnoreCase("BUCKETS AUTO")) + + result = sql "show partitions from test_autobucket_dynamic_partition" + logger.info("${result}") + // XXX: buckets at pos(8), next maybe impl by sql meta + // 10 is the default buckets without partition size + assertEquals(result.size(), 3) + assertEquals(Integer.valueOf(result.get(0).get(8)), 10) + assertEquals(Integer.valueOf(result.get(1).get(8)), 10) + assertEquals(Integer.valueOf(result.get(2).get(8)), 10) + + sql "drop table if exists test_autobucket_dynamic_partition" +} diff --git a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition.groovy b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition.groovy index f80de41841..16246bf629 100644 --- a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition.groovy +++ b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition.groovy @@ -14,44 +14,68 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - suite("test_dynamic_partition") { // todo: test dynamic partition sql "drop table if exists dy_par" sql """ - CREATE TABLE IF NOT EXISTS dy_par ( k1 date NOT NULL, k2 varchar(20) NOT NULL, k3 int sum NOT NULL ) - AGGREGATE KEY(k1,k2) - PARTITION BY RANGE(k1) ( ) - DISTRIBUTED BY HASH(k1) BUCKETS 3 - PROPERTIES ( - "dynamic_partition.enable"="true", - "dynamic_partition.end"="3", - "dynamic_partition.buckets"="10", - "dynamic_partition.start"="-3", - "dynamic_partition.prefix"="p", - "dynamic_partition.time_unit"="DAY", + CREATE TABLE IF NOT EXISTS dy_par ( k1 date NOT NULL, k2 varchar(20) NOT NULL, k3 int sum NOT NULL ) + AGGREGATE KEY(k1,k2) + PARTITION BY RANGE(k1) ( ) + DISTRIBUTED BY HASH(k1) BUCKETS 3 + PROPERTIES ( + "dynamic_partition.enable"="true", + "dynamic_partition.end"="3", + "dynamic_partition.buckets"="10", + "dynamic_partition.start"="-3", + "dynamic_partition.prefix"="p", + "dynamic_partition.time_unit"="DAY", "dynamic_partition.create_history_partition"="true", "dynamic_partition.replication_allocation" = "tag.location.default: 1") """ List> result = sql "show tables like 'dy_par'" logger.info("${result}") assertEquals(result.size(), 1) + result = sql "show partitions from dy_par" + // XXX: buckets at pos(8), next maybe impl by sql meta + assertEquals(Integer.valueOf(result.get(0).get(8)), 10) sql "drop table dy_par" - // + sql "drop table if exists dy_par_bucket_set_by_distribution" + sql """ + CREATE TABLE IF NOT EXISTS dy_par_bucket_set_by_distribution + ( k1 date NOT NULL, k2 varchar(20) NOT NULL, k3 int sum NOT NULL ) + AGGREGATE KEY(k1,k2) + PARTITION BY RANGE(k1) ( ) + DISTRIBUTED BY HASH(k1) BUCKETS 3 + PROPERTIES ( + "dynamic_partition.enable"="true", + "dynamic_partition.end"="3", + "dynamic_partition.start"="-3", + "dynamic_partition.prefix"="p", + "dynamic_partition.time_unit"="DAY", + "dynamic_partition.create_history_partition"="true", + "dynamic_partition.replication_allocation" = "tag.location.default: 1") + """ + result = sql "show tables like 'dy_par_bucket_set_by_distribution'" + logger.info("${result}") + assertEquals(result.size(), 1) + result = sql "show partitions from dy_par_bucket_set_by_distribution" + // XXX: buckets at pos(8), next maybe impl by sql meta + assertEquals(Integer.valueOf(result.get(0).get(8)), 3) + sql "drop table dy_par_bucket_set_by_distribution" sql "drop table if exists dy_par_bad" test { sql """ - CREATE TABLE IF NOT EXISTS dy_par_bad ( k1 date NOT NULL, k2 varchar(20) NOT NULL, k3 int sum NOT NULL ) - AGGREGATE KEY(k1,k2) - PARTITION BY RANGE(k1) ( ) - DISTRIBUTED BY HASH(k1) BUCKETS 3 - PROPERTIES ( - "dynamic_partition.enable"="true", - "dynamic_partition.end"="3", - "dynamic_partition.buckets"="10", - "dynamic_partition.start"="-3", - "dynamic_partition.prefix"="p", - "dynamic_partition.time_unit"="DAY", + CREATE TABLE IF NOT EXISTS dy_par_bad ( k1 date NOT NULL, k2 varchar(20) NOT NULL, k3 int sum NOT NULL ) + AGGREGATE KEY(k1,k2) + PARTITION BY RANGE(k1) ( ) + DISTRIBUTED BY HASH(k1) BUCKETS 3 + PROPERTIES ( + "dynamic_partition.enable"="true", + "dynamic_partition.end"="3", + "dynamic_partition.buckets"="10", + "dynamic_partition.start"="-3", + "dynamic_partition.prefix"="p", + "dynamic_partition.time_unit"="DAY", "dynamic_partition.create_history_partition"="true", "dynamic_partition.replication_allocation" = "tag.location.not_exist_tag: 1") """ @@ -59,7 +83,6 @@ suite("test_dynamic_partition") { exception "errCode = 2," } sql "drop table if exists dy_par_bad" - sql """ CREATE TABLE IF NOT EXISTS dy_par ( k1 datev2 NOT NULL, k2 varchar(20) NOT NULL, k3 int sum NOT NULL ) AGGREGATE KEY(k1,k2)