This is an automated email from the ASF dual-hosted git repository. zhangstar333 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push: new 8d966fa34a3 [bug](fold) fix fold constant core dump with variant type (#32265) 8d966fa34a3 is described below commit 8d966fa34a333ff4c36808e2caf269869bf2ba0c Author: zhangstar333 <87313068+zhangstar...@users.noreply.github.com> AuthorDate: Fri Mar 22 14:17:05 2024 +0800 [bug](fold) fix fold constant core dump with variant type (#32265) 1. variant type core dump at call get_data_at function, as not impl this function. 2. some case can't pass at old planner and fold_constant_by_be = on. 3. open enable_fold_constant_by_be = true. --- be/src/runtime/fold_constant_executor.cpp | 11 +++++++- .../expression/rules/FoldConstantRuleOnBE.java | 30 ++++++++++++++++++++-- .../java/org/apache/doris/qe/SessionVariable.java | 9 +++++-- .../org/apache/doris/analysis/SqlModeTest.java | 1 + .../apache/doris/catalog/CreateFunctionTest.java | 3 ++- .../apache/doris/planner/ConstantExpressTest.java | 1 + .../org/apache/doris/planner/QueryPlanTest.java | 1 + .../ExtractCommonFactorsRuleFunctionTest.java | 2 +- .../apache/doris/rewrite/InferFiltersRuleTest.java | 2 +- .../doris/rewrite/RewriteInPredicateRuleTest.java | 2 +- .../correctness/test_date_function_const.groovy | 2 ++ .../correctness/test_time_diff_microseconds.groovy | 2 ++ .../suites/correctness_p0/test_cast_null.groovy | 2 +- .../nereids/test_agg_state_nereids.groovy | 2 +- .../shape/query21.groovy | 1 + .../noStatsRfPrune/query21.groovy | 1 + .../no_stats_shape/query21.groovy | 1 + .../rf_prune/query21.groovy | 1 + .../shape/query21.groovy | 1 + 19 files changed, 64 insertions(+), 11 deletions(-) diff --git a/be/src/runtime/fold_constant_executor.cpp b/be/src/runtime/fold_constant_executor.cpp index 8a19ca1b35c..9120fa266d7 100644 --- a/be/src/runtime/fold_constant_executor.cpp +++ b/be/src/runtime/fold_constant_executor.cpp @@ -63,6 +63,12 @@ using std::map; namespace doris { +static std::unordered_set<PrimitiveType> PRIMITIVE_TYPE_SET { + TYPE_BOOLEAN, TYPE_TINYINT, TYPE_SMALLINT, TYPE_INT, TYPE_BIGINT, + TYPE_LARGEINT, TYPE_FLOAT, TYPE_TIME, TYPE_DOUBLE, TYPE_TIMEV2, + TYPE_CHAR, TYPE_VARCHAR, TYPE_STRING, TYPE_HLL, TYPE_OBJECT, + TYPE_DATE, TYPE_DATETIME, TYPE_DATEV2, TYPE_DATETIMEV2, TYPE_DECIMALV2}; + Status FoldConstantExecutor::fold_constant_vexpr(const TFoldConstantParams& params, PConstantExprResult* response) { const auto& expr_map = params.expr_map; @@ -106,7 +112,9 @@ Status FoldConstantExecutor::fold_constant_vexpr(const TFoldConstantParams& para } else { expr_result.set_success(true); StringRef string_ref; - if (!ctx->root()->type().is_complex_type()) { + auto type = ctx->root()->type().type; + //eg: strcut, array, map VARIANT... will not impl get_data_at, so could use column->to_string() + if (PRIMITIVE_TYPE_SET.contains(type)) { string_ref = column_ptr->get_data_at(0); } RETURN_IF_ERROR(_get_result((void*)string_ref.data, string_ref.size, @@ -264,6 +272,7 @@ Status FoldConstantExecutor::_get_result(void* src, size_t size, const TypeDescr result = column_type->to_string(*column_ptr, 0); break; } + case TYPE_VARIANT: case TYPE_QUANTILE_STATE: { result = column_type->to_string(*column_ptr, 0); break; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java index 27e1b34f9d0..38c6a483c9f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java @@ -32,7 +32,9 @@ import org.apache.doris.nereids.rules.expression.ExpressionRewriteContext; import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.Cast; import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.functions.scalar.Sleep; import org.apache.doris.nereids.trees.expressions.literal.Literal; +import org.apache.doris.nereids.trees.expressions.literal.NumericLiteral; import org.apache.doris.nereids.types.CharType; import org.apache.doris.nereids.types.DataType; import org.apache.doris.nereids.types.DateTimeV2Type; @@ -127,11 +129,21 @@ public class FoldConstantRuleOnBE extends AbstractExpressionRewriteRule { if (((Cast) expr).child().isNullLiteral()) { return; } + if (skipSleepFunction(((Cast) expr).child())) { + return; + } } // skip literal expr if (expr.isLiteral()) { return; } + // eg: avg_state(1) return is agg function serialize data + if (expr.getDataType().isAggStateType()) { + return; + } + if (skipSleepFunction(expr)) { + return; + } String id = idGenerator.getNextId().toString(); constMap.put(id, expr); Expr staleExpr; @@ -150,6 +162,20 @@ public class FoldConstantRuleOnBE extends AbstractExpressionRewriteRule { } } + // if sleep(5) will cause rpc timeout + private boolean skipSleepFunction(Expression expr) { + if (expr instanceof Sleep) { + Expression param = expr.child(0); + if (param instanceof Cast) { + param = param.child(0); + } + if (param instanceof NumericLiteral) { + return ((NumericLiteral) param).getDouble() >= 5.0; + } + } + return false; + } + private Map<String, Expression> evalOnBE(Map<String, Map<String, TExpr>> paramMap, Map<String, Expression> constMap, ConnectContext context) { @@ -183,8 +209,8 @@ public class FoldConstantRuleOnBE extends AbstractExpressionRewriteRule { // TODO: will be delete the debug log after find problem of timeout. LOG.info("fold query {} ", DebugUtil.printId(context.queryId())); - Future<PConstantExprResult> future = - BackendServiceProxy.getInstance().foldConstantExpr(brpcAddress, tParams); + Future<PConstantExprResult> future = BackendServiceProxy.getInstance().foldConstantExpr(brpcAddress, + tParams); PConstantExprResult result = future.get(5, TimeUnit.SECONDS); if (result.getStatus().getStatusCode() == 0) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index b2b1b9efb85..25076e803f6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -943,7 +943,7 @@ public class SessionVariable implements Serializable, Writable { private boolean enableJoinReorderBasedCost = false; @VariableMgr.VarAttr(name = ENABLE_FOLD_CONSTANT_BY_BE, fuzzy = true) - private boolean enableFoldConstantByBe = false; + public boolean enableFoldConstantByBe = true; @VariableMgr.VarAttr(name = ENABLE_REWRITE_ELEMENT_AT_TO_SLOT, fuzzy = true) private boolean enableRewriteElementAtToSlot = true; @@ -1801,7 +1801,12 @@ public class SessionVariable implements Serializable, Writable { default: break; } - + randomInt = random.nextInt(2); + if (randomInt % 2 == 0) { + this.enableFoldConstantByBe = false; + } else { + this.enableFoldConstantByBe = true; + } this.runtimeFilterType = 1 << randomInt; this.enableParallelScan = Config.pull_request_id % 2 == 0 ? randomInt % 2 == 0 : randomInt % 1 == 0; switch (randomInt) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/SqlModeTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/SqlModeTest.java index d70dfc826f0..c27743a951a 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/SqlModeTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/SqlModeTest.java @@ -100,6 +100,7 @@ public class SqlModeTest { } analyzer = AccessTestUtil.fetchAdminAnalyzer(false); + analyzer.getContext().getSessionVariable().setEnableFoldConstantByBe(false); try { parsedStmt.analyze(analyzer); ExprRewriter rewriter = analyzer.getExprRewriter(); diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateFunctionTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateFunctionTest.java index c38438cfd0a..0f464ba2946 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateFunctionTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateFunctionTest.java @@ -74,7 +74,7 @@ public class CreateFunctionTest { public void test() throws Exception { ConnectContext ctx = UtFrameUtils.createDefaultCtx(); ctx.getSessionVariable().setEnableNereidsPlanner(false); - + ctx.getSessionVariable().setEnableFoldConstantByBe(false); // create database db1 createDatabase(ctx, "create database db1;"); @@ -206,6 +206,7 @@ public class CreateFunctionTest { public void testCreateGlobalFunction() throws Exception { ConnectContext ctx = UtFrameUtils.createDefaultCtx(); ctx.getSessionVariable().setEnableNereidsPlanner(false); + ctx.getSessionVariable().setEnableFoldConstantByBe(false); // 1. create database db2 createDatabase(ctx, "create database db2;"); diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/ConstantExpressTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/ConstantExpressTest.java index dd16f69c60c..3809f56b272 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/ConstantExpressTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/ConstantExpressTest.java @@ -37,6 +37,7 @@ public class ConstantExpressTest { public static void beforeClass() throws Exception { UtFrameUtils.startFEServer(runningDir); connectContext = UtFrameUtils.createDefaultCtx(); + connectContext.getSessionVariable().setEnableFoldConstantByBe(false); } private static void testConstantExpressResult(String sql, String result) throws Exception { diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java index c39724efb4b..76fe82e6599 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -67,6 +67,7 @@ public class QueryPlanTest extends TestWithFeService { // create database createDatabase("test"); connectContext.getSessionVariable().setEnableNereidsPlanner(false); + connectContext.getSessionVariable().setEnableFoldConstantByBe(false); Config.enable_odbc_mysql_broker_table = true; createTable("create table test.test1\n" diff --git a/fe/fe-core/src/test/java/org/apache/doris/rewrite/ExtractCommonFactorsRuleFunctionTest.java b/fe/fe-core/src/test/java/org/apache/doris/rewrite/ExtractCommonFactorsRuleFunctionTest.java index f145f66cfa2..e034c2f3576 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/rewrite/ExtractCommonFactorsRuleFunctionTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/rewrite/ExtractCommonFactorsRuleFunctionTest.java @@ -237,7 +237,7 @@ public class ExtractCommonFactorsRuleFunctionTest { + "\"storage_format\" = \"V2\"\n" + ");"; dorisAssert.withTable(createTableSQL); - String query = "select /*+ SET_VAR(enable_nereids_planner=false) */ sum(l_extendedprice* (1 - l_discount)) as revenue " + String query = "select /*+ SET_VAR(enable_nereids_planner=false,enable_fold_constant_by_be=false) */ sum(l_extendedprice* (1 - l_discount)) as revenue " + "from lineitem, part " + "where ( p_partkey = l_partkey and p_brand = 'Brand#11' " + "and p_container in ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') " diff --git a/fe/fe-core/src/test/java/org/apache/doris/rewrite/InferFiltersRuleTest.java b/fe/fe-core/src/test/java/org/apache/doris/rewrite/InferFiltersRuleTest.java index 0710871d817..d5cb69272f3 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/rewrite/InferFiltersRuleTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/rewrite/InferFiltersRuleTest.java @@ -301,7 +301,7 @@ public class InferFiltersRuleTest { sessionVariable.setEnableInferPredicate(true); sessionVariable.setEnableRewriteElementAtToSlot(false); Assert.assertTrue(sessionVariable.isEnableInferPredicate()); - String query = "select /*+ SET_VAR(enable_nereids_planner=false) */ * from tb1 inner join tb2 inner join tb3" + String query = "select /*+ SET_VAR(enable_nereids_planner=false,enable_fold_constant_by_be=false) */ * from tb1 inner join tb2 inner join tb3" + " where tb1.k1 = tb3.k1 and tb2.k1 = tb3.k1 and tb1.k1 is not null"; String planString = dorisAssert.query(query).explainQuery(); Assert.assertTrue(planString.contains("`tb3`.`k1` IS NOT NULL")); diff --git a/fe/fe-core/src/test/java/org/apache/doris/rewrite/RewriteInPredicateRuleTest.java b/fe/fe-core/src/test/java/org/apache/doris/rewrite/RewriteInPredicateRuleTest.java index 6d84a967e86..ba13e6602a4 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/rewrite/RewriteInPredicateRuleTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/rewrite/RewriteInPredicateRuleTest.java @@ -131,7 +131,7 @@ public class RewriteInPredicateRuleTest extends TestWithFeService { List<String> list = Lists.newArrayList(); Lists.newArrayList(literals).forEach(e -> list.add("%s")); list.remove(list.size() - 1); - String queryFormat = "select /*+ SET_VAR(enable_nereids_planner=false) */ * from %s where id in (" + Joiner.on(", ").join(list) + ");"; + String queryFormat = "select /*+ SET_VAR(enable_nereids_planner=false,enable_fold_constant_by_be=false) */ * from %s where id in (" + Joiner.on(", ").join(list) + ");"; String query = String.format(queryFormat, literals); StmtExecutor executor1 = getSqlStmtExecutor(query); Expr expr1 = ((SelectStmt) executor1.getParsedStmt()).getWhereClause(); diff --git a/regression-test/suites/correctness/test_date_function_const.groovy b/regression-test/suites/correctness/test_date_function_const.groovy index fd5607d519e..85d8e5ba711 100644 --- a/regression-test/suites/correctness/test_date_function_const.groovy +++ b/regression-test/suites/correctness/test_date_function_const.groovy @@ -17,6 +17,7 @@ suite("test_date_function_const") { sql 'set enable_nereids_planner=false' + sql 'set enable_fold_constant_by_be = false;' qt_select1 """ select hours_add('2023-03-30 22:23:45.23452',8) @@ -36,6 +37,7 @@ suite("test_date_function_const") { """ sql 'set enable_nereids_planner=true' + sql 'set enable_fold_constant_by_be = true;' sql 'set enable_fallback_to_original_planner=false' diff --git a/regression-test/suites/correctness/test_time_diff_microseconds.groovy b/regression-test/suites/correctness/test_time_diff_microseconds.groovy index e754515250b..335a237ddd5 100644 --- a/regression-test/suites/correctness/test_time_diff_microseconds.groovy +++ b/regression-test/suites/correctness/test_time_diff_microseconds.groovy @@ -43,6 +43,7 @@ suite("test_time_diff_microseconds") { """ sql """set enable_nereids_planner=false""" + sql """set enable_fold_constant_by_be=false""" qt_select1 """ select timediff(t1,t2) from tbl_time order by id @@ -68,6 +69,7 @@ suite("test_time_diff_microseconds") { """ sql """set enable_nereids_planner=true """ + sql """set enable_fold_constant_by_be=true""" sql """set enable_fallback_to_original_planner=false""" qt_select5 """ diff --git a/regression-test/suites/correctness_p0/test_cast_null.groovy b/regression-test/suites/correctness_p0/test_cast_null.groovy index 368a0a1ae79..9ea3b125903 100644 --- a/regression-test/suites/correctness_p0/test_cast_null.groovy +++ b/regression-test/suites/correctness_p0/test_cast_null.groovy @@ -61,7 +61,7 @@ suite("test_cast_null") { """ explain { - sql """SELECT * FROM test_table_t53 LEFT JOIN test_table_t0 ON (('I4') LIKE (CAST(CAST(DATE '1970-05-06' AS FLOAT) AS VARCHAR) ));""" + sql """SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/ * FROM test_table_t53 LEFT JOIN test_table_t0 ON (('I4') LIKE (CAST(CAST(DATE '1970-05-06' AS FLOAT) AS VARCHAR) ));""" contains "19700506" } diff --git a/regression-test/suites/datatype_p0/agg_state/nereids/test_agg_state_nereids.groovy b/regression-test/suites/datatype_p0/agg_state/nereids/test_agg_state_nereids.groovy index 0d9115664a5..a302c274058 100644 --- a/regression-test/suites/datatype_p0/agg_state/nereids/test_agg_state_nereids.groovy +++ b/regression-test/suites/datatype_p0/agg_state/nereids/test_agg_state_nereids.groovy @@ -16,7 +16,7 @@ // under the License. suite("test_agg_state_nereids") { - sql "set global enable_agg_state=true" + sql "set enable_agg_state=true" sql "set enable_nereids_planner=true;" sql "set enable_fallback_to_original_planner=false;" diff --git a/regression-test/suites/nereids_tpcds_shape_sf1000_p0/shape/query21.groovy b/regression-test/suites/nereids_tpcds_shape_sf1000_p0/shape/query21.groovy index 4fd5428ec77..08785b431e4 100644 --- a/regression-test/suites/nereids_tpcds_shape_sf1000_p0/shape/query21.groovy +++ b/regression-test/suites/nereids_tpcds_shape_sf1000_p0/shape/query21.groovy @@ -22,6 +22,7 @@ suite("query21") { sql "use ${db}" sql 'set enable_nereids_planner=true' sql 'set enable_fallback_to_original_planner=false' + sql 'SET enable_fold_constant_by_be = false' //plan shape will be different sql 'set exec_mem_limit=21G' sql 'set be_number_for_test=3' sql 'set parallel_fragment_exec_instance_num=8; ' diff --git a/regression-test/suites/nereids_tpcds_shape_sf100_p0/noStatsRfPrune/query21.groovy b/regression-test/suites/nereids_tpcds_shape_sf100_p0/noStatsRfPrune/query21.groovy index a65a00bd684..0ef9181f791 100644 --- a/regression-test/suites/nereids_tpcds_shape_sf100_p0/noStatsRfPrune/query21.groovy +++ b/regression-test/suites/nereids_tpcds_shape_sf100_p0/noStatsRfPrune/query21.groovy @@ -22,6 +22,7 @@ suite("query21") { sql "use ${db}" sql 'set enable_nereids_planner=true' sql 'set enable_fallback_to_original_planner=false' + sql 'SET enable_fold_constant_by_be = false' //plan shape will be different sql 'set exec_mem_limit=21G' sql 'set be_number_for_test=3' sql 'set enable_runtime_filter_prune=true' diff --git a/regression-test/suites/nereids_tpcds_shape_sf100_p0/no_stats_shape/query21.groovy b/regression-test/suites/nereids_tpcds_shape_sf100_p0/no_stats_shape/query21.groovy index f970316dc61..7636c71c8d8 100644 --- a/regression-test/suites/nereids_tpcds_shape_sf100_p0/no_stats_shape/query21.groovy +++ b/regression-test/suites/nereids_tpcds_shape_sf100_p0/no_stats_shape/query21.groovy @@ -22,6 +22,7 @@ suite("query21") { sql "use ${db}" sql 'set enable_nereids_planner=true' sql 'set enable_fallback_to_original_planner=false' + sql 'SET enable_fold_constant_by_be = false' //plan shape will be different sql 'set exec_mem_limit=21G' sql 'set be_number_for_test=3' sql 'set enable_runtime_filter_prune=false' diff --git a/regression-test/suites/nereids_tpcds_shape_sf100_p0/rf_prune/query21.groovy b/regression-test/suites/nereids_tpcds_shape_sf100_p0/rf_prune/query21.groovy index fa7b40fa909..08149220877 100644 --- a/regression-test/suites/nereids_tpcds_shape_sf100_p0/rf_prune/query21.groovy +++ b/regression-test/suites/nereids_tpcds_shape_sf100_p0/rf_prune/query21.groovy @@ -22,6 +22,7 @@ suite("query21") { sql "use ${db}" sql 'set enable_nereids_planner=true' sql 'set enable_fallback_to_original_planner=false' + sql 'SET enable_fold_constant_by_be = false' //plan shape will be different sql 'set exec_mem_limit=21G' sql 'set be_number_for_test=3' sql 'set parallel_fragment_exec_instance_num=8; ' diff --git a/regression-test/suites/nereids_tpcds_shape_sf100_p0/shape/query21.groovy b/regression-test/suites/nereids_tpcds_shape_sf100_p0/shape/query21.groovy index f4ce2bc117e..a5986974dab 100644 --- a/regression-test/suites/nereids_tpcds_shape_sf100_p0/shape/query21.groovy +++ b/regression-test/suites/nereids_tpcds_shape_sf100_p0/shape/query21.groovy @@ -22,6 +22,7 @@ suite("query21") { sql "use ${db}" sql 'set enable_nereids_planner=true' sql 'set enable_fallback_to_original_planner=false' + sql 'SET enable_fold_constant_by_be = false' //plan shape will be different sql 'set exec_mem_limit=21G' sql 'set be_number_for_test=3' sql 'set parallel_fragment_exec_instance_num=8; ' --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org