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 3e113ded1604ab998205fad5aa3847e0e33631e4 Author: Xujian Duan <[email protected]> AuthorDate: Thu Mar 31 13:51:13 2022 +0800 [fix](sql-block-rule)Fix sql block rule bug (#8738) 1. Check properties' effectiveness of sql_block_rule, can't set limitations of sql_block_rule to be negative. 2. Optimize the judgment conditions when a query hits a sql_block_rule 3. Check if sql_block_rule has already exist when exec set property for "user" "sql_block_rule" = "xxx" 4. Add UT ad SqlBlockRuleMgrTest.java --- .../apache/doris/blockrule/SqlBlockRuleMgr.java | 21 ++++++++++-- .../apache/doris/mysql/privilege/UserProperty.java | 7 ++++ .../doris/blockrule/SqlBlockRuleMgrTest.java | 40 +++++++++++++++++++++- 3 files changed, 64 insertions(+), 4 deletions(-) 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 346cb95..ba16da9 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 @@ -79,6 +79,19 @@ public class SqlBlockRuleMgr implements Writable { return Lists.newArrayList(nameToSqlBlockRuleMap.values()); } + // check limitation's effectiveness of a sql_block_rule + public static void verifyLimitations(SqlBlockRule sqlBlockRule) throws DdlException { + if (sqlBlockRule.getPartitionNum() < 0){ + throw new DdlException("the value of partition_num can't be a negative"); + } + if (sqlBlockRule.getTabletNum() < 0){ + throw new DdlException("the value of tablet_num can't be a negative"); + } + if (sqlBlockRule.getCardinality() < 0){ + throw new DdlException("the value of cardinality can't be a negative"); + } + } + public void createSqlBlockRule(CreateSqlBlockRuleStmt stmt) throws UserException { writeLock(); try { @@ -87,6 +100,7 @@ public class SqlBlockRuleMgr implements Writable { if (existRule(ruleName)) { throw new DdlException("the sql block rule " + ruleName + " already create"); } + verifyLimitations(sqlBlockRule); unprotectedAdd(sqlBlockRule); Catalog.getCurrentCatalog().getEditLog().logCreateSqlBlockRule(sqlBlockRule); } finally { @@ -107,6 +121,7 @@ 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())) { @@ -231,11 +246,11 @@ public class SqlBlockRuleMgr implements Writable { || (rule.getTabletNum() != 0 && rule.getTabletNum() < tabletNum) || (rule.getCardinality() != 0 && rule.getCardinality() < cardinality)) { MetricRepo.COUNTER_HIT_SQL_BLOCK_RULE.increase(1L); - if (rule.getPartitionNum() < partitionNum) { + if (rule.getPartitionNum() < partitionNum && rule.getPartitionNum() != 0) { throw new AnalysisException("sql hits sql block rule: " + rule.getName() + ", reach partition_num : " + rule.getPartitionNum()); - } else if (rule.getTabletNum() < tabletNum) { + } else if (rule.getTabletNum() < tabletNum && rule.getTabletNum() != 0) { throw new AnalysisException("sql hits sql block rule: " + rule.getName() + ", reach tablet_num : " + rule.getTabletNum()); - } else if (rule.getCardinality() < cardinality) { + } else if (rule.getCardinality() < cardinality && rule.getCardinality() != 0) { throw new AnalysisException("sql hits sql block rule: " + rule.getName() + ", reach cardinality : " + rule.getCardinality()); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserProperty.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserProperty.java index 547257f..4feb3a3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserProperty.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserProperty.java @@ -274,6 +274,13 @@ public class UserProperty implements Writable { if (keyArr.length != 1) { throw new DdlException(PROP_SQL_BLOCK_RULES + " format error"); } + + // check if sql_block_rule has already exist + for (String ruleName : value.replaceAll(" ","").split(",")){ + if (!ruleName.equals("") && !Catalog.getCurrentCatalog().getSqlBlockRuleMgr().existRule(ruleName)){ + throw new DdlException("the sql block rule " + ruleName + " not exist"); + } + } sqlBlockRules = value; } else if (keyArr[0].equalsIgnoreCase(PROP_CPU_RESOURCE_LIMIT)) { // set property "cpu_resource_limit" = "2"; 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 7127f9f..4005e3b 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 @@ -109,7 +109,7 @@ public class SqlBlockRuleMgrTest { String sql = "select * from table1 limit 10"; String sqlHash = DigestUtils.md5Hex(sql); SqlBlockRule sqlRule = new SqlBlockRule("test_rule1", null, sqlHash, 0L, 0L, 0L, false, true); - SqlBlockRuleMgr mgr = new SqlBlockRuleMgr(); + SqlBlockRuleMgr mgr = Catalog.getCurrentCatalog().getSqlBlockRuleMgr(); mgr.replayCreate(sqlRule); // sql block rules String setPropertyStr = "set property for \"root\" \"sql_block_rules\" = \"test_rule1\""; @@ -287,4 +287,42 @@ public class SqlBlockRuleMgrTest { Assert.assertEquals(1, mgr.getSqlBlockRule(showStmt).size()); Assert.assertEquals("select \\* from test_table", mgr.getSqlBlockRule(showStmt).get(0).getSql()); } + + @Test + public void testLimitationsInvalid() throws Exception { + SqlBlockRuleMgr mgr = new SqlBlockRuleMgr(); + + // create sql_block_rule with partition_num = -1 + // DdlException: the value of partition_num can't be a negative + String createSql = "CREATE SQL_BLOCK_RULE test_rule PROPERTIES(\"partition_num\"=\"-1\",\"enable\"=\"true\")"; + CreateSqlBlockRuleStmt stmt = (CreateSqlBlockRuleStmt) UtFrameUtils.parseAndAnalyzeStmt(createSql, connectContext); + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "the value of partition_num can't be a negative", + () -> mgr.createSqlBlockRule(stmt)); + + // create sql_block_rule with tablet_num = -1 + // DdlException: the value of tablet_num can't be a negative + String createSql1 = "CREATE SQL_BLOCK_RULE test_rule PROPERTIES(\"tablet_num\"=\"-1\",\"enable\"=\"true\")"; + CreateSqlBlockRuleStmt stmt1 = (CreateSqlBlockRuleStmt) UtFrameUtils.parseAndAnalyzeStmt(createSql1, connectContext); + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "the value of tablet_num can't be a negative", + () -> mgr.createSqlBlockRule(stmt1)); + + // create sql_block_rule with cardinality = -1 + // DdlException: the value of cardinality can't be a negative + String createSql2 = "CREATE SQL_BLOCK_RULE test_rule PROPERTIES(\"cardinality\"=\"-1\",\"enable\"=\"true\")"; + CreateSqlBlockRuleStmt stmt2 = (CreateSqlBlockRuleStmt) UtFrameUtils.parseAndAnalyzeStmt(createSql2, connectContext); + ExceptionChecker.expectThrowsWithMsg(DdlException.class, "the value of cardinality can't be a negative", + () -> mgr.createSqlBlockRule(stmt2)); + } + + @Test + public void testUserPropertyInvalid() throws Exception { + // sql block rules + String ruleName = "test_rule_name"; + String setPropertyStr = String.format("set property for \"root\" \"sql_block_rules\" = \"%s\"", ruleName); + SetUserPropertyStmt setUserPropertyStmt = (SetUserPropertyStmt) UtFrameUtils.parseAndAnalyzeStmt(setPropertyStr, connectContext); + + ExceptionChecker.expectThrowsWithMsg(DdlException.class, String.format("the sql block rule %s not exist", ruleName), + () -> Catalog.getCurrentCatalog().getAuth().updateUserProperty(setUserPropertyStmt)); + + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
