From 51af468cdd51d5a53e88d58e2594f916e4676598 Mon Sep 17 00:00:00 2001 From: shee <13843187+qzsee@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:30:18 +0800 Subject: [PATCH] [BUG] fix partition storage policy info lost (#38700) ## Proposed changes 1. fix partition storage policy info lost When adding a storage policy to a table through an alter statement, the partition policy is lost when the FE is restarted because the storage policy is not set for the partition synchronously. 2. when setting policies, check the uniq table in advance to prevent metadata inconsistencies 3. show storage policy using for stmt support any string policy name If the policy name begins with a number, the statement cannot be parsed. Issue Number: close #xxx --------- Co-authored-by: garenshi --- fe/fe-core/src/main/cup/sql_parser.cup | 2 +- .../java/org/apache/doris/alter/Alter.java | 15 ++++++++++++++- .../doris/alter/SchemaChangeHandler.java | 5 ----- .../java/org/apache/doris/catalog/Env.java | 3 +++ .../java/org/apache/doris/alter/AlterTest.java | 18 ++++++++++++++++++ 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/fe/fe-core/src/main/cup/sql_parser.cup b/fe/fe-core/src/main/cup/sql_parser.cup index a0636a7650d228..1cbd28ee43f5f4 100644 --- a/fe/fe-core/src/main/cup/sql_parser.cup +++ b/fe/fe-core/src/main/cup/sql_parser.cup @@ -3952,7 +3952,7 @@ show_stmt ::= {: RESULT = new ShowStoragePolicyUsingStmt(null); :} - | KW_SHOW KW_STORAGE KW_POLICY KW_USING KW_FOR ident:policy + | KW_SHOW KW_STORAGE KW_POLICY KW_USING KW_FOR ident_or_text:policy {: RESULT = new ShowStoragePolicyUsingStmt(policy); :} 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 12166bfe4b7e35..6ba14954bad7a4 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 @@ -76,6 +76,7 @@ import org.apache.doris.thrift.TTabletType; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -177,7 +178,19 @@ private boolean processAlterOlapTable(AlterTableStmt stmt, OlapTable olapTable, } // check currentStoragePolicy resource exist. Env.getCurrentEnv().getPolicyMgr().checkStoragePolicyExist(currentStoragePolicy); - + boolean enableUniqueKeyMergeOnWrite; + olapTable.readLock(); + try { + enableUniqueKeyMergeOnWrite = olapTable.getEnableUniqueKeyMergeOnWrite(); + } finally { + olapTable.readUnlock(); + } + // must check here whether you can set the policy, otherwise there will be inconsistent metadata + if (enableUniqueKeyMergeOnWrite && !Strings.isNullOrEmpty(currentStoragePolicy)) { + throw new UserException( + "Can not set UNIQUE KEY table that enables Merge-On-write" + + " with storage policy(" + currentStoragePolicy + ")"); + } olapTable.setStoragePolicy(currentStoragePolicy); needProcessOutsideTableLock = true; } else if (currentAlterOps.checkIsBeingSynced(alterClauses)) { 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 633028ddaee976..8caedae0699074 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 @@ -2215,11 +2215,6 @@ public void updateTableProperties(Database db, String tableName, Map !p.isEmpty()) .ifPresent(p -> olapTable.getPartitionInfo().setStoragePolicy(partition.getId(), p)); + Optional.ofNullable(tableProperty.getStoragePolicy()).filter(p -> !p.isEmpty()) + .ifPresent(p -> olapTable.getPartitionInfo().getDataProperty(partition.getId()) + .setStoragePolicy(p)); } break; case OperationType.OP_UPDATE_BINLOG_CONFIG: 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 bb9cbe0d717e18..5dbc2283e1784f 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 @@ -27,6 +27,7 @@ import org.apache.doris.analysis.DateLiteral; import org.apache.doris.analysis.DropResourceStmt; import org.apache.doris.analysis.ShowCreateMaterializedViewStmt; +import org.apache.doris.analysis.ShowCreateTableStmt; import org.apache.doris.catalog.ColocateGroupSchema; import org.apache.doris.catalog.ColocateTableIndex.GroupId; import org.apache.doris.catalog.Column; @@ -244,6 +245,10 @@ public static void beforeClass() throws Exception { createTable("create table test.unique_sequence_col (k1 int, v1 int, v2 date) ENGINE=OLAP " + " UNIQUE KEY(`k1`) DISTRIBUTED BY HASH(`k1`) BUCKETS 1" + " PROPERTIES (\"replication_num\" = \"1\", \"function_column.sequence_col\" = \"v1\");"); + + createTable("CREATE TABLE test.tbl_storage(k1 int) ENGINE=OLAP UNIQUE KEY (k1)\n" + + "DISTRIBUTED BY HASH(k1) BUCKETS 3\n" + + "PROPERTIES('replication_num' = '1','enable_unique_key_merge_on_write' = 'true');"); } @AfterClass @@ -1433,4 +1438,17 @@ public void testModifySequenceCol() { String stmt = "alter table test.unique_sequence_col modify column v1 Date"; alterTable(stmt, true); } + + @Test + public void testModifyTableForStoragePolicy() throws Exception { + String sql = "ALTER TABLE test.tbl_storage SET ('storage_policy' = 'testPolicy')"; + alterTableWithExceptionMsg(sql, "errCode = 2, detailMessage = Can not set UNIQUE KEY table that enables " + + "Merge-On-write with storage policy(testPolicy)"); + String showSQl = "show create table test.tbl_storage"; + ShowCreateTableStmt showStmt = (ShowCreateTableStmt) UtFrameUtils.parseAndAnalyzeStmt(showSQl, connectContext); + ShowExecutor executor = new ShowExecutor(connectContext, showStmt); + List> resultRows = executor.execute().getResultRows(); + String createSql = resultRows.get(0).get(1); + Assert.assertFalse(createSql.contains("storage_policy")); + } }