From 8c2713b2e5af32404b13448782ccd606da58bb97 Mon Sep 17 00:00:00 2001 From: camby <104178625@qq.com> Date: Fri, 20 Jun 2025 12:18:36 +0800 Subject: [PATCH] [fix](alter) alter partition without storage_policy property will also cancel storage_policy (#51662) (#51910) ### What problem does this PR solve? pick #51662 to branch-2.1 ### Release note None ### Check List (For Author) - Test - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason - Behavior changed: - [ ] No. - [ ] Yes. - Does this need documentation? - [ ] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label --- .../src/main/java/org/apache/doris/alter/Alter.java | 4 +++- .../apache/doris/common/util/PropertyAnalyzer.java | 7 +++++++ .../test_show_storage_policy_using.groovy | 11 +++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) 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 f8854b6862..3b5c64b7ff 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 @@ -860,7 +860,9 @@ public class Alter { // check currentStoragePolicy resource exist. Env.getCurrentEnv().getPolicyMgr().checkStoragePolicyExist(currentStoragePolicy); partitionInfo.setStoragePolicy(partition.getId(), currentStoragePolicy); - } else { + } else if (PropertyAnalyzer.hasStoragePolicy(properties)) { + // only set "storage_policy" = "", means cancel storage policy + // if current partition is already in remote storage if (partition.getRemoteDataSize() > 0) { throw new AnalysisException( "Cannot cancel storage policy for partition which is already on code storage."); diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java index 81ab5f8d20..a6e4bfcff2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java @@ -1025,6 +1025,13 @@ public class PropertyAnalyzer { return storagePolicy; } + public static boolean hasStoragePolicy(Map properties) { + if (properties != null && properties.containsKey(PROPERTIES_STORAGE_POLICY)) { + return true; + } + return false; + } + // analyze property like : "type" = "xxx"; public static String analyzeType(Map properties) throws AnalysisException { String type = null; diff --git a/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy b/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy index fcf8485eaf..b3ea5c7c8d 100644 --- a/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy +++ b/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy @@ -156,6 +156,17 @@ suite("test_show_storage_policy_using") { """ assertTrue(show_result.size() >= 4) + // alter other property, will not cancel storage_policy + sql """ ALTER STORAGE POLICY ${policy_name} PROPERTIES("cooldown_ttl" = "1"); """ + sql """ + ALTER TABLE partition_with_multiple_storage_policy MODIFY PARTITION (`p201701`) SET ("replication_num"="1") + """ + show_result = sql """ + show storage policy using for ${policy_name} + """ + assertEquals(show_result.size(), 2) + sql """ ALTER STORAGE POLICY ${policy_name} PROPERTIES("cooldown_ttl" = "300"); """ + // test cancel a partition's storage policy sql """ ALTER TABLE partition_with_multiple_storage_policy MODIFY PARTITION (`p201701`) SET ("storage_policy"="")