This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 3d210d0d85ae6509d726b678c4fb28dcbe636a2c
Author: shee <13843187+qz...@users.noreply.github.com>
AuthorDate: Wed Aug 21 09:30:18 2024 +0800
[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
<!--Describe your changes.-->
---------
Co-authored-by: garenshi <garen...@tencent.com>
---
fe/fe-core/src/main/cup/sql_parser.cup | 2 +-
.../src/main/java/org/apache/doris/alter/Alter.java | 15 ++++++++++++++-
.../org/apache/doris/alter/SchemaChangeHandler.java | 5 -----
.../src/main/java/org/apache/doris/catalog/Env.java | 3 +++
.../test/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 42577629ba5..dda8ccb1862 100644
--- a/fe/fe-core/src/main/cup/sql_parser.cup
+++ b/fe/fe-core/src/main/cup/sql_parser.cup
@@ -4178,7 +4178,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 f1879798a93..95ad5ae824b 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
@@ -80,6 +80,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;
@@ -191,7 +192,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 1a4900a3fd3..3fbcd3f629b 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
@@ -2318,11 +2318,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 760306478c6..38483e03ab5 100644
--- 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
@@ -5327,6 +5327,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 35e8b6b91e6..4c6a6796bfb 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<List<String>> resultRows = executor.execute().getResultRows();
+ String createSql = resultRows.get(0).get(1);
+ Assert.assertFalse(createSql.contains("storage_policy"));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
