From d64d65322bdf7625bb7eff636f354bc060c54530 Mon Sep 17 00:00:00 2001 From: WangCong <1018957763@qq.com> Date: Sun, 2 Aug 2020 09:03:57 -0500 Subject: [PATCH] [Bug][DynamicPartition]Fix bug that Modify a dynamic partition property in a non-dynamic partition table will throw a Exception (#4127) --- .../doris/alter/SchemaChangeHandler.java | 10 ++ .../common/util/DynamicPartitionUtil.java | 6 +- .../org/apache/doris/alter/AlterTest.java | 136 +++++++++++++++--- .../doris/alter/SchemaChangeJobV2Test.java | 3 +- 4 files changed, 129 insertions(+), 26 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 98074f8331..6345f45802 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 @@ -1392,6 +1392,16 @@ public class SchemaChangeHandler extends AlterHandler { sendClearAlterTask(db, olapTable); return; } else if (DynamicPartitionUtil.checkDynamicPartitionPropertiesExist(properties)) { + if (!olapTable.dynamicPartitionExists()) { + try { + DynamicPartitionUtil.checkInputDynamicPartitionProperties(properties, olapTable.getPartitionInfo()); + } catch (DdlException e) { + // This table is not a dynamic partition table and didn't supply all dynamic partition properties + throw new DdlException("Table " + db.getFullName() + "." + + olapTable.getName() + " is not a dynamic partition table. Use command `HELP ALTER TABLE` " + + "to see how to change a normal table to a dynamic partition table."); + } + } Catalog.getCurrentCatalog().modifyTableDynamicPartition(db, olapTable, properties); return; } else if (properties.containsKey("default." + PropertyAnalyzer.PROPERTIES_REPLICATION_NUM)) { 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 8be5b2e203..c83eac0e11 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 @@ -179,7 +179,7 @@ public class DynamicPartitionUtil { properties.containsKey(DynamicPartitionProperty.START_DAY_OF_MONTH); } - private static boolean checkInputDynamicPartitionProperties(Map properties, PartitionInfo partitionInfo) throws DdlException{ + public static boolean checkInputDynamicPartitionProperties(Map properties, PartitionInfo partitionInfo) throws DdlException{ if (properties == null || properties.isEmpty()) { return false; } @@ -192,7 +192,6 @@ public class DynamicPartitionUtil { String timeZone = properties.get(DynamicPartitionProperty.TIME_ZONE); String end = properties.get(DynamicPartitionProperty.END); String buckets = properties.get(DynamicPartitionProperty.BUCKETS); - String replicationNum = properties.get(DynamicPartitionProperty.REPLICATION_NUM); String enable = properties.get(DynamicPartitionProperty.ENABLE); if (!((Strings.isNullOrEmpty(enable) && Strings.isNullOrEmpty(timeUnit) && @@ -312,7 +311,8 @@ public class DynamicPartitionUtil { if (tableProperty != null && tableProperty.getDynamicPartitionProperty() != null && tableProperty.getDynamicPartitionProperty().isExist() && tableProperty.getDynamicPartitionProperty().getEnable()) { - throw new DdlException("Cannot add/drop partition on a Dynamic Partition Table, set `dynamic_partition.enable` to false firstly."); + throw new DdlException("Cannot add/drop partition on a Dynamic Partition Table, " + + "Use command `ALTER TABLE tbl_name SET (\"dynamic_partition.enable\" = \"false\")` firstly."); } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java index 41dadc0ebf..2e921f6abb 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java @@ -66,18 +66,18 @@ public class AlterTest { CreateDbStmt createDbStmt = (CreateDbStmt) UtFrameUtils.parseAndAnalyzeStmt(createDbStmtStr, connectContext); Catalog.getCurrentCatalog().createDb(createDbStmt); - createTable("CREATE TABLE test.tbl1\n" + - "(\n" + - " k1 date,\n" + - " k2 int,\n" + - " v1 int sum\n" + - ")\n" + - "PARTITION BY RANGE(k1)\n" + - "(\n" + - " PARTITION p1 values less than('2020-02-01'),\n" + - " PARTITION p2 values less than('2020-03-01')\n" + - ")\n" + - "DISTRIBUTED BY HASH(k2) BUCKETS 3\n" + + createTable("CREATE TABLE test.tbl1\n" + + "(\n" + + " k1 date,\n" + + " k2 int,\n" + + " v1 int sum\n" + + ")\n" + + "PARTITION BY RANGE(k1)\n" + + "(\n" + + " PARTITION p1 values less than('2020-02-01'),\n" + + " PARTITION p2 values less than('2020-03-01')\n" + + ")\n" + + "DISTRIBUTED BY HASH(k2) BUCKETS 3\n" + "PROPERTIES('replication_num' = '1');"); createTable("CREATE TABLE test.tbl2\n" + @@ -151,6 +151,15 @@ public class AlterTest { } } + private static void alterTableWithExceptionMsg(String sql, String msg) throws Exception { + AlterTableStmt alterTableStmt = (AlterTableStmt) UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext); + try { + Catalog.getCurrentCatalog().alterTable(alterTableStmt); + } catch (Exception e) { + Assert.assertEquals(msg, e.getMessage()); + } + } + @Test public void testConflictAlterOperations() throws Exception { String stmt = "alter table test.tbl1 add partition p3 values less than('2020-04-01'), add partition p4 values less than('2020-05-01')"; @@ -169,7 +178,7 @@ public class AlterTest { stmt = "alter table test.tbl1 add column k3 int, add column k4 int"; alterTable(stmt, false); waitSchemaChangeJobDone(false); - + stmt = "alter table test.tbl1 add rollup r1 (k1)"; alterTable(stmt, false); waitSchemaChangeJobDone(true); @@ -177,22 +186,22 @@ public class AlterTest { stmt = "alter table test.tbl1 add rollup r2 (k1), r3 (k1)"; alterTable(stmt, false); waitSchemaChangeJobDone(true); - + // enable dynamic partition // not adding the `start` property so that it won't drop the origin partition p1, p2 and p3 - stmt = "alter table test.tbl1 set (\n" + - "'dynamic_partition.enable' = 'true',\n" + + stmt = "alter table test.tbl1 set (\n" + + "'dynamic_partition.enable' = 'true',\n" + "'dynamic_partition.time_unit' = 'DAY',\n" + - "'dynamic_partition.end' = '3',\n" + - "'dynamic_partition.prefix' = 'p',\n" + - "'dynamic_partition.buckets' = '3'\n" + + "'dynamic_partition.end' = '3',\n" + + "'dynamic_partition.prefix' = 'p',\n" + + "'dynamic_partition.buckets' = '3'\n" + " );"; alterTable(stmt, false); Database db = Catalog.getCurrentCatalog().getDb("default_cluster:test"); - OlapTable tbl = (OlapTable)db.getTable("tbl1"); + OlapTable tbl = (OlapTable) db.getTable("tbl1"); Assert.assertTrue(tbl.getTableProperty().getDynamicPartitionProperty().getEnable()); Assert.assertEquals(4, tbl.getIndexIdToSchema().size()); - + // add partition when dynamic partition is enable stmt = "alter table test.tbl1 add partition p3 values less than('2020-04-01') distributed by hash(k2) buckets 4 PROPERTIES ('replication_num' = '1')"; alterTable(stmt, true); @@ -206,7 +215,7 @@ public class AlterTest { stmt = "alter table test.tbl1 set ('dynamic_partition.enable' = 'false')"; alterTable(stmt, false); Assert.assertFalse(tbl.getTableProperty().getDynamicPartitionProperty().getEnable()); - + // add partition when dynamic partition is disable stmt = "alter table test.tbl1 add partition p3 values less than('2020-04-01') distributed by hash(k2) buckets 4"; alterTable(stmt, false); @@ -336,4 +345,87 @@ public class AlterTest { Assert.assertEquals(AlterJobV2.JobState.FINISHED, alterJobV2.getJobState()); } } + + @Test + public void testSetDynamicPropertiesInNormalTable() throws Exception { + String tableName = "no_dynamic_table"; + String createOlapTblStmt = "CREATE TABLE test.`" + tableName + "` (\n" + + " `k1` date NULL COMMENT \"\",\n" + + " `k2` int NULL COMMENT \"\",\n" + + " `k3` smallint NULL COMMENT \"\",\n" + + " `v1` varchar(2048) NULL COMMENT \"\",\n" + + " `v2` datetime NULL COMMENT \"\"\n" + + ") ENGINE=OLAP\n" + + "DUPLICATE KEY(`k1`, `k2`, `k3`)\n" + + "COMMENT \"OLAP\"\n" + + "PARTITION BY RANGE (k1)\n" + + "(\n" + + "PARTITION p1 VALUES LESS THAN (\"2014-01-01\"),\n" + + "PARTITION p2 VALUES LESS THAN (\"2014-06-01\"),\n" + + "PARTITION p3 VALUES LESS THAN (\"2014-12-01\")\n" + + ")\n" + + "DISTRIBUTED BY HASH(`k1`) BUCKETS 32\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\"\n" + + ");"; + createTable(createOlapTblStmt); + String alterStmt = "alter table test." + tableName + " set (\"dynamic_partition.enable\" = \"true\");"; + String errorMsg = "errCode = 2, detailMessage = Table default_cluster:test.no_dynamic_table is not a dynamic partition table. " + + "Use command `HELP ALTER TABLE` to see how to change a normal table to a dynamic partition table."; + alterTableWithExceptionMsg(alterStmt, errorMsg); + // test set dynamic properties in a no dynamic partition table + String stmt = "alter table test." + tableName + " set (\n" + + "'dynamic_partition.enable' = 'true',\n" + + "'dynamic_partition.time_unit' = 'DAY',\n" + + "'dynamic_partition.start' = '-3',\n" + + "'dynamic_partition.end' = '3',\n" + + "'dynamic_partition.prefix' = 'p',\n" + + "'dynamic_partition.buckets' = '3'\n" + + " );"; + alterTable(stmt, false); + } + + @Test + public void testSetDynamicPropertiesInDynamicPartitionTable() throws Exception { + String tableName = "dynamic_table"; + String createOlapTblStmt = "CREATE TABLE test.`" + tableName + "` (\n" + + " `k1` date NULL COMMENT \"\",\n" + + " `k2` int NULL COMMENT \"\",\n" + + " `k3` smallint NULL COMMENT \"\",\n" + + " `v1` varchar(2048) NULL COMMENT \"\",\n" + + " `v2` datetime NULL COMMENT \"\"\n" + + ") ENGINE=OLAP\n" + + "DUPLICATE KEY(`k1`, `k2`, `k3`)\n" + + "COMMENT \"OLAP\"\n" + + "PARTITION BY RANGE (k1)\n" + + "(\n" + + "PARTITION p1 VALUES LESS THAN (\"2014-01-01\"),\n" + + "PARTITION p2 VALUES LESS THAN (\"2014-06-01\"),\n" + + "PARTITION p3 VALUES LESS THAN (\"2014-12-01\")\n" + + ")\n" + + "DISTRIBUTED BY HASH(`k1`) BUCKETS 32\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"dynamic_partition.enable\" = \"true\",\n" + + "\"dynamic_partition.start\" = \"-3\",\n" + + "\"dynamic_partition.end\" = \"3\",\n" + + "\"dynamic_partition.time_unit\" = \"day\",\n" + + "\"dynamic_partition.prefix\" = \"p\",\n" + + "\"dynamic_partition.buckets\" = \"1\"\n" + + ");"; + + createTable(createOlapTblStmt); + String alterStmt1 = "alter table test." + tableName + " set (\"dynamic_partition.enable\" = \"false\");"; + alterTable(alterStmt1, false); + String alterStmt2 = "alter table test." + tableName + " set (\"dynamic_partition.time_unit\" = \"week\");"; + alterTable(alterStmt2, false); + String alterStmt3 = "alter table test." + tableName + " set (\"dynamic_partition.start\" = \"-10\");"; + alterTable(alterStmt3, false); + String alterStmt4 = "alter table test." + tableName + " set (\"dynamic_partition.end\" = \"10\");"; + alterTable(alterStmt4, false); + String alterStmt5 = "alter table test." + tableName + " set (\"dynamic_partition.prefix\" = \"pp\");"; + alterTable(alterStmt5, false); + String alterStmt6 = "alter table test." + tableName + " set (\"dynamic_partition.buckets\" = \"5\");"; + alterTable(alterStmt6, false); + } } 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 1eea8bb5b0..b48c8b811d 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 @@ -372,7 +372,8 @@ public class SchemaChangeJobV2Test { OlapTable olapTable = (OlapTable) db.getTable(CatalogMocker.TEST_TBL2_ID); expectedEx.expect(DdlException.class); - expectedEx.expectMessage(String.format("Must assign %s properties", missPropertyKey)); + expectedEx.expectMessage("errCode = 2, detailMessage = Table test_db.test_tbl2 is not a dynamic partition table. " + + "Use command `HELP ALTER TABLE` to see how to change a normal table to a dynamic partition table."); schemaChangeHandler.process(alterClauses, "default_cluster", db, olapTable); }