From 76b7a5291aabc1399bc43735579de532b5c36be8 Mon Sep 17 00:00:00 2001 From: shee <13843187+qzsee@users.noreply.github.com> Date: Thu, 22 Aug 2024 00:23:19 +0800 Subject: [PATCH] [BUG] fix partition storage policy info lost (#38700) (#39677) ## Proposed changes cherry-pick from #38700 Issue Number: close #xxx --------- ## Proposed changes 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 a0636a7650..1cbd28ee43 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 12166bfe4b..6ba14954ba 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.TSortType; 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 @@ public class Alter { } // 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 633028ddae..8caedae069 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 class SchemaChangeHandler extends AlterHandler { } } String storagePolicy = properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY); - if (enableUniqueKeyMergeOnWrite && !Strings.isNullOrEmpty(storagePolicy)) { - throw new UserException( - "Can not set UNIQUE KEY table that enables Merge-On-write" + " with storage policy(" + storagePolicy - + ")"); - } long storagePolicyId = storagePolicyNameToId(storagePolicy); String compactionPolicy = properties.get(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java index 73e1218bd7..785b7add3a 100755 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java @@ -5144,6 +5144,9 @@ public class Env { // storage policy re-use modify in memory Optional.ofNullable(tableProperty.getStoragePolicy()).filter(p -> !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 bb9cbe0d71..5dbc2283e1 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.CreateTableStmt; 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 class AlterTest { 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 class AlterTest { 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")); + } }