From 7fefac990b5aadacba90e680c96ff6befbfdf486 Mon Sep 17 00:00:00 2001 From: seawinde Date: Mon, 21 Oct 2024 14:19:46 +0800 Subject: [PATCH 1/3] [test](mtmv) Auto calc the value of sync_cbo_rewrite variable to make sure the same test code in different version --- .../mv/MaterializationContext.java | 3 +- .../doris/regression/suite/Suite.groovy | 67 +++++++++++++------ 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java index 609125280ded4b..df535d59d87399 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java @@ -35,7 +35,6 @@ import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.algebra.Relation; import org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel; -import org.apache.doris.nereids.trees.plans.physical.PhysicalPlan; import org.apache.doris.nereids.trees.plans.physical.PhysicalRelation; import org.apache.doris.nereids.trees.plans.visitor.DefaultPlanVisitor; import org.apache.doris.statistics.ColumnStatistic; @@ -348,7 +347,7 @@ public String toString() { * ToSummaryString, this contains only summary info. */ public static String toSummaryString(List materializationContexts, - PhysicalPlan physicalPlan) { + Plan physicalPlan) { if (materializationContexts.isEmpty()) { return ""; } diff --git a/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy b/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy index 4aab3f774d76fe..9591489b7103b2 100644 --- a/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy +++ b/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy @@ -1684,6 +1684,17 @@ class Suite implements GroovyInterceptable { return result.values().toList() } + // enable_sync_mv_cost_based_rewrite is true or not + boolean enable_sync_mv_cost_based_rewrite () { + def showVariable = "show variables like 'enable_sync_mv_cost_based_rewrite';" + List> result = sql(showVariable) + logger.info("enable_sync_mv_cost_based_rewrite = " + result) + if (result.isEmpty()) { + return false; + } + return Boolean.parseBoolean(result.get(0).get(1)); + } + // Given tables to decide whether the table partition row count statistic is ready or not boolean is_partition_statistics_ready(db, tables) { boolean isReady = true; @@ -1763,12 +1774,14 @@ class Suite implements GroovyInterceptable { // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite // is_partition_statistics_ready is the bool value which identifying if partition row count is valid or not // if true, check if chosen by cbo or doesn't check - def mv_rewrite_success = { query_sql, mv_name, sync_cbo_rewrite = true, is_partition_statistics_ready = true -> + void mv_rewrite_success(query_sql, mv_name, sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite(), + is_partition_statistics_ready = true) { logger.info("query_sql = " + query_sql + ", mv_name = " + mv_name + ", sync_cbo_rewrite = " +sync_cbo_rewrite + ", is_partition_statistics_ready = " + is_partition_statistics_ready) if (!is_partition_statistics_ready) { // If partition statistics is no ready, degrade to without check cbo chosen - return mv_rewrite_success_without_check_chosen(query_sql, mv_name, sync_cbo_rewrite) + mv_rewrite_success_without_check_chosen(query_sql, mv_name, sync_cbo_rewrite) + return } if (!sync_cbo_rewrite) { explain { @@ -1787,12 +1800,14 @@ class Suite implements GroovyInterceptable { // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite // is_partition_statistics_ready is the bool value which identifying if partition row count is valid or not // if true, check if chosen by cbo or doesn't check - def mv_rewrite_all_success = { query_sql, mv_names, sync_cbo_rewrite = true, is_partition_statistics_ready = true -> + void mv_rewrite_all_success( query_sql, mv_names, sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite(), + is_partition_statistics_ready = true) { logger.info("query_sql = " + query_sql + ", mv_names = " + mv_names + ", sync_cbo_rewrite = " +sync_cbo_rewrite + ", is_partition_statistics_ready = " + is_partition_statistics_ready) if (!is_partition_statistics_ready) { // If partition statistics is no ready, degrade to without check cbo chosen - return mv_rewrite_all_success_without_check_chosen(query_sql, mv_names, sync_cbo_rewrite) + mv_rewrite_all_success_without_check_chosen(query_sql, mv_names, sync_cbo_rewrite) + return } if (!sync_cbo_rewrite) { explain { @@ -1823,12 +1838,14 @@ class Suite implements GroovyInterceptable { // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite // is_partition_statistics_ready is the bool value which identifying if partition row count is valid or not // if true, check if chosen by cbo or doesn't check - def mv_rewrite_any_success = { query_sql, mv_names, sync_cbo_rewrite = true, is_partition_statistics_ready = true -> + void mv_rewrite_any_success(query_sql, mv_names, sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite(), + is_partition_statistics_ready = true) { logger.info("query_sql = " + query_sql + ", mv_names = " + mv_names + ", sync_cbo_rewrite = " +sync_cbo_rewrite + ", is_partition_statistics_ready = " + is_partition_statistics_ready) if (!is_partition_statistics_ready) { // If partition statistics is no ready, degrade to without check cbo chosen - return mv_rewrite_any_success_without_check_chosen(query_sql, mv_names, sync_cbo_rewrite) + mv_rewrite_any_success_without_check_chosen(query_sql, mv_names, sync_cbo_rewrite) + return } if (!sync_cbo_rewrite) { explain { @@ -1857,7 +1874,8 @@ class Suite implements GroovyInterceptable { // multi mv part in rewrite process, all rewrte success without check if chosen by cbo // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite - def mv_rewrite_all_success_without_check_chosen = { query_sql, mv_names, sync_cbo_rewrite = true -> + void mv_rewrite_all_success_without_check_chosen(query_sql, mv_names, + sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite()){ logger.info("query_sql = " + query_sql + ", mv_names = " + mv_names) if (!sync_cbo_rewrite) { explain { @@ -1865,7 +1883,9 @@ class Suite implements GroovyInterceptable { check { result -> boolean success = true; for (String mv_name : mv_names) { - success = success && result.contains("(${mv_name})") + def splitResult = result.split("MaterializedViewRewriteFail") + def each_result = splitResult.length == 2 ? splitResult[0].contains(mv_name) : false + success = success && (result.contains"(${mv_name})" || each_result) } Assert.assertEquals(true, success) } @@ -1887,7 +1907,8 @@ class Suite implements GroovyInterceptable { // multi mv part in rewrite process, any of them rewrte success without check if chosen by cbo or not // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite - def mv_rewrite_any_success_without_check_chosen = { query_sql, mv_names, sync_cbo_rewrite = true -> + void mv_rewrite_any_success_without_check_chosen(query_sql, mv_names, + sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite()) { logger.info("query_sql = " + query_sql + ", mv_names = " + mv_names) if (!sync_cbo_rewrite) { explain { @@ -1895,7 +1916,9 @@ class Suite implements GroovyInterceptable { check { result -> boolean success = false; for (String mv_name : mv_names) { - success = success || result.contains("(${mv_name})") + def splitResult = result.split("MaterializedViewRewriteFail") + def each_result = splitResult.length == 2 ? splitResult[0].contains(mv_name) : false + success = success || (result.contains"(${mv_name})" || each_result) } Assert.assertEquals(true, success) } @@ -1904,7 +1927,7 @@ class Suite implements GroovyInterceptable { } explain { sql(" memo plan ${query_sql}") - check {result -> + check { result -> boolean success = false for (String mv_name : mv_names) { success = success || result.contains("${mv_name} chose") || result.contains("${mv_name} not chose") @@ -1916,12 +1939,16 @@ class Suite implements GroovyInterceptable { // multi mv part in rewrite process, rewrte success without check if chosen by cbo or not // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite - def mv_rewrite_success_without_check_chosen = { query_sql, mv_name, sync_cbo_rewrite = true -> + void mv_rewrite_success_without_check_chosen(query_sql, mv_name, + sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite()) { logger.info("query_sql = " + query_sql + ", mv_name = " + mv_name) if (!sync_cbo_rewrite) { explain { sql("${query_sql}") - contains("(${mv_name})") + check { result -> + def splitResult = result.split("MaterializedViewRewriteFail") + result.contains"(${mv_name})" || (splitResult.length == 2 ? splitResult[0].contains(mv_name) : false) + } } return } @@ -1935,12 +1962,12 @@ class Suite implements GroovyInterceptable { // single mv part in rewrite process, rewrte fail // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite - def mv_rewrite_fail = { query_sql, mv_name, sync_cbo_rewrite = true -> + void mv_rewrite_fail(query_sql, mv_name, sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite()) { logger.info("query_sql = " + query_sql + ", mv_name = " + mv_name) if (!sync_cbo_rewrite) { explain { sql("${query_sql}") - nonContains("(${mv_name})") + notContains("(${mv_name})") } return } @@ -1952,7 +1979,7 @@ class Suite implements GroovyInterceptable { // multi mv part in rewrite process, all rewrte fail // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite - def mv_rewrite_all_fail = {query_sql, mv_names, sync_cbo_rewrite = true -> + void mv_rewrite_all_fail(query_sql, mv_names, sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite()) { logger.info("query_sql = " + query_sql + ", mv_names = " + mv_names) if (!sync_cbo_rewrite) { explain { @@ -1983,7 +2010,7 @@ class Suite implements GroovyInterceptable { // multi mv part in rewrite process, any rewrte fail // sync_cbo_rewrite is the bool value which control sync mv is use cbo based mv rewrite - def mv_rewrite_any_fail = {query_sql, mv_names, sync_cbo_rewrite = true -> + void mv_rewrite_any_fail (query_sql, mv_names, sync_cbo_rewrite = enable_sync_mv_cost_based_rewrite()) { logger.info("query_sql = " + query_sql + ", mv_names = " + mv_names) if (!sync_cbo_rewrite) { explain { @@ -2022,7 +2049,7 @@ class Suite implements GroovyInterceptable { """ def job_name = getJobName(db, mv_name); waitingMTMVTaskFinished(job_name) - mv_rewrite_success(query_sql, mv_name) + mv_rewrite_success(query_sql, mv_name, true) } def async_mv_rewrite_success_without_check_chosen = { db, mv_sql, query_sql, mv_name -> @@ -2038,7 +2065,7 @@ class Suite implements GroovyInterceptable { def job_name = getJobName(db, mv_name); waitingMTMVTaskFinished(job_name) - mv_rewrite_success_without_check_chosen(query_sql, mv_name) + mv_rewrite_success_without_check_chosen(query_sql, mv_name, true) } @@ -2055,7 +2082,7 @@ class Suite implements GroovyInterceptable { def job_name = getJobName(db, mv_name); waitingMTMVTaskFinished(job_name) - mv_rewrite_fail(query_sql, mv_name) + mv_rewrite_fail(query_sql, mv_name, true) } def token = context.config.metaServiceToken From 7ca62de2cbf162d622db5080c912428c865d7ceb Mon Sep 17 00:00:00 2001 From: seawinde Date: Tue, 22 Oct 2024 23:23:58 +0800 Subject: [PATCH 2/3] fix regression test --- .../groovy/org/apache/doris/regression/suite/Suite.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy b/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy index 9591489b7103b2..698088906de924 100644 --- a/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy +++ b/regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy @@ -1885,7 +1885,7 @@ class Suite implements GroovyInterceptable { for (String mv_name : mv_names) { def splitResult = result.split("MaterializedViewRewriteFail") def each_result = splitResult.length == 2 ? splitResult[0].contains(mv_name) : false - success = success && (result.contains"(${mv_name})" || each_result) + success = success && (result.contains("(${mv_name})") || each_result) } Assert.assertEquals(true, success) } @@ -1918,7 +1918,7 @@ class Suite implements GroovyInterceptable { for (String mv_name : mv_names) { def splitResult = result.split("MaterializedViewRewriteFail") def each_result = splitResult.length == 2 ? splitResult[0].contains(mv_name) : false - success = success || (result.contains"(${mv_name})" || each_result) + success = success || (result.contains("(${mv_name})") || each_result) } Assert.assertEquals(true, success) } @@ -1947,7 +1947,7 @@ class Suite implements GroovyInterceptable { sql("${query_sql}") check { result -> def splitResult = result.split("MaterializedViewRewriteFail") - result.contains"(${mv_name})" || (splitResult.length == 2 ? splitResult[0].contains(mv_name) : false) + result.contains("(${mv_name})") || (splitResult.length == 2 ? splitResult[0].contains(mv_name) : false) } } return From c8d292b3672a0e2302ecdf95444cf4e74f5b6c3e Mon Sep 17 00:00:00 2001 From: seawinde Date: Wed, 23 Oct 2024 07:46:35 +0800 Subject: [PATCH 3/3] optimize code usage --- .../main/java/org/apache/doris/nereids/NereidsPlanner.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java index 3ef68969455291..c8bd147f24cd8b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java @@ -577,11 +577,11 @@ public String getExplainString(ExplainOptions explainOptions) { ExplainLevel explainLevel = getExplainLevel(explainOptions); String plan = ""; String mvSummary = ""; - if (this.getPhysicalPlan() != null && cascadesContext != null) { + if ((this.getPhysicalPlan() != null || this.getOptimizedPlan() != null) && cascadesContext != null) { mvSummary = cascadesContext.getMaterializationContexts().isEmpty() ? "" : "\n\n========== MATERIALIZATIONS ==========\n" + MaterializationContext.toSummaryString(cascadesContext.getMaterializationContexts(), - this.getPhysicalPlan()); + this.getPhysicalPlan() == null ? this.getOptimizedPlan() : this.getPhysicalPlan()); } switch (explainLevel) { case PARSED_PLAN: