You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2022/04/18 11:57:41 UTC

[incubator-doris] 02/17: [fix](sql_block_rule) optimization of alter sql_block_rule stmt (#8971)

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch dev-1.0.1
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git

commit 7019819151c33411206e987ad7938af9c1a2b38a
Author: Xujian Duan <50...@users.noreply.github.com>
AuthorDate: Sat Apr 16 11:05:31 2022 +0800

    [fix](sql_block_rule) optimization of alter sql_block_rule stmt (#8971)
    
    Optimization of alter sql_block_rule stmt.
---
 .../doris/analysis/AlterSqlBlockRuleStmt.java      |  8 ++--
 .../apache/doris/blockrule/SqlBlockRuleMgr.java    | 16 ++++---
 .../org/apache/doris/common/util/SqlBlockUtil.java | 17 +++----
 .../doris/blockrule/SqlBlockRuleMgrTest.java       | 54 ++++++++++++++++++++++
 4 files changed, 77 insertions(+), 18 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterSqlBlockRuleStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterSqlBlockRuleStmt.java
index dac0932447..cc2a13c228 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterSqlBlockRuleStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterSqlBlockRuleStmt.java
@@ -34,6 +34,8 @@ import java.util.Map;
 
 public class AlterSqlBlockRuleStmt extends DdlStmt {
 
+    public static final Long LONG_NOT_SET = SqlBlockUtil.LONG_MINUS_ONE;
+
     private final String ruleName;
 
     private String sql;
@@ -78,9 +80,9 @@ public class AlterSqlBlockRuleStmt extends DdlStmt {
 
         SqlBlockUtil.checkSqlAndSqlHashSetBoth(sql, sqlHash);
         SqlBlockUtil.checkSqlAndLimitationsSetBoth(sql, sqlHash, partitionNumString, tabletNumString, cardinalityString);
-        this.partitionNum = Util.getLongPropertyOrDefault(partitionNumString, 0L, null, CreateSqlBlockRuleStmt.SCANNED_PARTITION_NUM + " should be a long");
-        this.tabletNum = Util.getLongPropertyOrDefault(tabletNumString, 0L, null, CreateSqlBlockRuleStmt.SCANNED_TABLET_NUM + " should be a long");
-        this.cardinality = Util.getLongPropertyOrDefault(cardinalityString, 0L, null, CreateSqlBlockRuleStmt.SCANNED_CARDINALITY + " should be a long");
+        this.partitionNum = Util.getLongPropertyOrDefault(partitionNumString, LONG_NOT_SET, null, CreateSqlBlockRuleStmt.SCANNED_PARTITION_NUM + " should be a long");
+        this.tabletNum = Util.getLongPropertyOrDefault(tabletNumString, LONG_NOT_SET, null, CreateSqlBlockRuleStmt.SCANNED_TABLET_NUM + " should be a long");
+        this.cardinality = Util.getLongPropertyOrDefault(cardinalityString, LONG_NOT_SET, null, CreateSqlBlockRuleStmt.SCANNED_CARDINALITY + " should be a long");
         // allow null, represents no modification
         String globalStr = properties.get(CreateSqlBlockRuleStmt.GLOBAL_PROPERTY);
         this.global = StringUtils.isNotEmpty(globalStr) ? Boolean.parseBoolean(globalStr) : null;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/blockrule/SqlBlockRuleMgr.java b/fe/fe-core/src/main/java/org/apache/doris/blockrule/SqlBlockRuleMgr.java
index ba16da9cf0..c1ddf633df 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/blockrule/SqlBlockRuleMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/blockrule/SqlBlockRuleMgr.java
@@ -121,22 +121,21 @@ public class SqlBlockRuleMgr implements Writable {
             if (!existRule(ruleName)) {
                 throw new DdlException("the sql block rule " + ruleName + " not exist");
             }
-            verifyLimitations(sqlBlockRule);
             SqlBlockRule originRule = nameToSqlBlockRuleMap.get(ruleName);
-            SqlBlockUtil.checkAlterValidate(sqlBlockRule, originRule);
-            if (StringUtils.isEmpty(sqlBlockRule.getSql())) {
+
+            if (sqlBlockRule.getSql().equals(CreateSqlBlockRuleStmt.STRING_NOT_SET)) {
                 sqlBlockRule.setSql(originRule.getSql());
             }
-            if (StringUtils.isEmpty(sqlBlockRule.getSqlHash())) {
+            if (sqlBlockRule.getSqlHash().equals(CreateSqlBlockRuleStmt.STRING_NOT_SET)) {
                 sqlBlockRule.setSqlHash(originRule.getSqlHash());
             }
-            if (StringUtils.isEmpty(sqlBlockRule.getPartitionNum().toString())) {
+            if (sqlBlockRule.getPartitionNum().equals(AlterSqlBlockRuleStmt.LONG_NOT_SET)) {
                 sqlBlockRule.setPartitionNum(originRule.getPartitionNum());
             }
-            if (StringUtils.isEmpty(sqlBlockRule.getTabletNum().toString())) {
+            if (sqlBlockRule.getTabletNum().equals(AlterSqlBlockRuleStmt.LONG_NOT_SET)) {
                 sqlBlockRule.setTabletNum(originRule.getTabletNum());
             }
-            if (StringUtils.isEmpty(sqlBlockRule.getCardinality().toString())) {
+            if (sqlBlockRule.getCardinality().equals(AlterSqlBlockRuleStmt.LONG_NOT_SET)) {
                 sqlBlockRule.setCardinality(originRule.getCardinality());
             }
             if (sqlBlockRule.getGlobal() == null) {
@@ -145,6 +144,9 @@ public class SqlBlockRuleMgr implements Writable {
             if (sqlBlockRule.getEnable() == null) {
                 sqlBlockRule.setEnable(originRule.getEnable());
             }
+            verifyLimitations(sqlBlockRule);
+            SqlBlockUtil.checkAlterValidate(sqlBlockRule);
+
             unprotectedUpdate(sqlBlockRule);
             Catalog.getCurrentCatalog().getEditLog().logAlterSqlBlockRule(sqlBlockRule);
         } finally {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/SqlBlockUtil.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/SqlBlockUtil.java
index c73b4bc45a..a798d03a38 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/common/util/SqlBlockUtil.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/SqlBlockUtil.java
@@ -30,6 +30,7 @@ public class SqlBlockUtil {
     public static final String STRING_DEFAULT = "NULL";
     public static final String LONG_DEFAULT = "0";
     public static final Long LONG_ZERO = 0L;
+    public static final Long LONG_MINUS_ONE = -1L;
 
 
     public static void checkSqlAndSqlHashSetBoth(String sql, String sqlHash) throws AnalysisException{
@@ -73,23 +74,23 @@ public class SqlBlockUtil {
     }
 
     // alter operation not allowed to change other properties that not set
-    public static void checkAlterValidate(SqlBlockRule sqlBlockRule, SqlBlockRule originRule) throws AnalysisException {
+    public static void checkAlterValidate(SqlBlockRule sqlBlockRule) throws AnalysisException {
         if (!STRING_DEFAULT.equals(sqlBlockRule.getSql())) {
-            if (!STRING_DEFAULT.equals(originRule.getSqlHash()) && StringUtils.isNotEmpty(originRule.getSqlHash())) {
+            if (!STRING_DEFAULT.equals(sqlBlockRule.getSqlHash()) && StringUtils.isNotEmpty(sqlBlockRule.getSqlHash())) {
                 throw new AnalysisException("Only sql or sqlHash can be configured");
-            } else if (!isSqlBlockLimitationsDefault(originRule.getPartitionNum(), originRule.getTabletNum(), originRule.getCardinality())
-                    &&!isSqlBlockLimitationsNull(originRule.getPartitionNum(), originRule.getTabletNum(), originRule.getCardinality())) {
+            } else if (!isSqlBlockLimitationsDefault(sqlBlockRule.getPartitionNum(), sqlBlockRule.getTabletNum(), sqlBlockRule.getCardinality())
+                    &&!isSqlBlockLimitationsNull(sqlBlockRule.getPartitionNum(), sqlBlockRule.getTabletNum(), sqlBlockRule.getCardinality())) {
                 ErrorReport.reportAnalysisException(ErrorCode.ERROR_SQL_AND_LIMITATIONS_SET_IN_ONE_RULE);
             }
         } else if (!STRING_DEFAULT.equals(sqlBlockRule.getSqlHash())) {
-            if (!STRING_DEFAULT.equals(originRule.getSql()) && StringUtils.isNotEmpty(originRule.getSql())) {
+            if (!STRING_DEFAULT.equals(sqlBlockRule.getSql()) && StringUtils.isNotEmpty(sqlBlockRule.getSql())) {
                 throw new AnalysisException("Only sql or sqlHash can be configured");
-            } else if (!isSqlBlockLimitationsDefault(originRule.getPartitionNum(), originRule.getTabletNum(), originRule.getCardinality())
-                    && !isSqlBlockLimitationsNull(originRule.getPartitionNum(), originRule.getTabletNum(), originRule.getCardinality())) {
+            } else if (!isSqlBlockLimitationsDefault(sqlBlockRule.getPartitionNum(), sqlBlockRule.getTabletNum(), sqlBlockRule.getCardinality())
+                    && !isSqlBlockLimitationsNull(sqlBlockRule.getPartitionNum(), sqlBlockRule.getTabletNum(), sqlBlockRule.getCardinality())) {
                 ErrorReport.reportAnalysisException(ErrorCode.ERROR_SQL_AND_LIMITATIONS_SET_IN_ONE_RULE);
             }
         } else if (!isSqlBlockLimitationsDefault(sqlBlockRule.getPartitionNum(), sqlBlockRule.getTabletNum(), sqlBlockRule.getCardinality())) {
-            if (!STRING_DEFAULT.equals(originRule.getSql()) || !STRING_DEFAULT.equals(originRule.getSqlHash())) {
+            if (!STRING_DEFAULT.equals(sqlBlockRule.getSql()) || !STRING_DEFAULT.equals(sqlBlockRule.getSqlHash())) {
                 ErrorReport.reportAnalysisException(ErrorCode.ERROR_SQL_AND_LIMITATIONS_SET_IN_ONE_RULE);
             }
         }
diff --git a/fe/fe-core/src/test/java/org/apache/doris/blockrule/SqlBlockRuleMgrTest.java b/fe/fe-core/src/test/java/org/apache/doris/blockrule/SqlBlockRuleMgrTest.java
index 4005e3b71b..bf93eae895 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/blockrule/SqlBlockRuleMgrTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/blockrule/SqlBlockRuleMgrTest.java
@@ -325,4 +325,58 @@ public class SqlBlockRuleMgrTest {
                 () -> Catalog.getCurrentCatalog().getAuth().updateUserProperty(setUserPropertyStmt));
 
     }
+
+    @Test
+    public void testAlterSqlBlock() throws Exception{
+        Analyzer analyzer = new Analyzer(Catalog.getCurrentCatalog(), connectContext);
+        SqlBlockRuleMgr mgr = Catalog.getCurrentCatalog().getSqlBlockRuleMgr();
+
+        // create : sql
+        // alter : global
+        SqlBlockRule sqlBlockRule = new SqlBlockRule("test_rule", "select \\* from test_table", "NULL", 0L, 0L, 0L, true, true);
+        mgr.unprotectedAdd(sqlBlockRule);
+        Assert.assertEquals(true, mgr.existRule("test_rule"));
+
+        Map<String, String> properties = new HashMap<>();
+        properties.put(CreateSqlBlockRuleStmt.GLOBAL_PROPERTY, "false");
+        AlterSqlBlockRuleStmt stmt = new AlterSqlBlockRuleStmt("test_rule", properties);
+        stmt.analyze(analyzer);
+        mgr.alterSqlBlockRule(stmt);
+
+        ShowSqlBlockRuleStmt showStmt = new ShowSqlBlockRuleStmt("test_rule");
+        SqlBlockRule alteredSqlBlockRule = mgr.getSqlBlockRule(showStmt).get(0);
+
+        Assert.assertEquals("select \\* from test_table", alteredSqlBlockRule.getSql());
+        Assert.assertEquals("NULL", alteredSqlBlockRule.getSqlHash());
+        Assert.assertEquals(0L, (long)alteredSqlBlockRule.getPartitionNum());
+        Assert.assertEquals(0L, (long)alteredSqlBlockRule.getTabletNum());
+        Assert.assertEquals(0L, (long)alteredSqlBlockRule.getCardinality());
+        Assert.assertEquals(false, alteredSqlBlockRule.getGlobal());
+        Assert.assertEquals(true, alteredSqlBlockRule.getEnable());
+
+        // create : partitionNum
+        // alter : tabletNum
+        SqlBlockRule sqlBlockRule2 = new SqlBlockRule("test_rule2", "NULL", "NULL", 100L, 0L, 0L, true, true);
+        mgr.unprotectedAdd(sqlBlockRule2);
+        Assert.assertEquals(true, mgr.existRule("test_rule2"));
+
+        Map<String, String> properties2 = new HashMap<>();
+        properties2.put(CreateSqlBlockRuleStmt.SCANNED_TABLET_NUM, "500");
+        AlterSqlBlockRuleStmt stmt2 = new AlterSqlBlockRuleStmt("test_rule2", properties2);
+        stmt2.analyze(analyzer);
+        mgr.alterSqlBlockRule(stmt2);
+
+        ShowSqlBlockRuleStmt showStmt2 = new ShowSqlBlockRuleStmt("test_rule2");
+        SqlBlockRule alteredSqlBlockRule2 = mgr.getSqlBlockRule(showStmt2).get(0);
+
+        Assert.assertEquals("NULL", alteredSqlBlockRule2.getSql());
+        Assert.assertEquals("NULL", alteredSqlBlockRule2.getSqlHash());
+        Assert.assertEquals(100L, (long)alteredSqlBlockRule2.getPartitionNum());
+        Assert.assertEquals(500L, (long)alteredSqlBlockRule2.getTabletNum());
+        Assert.assertEquals(0L, (long)alteredSqlBlockRule2.getCardinality());
+        Assert.assertEquals(true, alteredSqlBlockRule2.getGlobal());
+        Assert.assertEquals(true, alteredSqlBlockRule2.getEnable());
+
+
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org