Skip to content

Commit

Permalink
[bug](fold) fix fold constant core dump with variant type (#32265)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
zhangstar333 authored Mar 22, 2024
1 parent a118eb9 commit 8d966fa
Show file tree
Hide file tree
Showing 19 changed files with 64 additions and 11 deletions.
11 changes: 10 additions & 1 deletion be/src/runtime/fold_constant_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
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;
Expand Down Expand Up @@ -127,11 +129,21 @@ private void collectConst(Expression expr, Map<String, Expression> constMap, Map
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;
Expand All @@ -150,6 +162,20 @@ private void collectConst(Expression expr, Map<String, Expression> constMap, Map
}
}

// 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) {

Expand Down Expand Up @@ -183,8 +209,8 @@ private Map<String, Expression> evalOnBE(Map<String, Map<String, TExpr>> paramMa

// 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1801,7 +1801,12 @@ public void initFuzzyModeVariables() {
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public void testPipesAsConcatModeNull() {
}

analyzer = AccessTestUtil.fetchAdminAnalyzer(false);
analyzer.getContext().getSessionVariable().setEnableFoldConstantByBe(false);
try {
parsedStmt.analyze(analyzer);
ExprRewriter rewriter = analyzer.getExprRewriter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public static void teardown() {
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;");

Expand Down Expand Up @@ -206,6 +206,7 @@ public void test() throws Exception {
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;");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ protected void runBeforeAll() throws Exception {
// 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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public void testComplexQuery() throws Exception {
+ "\"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') "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public void testWhereIsNotNullPredicate() throws Exception {
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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private void testBase(int childrenNum, PrimitiveType type, long expectedOfChild1
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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'


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; '
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; '
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; '
Expand Down

0 comments on commit 8d966fa

Please sign in to comment.