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 346cb951b7..ba16da9cf0 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 547257f952..4feb3a3dbd 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 7127f9fc75..4005e3b71b 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)); + + } }