From d76e2df6efa1bf772ac16bc0d05c251110bace2e Mon Sep 17 00:00:00 2001 From: yinyj17 Date: Mon, 6 Nov 2023 09:39:30 +0000 Subject: [PATCH] fix distinct fd judgement bug --- src/sql/optimizer/ob_optimizer_util.cpp | 90 ++++++++++++------- src/sql/optimizer/ob_optimizer_util.h | 11 ++- .../expr/ob_raw_expr_info_extractor.cpp | 13 +-- .../rewrite/ob_transform_simplify_groupby.cpp | 4 + .../rewrite/ob_transform_simplify_groupby.h | 3 - 5 files changed, 73 insertions(+), 48 deletions(-) diff --git a/src/sql/optimizer/ob_optimizer_util.cpp b/src/sql/optimizer/ob_optimizer_util.cpp index cce01c073e..4d56dfb947 100644 --- a/src/sql/optimizer/ob_optimizer_util.cpp +++ b/src/sql/optimizer/ob_optimizer_util.cpp @@ -8300,31 +8300,67 @@ int ObOptimizerUtil::check_contain_batch_stmt_parameter(ObRawExpr* expr, bool &c } /* Check whether src_expr can be calculated by const exprs and dst_exprs */ -int ObOptimizerUtil::expr_calculable_by_exprs(const ObRawExpr *src_expr, +int ObOptimizerUtil::expr_calculable_by_exprs(ObRawExpr *src_expr, const ObIArray &dst_exprs, + const bool need_check_contain, + const bool used_in_compare, bool &is_calculable) { int ret = OB_SUCCESS; + ObSEArray parent_exprs; + if (OB_FAIL(expr_calculable_by_exprs(src_expr, dst_exprs, parent_exprs, + need_check_contain, used_in_compare, is_calculable))) { + LOG_WARN("fail to check expr is calculable by other exprs", K(ret)); + } + return ret; +} + + +int ObOptimizerUtil::expr_calculable_by_exprs(ObRawExpr *src_expr, + const ObIArray &dst_exprs, + ObIArray &parent_exprs, + const bool need_check_contain, + const bool used_in_compare, + bool &is_calculable) +{ + int ret = OB_SUCCESS; + bool can_replace = false; + bool is_const_inherit = false; is_calculable = true; if (OB_ISNULL(src_expr)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("unexpected NULL", K(ret)); } else if (src_expr->is_const_expr()) { + // is calculable } else if (dst_exprs.empty()) { is_calculable = false; - } else if (ObOptimizerUtil::find_item(dst_exprs, src_expr)) { + } else if (OB_FAIL(ObTransformUtils::check_can_replace(src_expr, parent_exprs, + used_in_compare, can_replace))) { + LOG_WARN("failed to check can replace expr", K(ret)); + } else if (!can_replace) { + is_calculable = false; + } else if (need_check_contain && ObOptimizerUtil::find_item(dst_exprs, src_expr)) { + // is calculable + } else if (OB_FAIL(src_expr->is_const_inherit_expr(is_const_inherit, true))) { + LOG_WARN("failed to check is const inherit expr", K(ret)); + } else if (!is_const_inherit) { + is_calculable = false; } else { - int64_t N = src_expr->get_param_count(); - if (N > 0) { - for (int64_t i = 0; OB_SUCC(ret) && is_calculable && i < N; ++i) { - if (OB_FAIL(SMART_CALL(expr_calculable_by_exprs(src_expr->get_param_expr(i), - dst_exprs, - is_calculable)))) { - LOG_WARN("failed to smart call", K(ret)); - } + if (OB_FAIL(parent_exprs.push_back(src_expr))) { + LOG_WARN("failed to push back", K(ret)); + } + for (int64_t i = 0; OB_SUCC(ret) && is_calculable && i < src_expr->get_param_count(); ++i) { + if (OB_FAIL(SMART_CALL(expr_calculable_by_exprs(src_expr->get_param_expr(i), + dst_exprs, + parent_exprs, + true, + used_in_compare, + is_calculable)))) { + LOG_WARN("failed to smart call expr_calculable_by_exprs", K(ret)); } - } else { - is_calculable = false; + } + if (OB_SUCC(ret)) { + parent_exprs.pop_back(); } } return ret; @@ -8365,27 +8401,17 @@ int ObOptimizerUtil::get_minset_of_exprs(const ObIArray &src_exprs, int ret = OB_SUCCESS; //find expr which can not be evaluated by other exprs for (int i = 0; OB_SUCC(ret) && i < src_exprs.count(); ++i) { - if (OB_ISNULL(src_exprs.at(i))) { + ObRawExpr *expr = src_exprs.at(i); + bool is_calculable = false; + if (OB_ISNULL(expr)) { ret = OB_ERR_UNEXPECTED; - LOG_WARN("unexpected null", K(ret), K(src_exprs.at(i))); - } else if (src_exprs.at(i)->get_param_count() == 0 || - src_exprs.at(i)->has_flag(CNT_WINDOW_FUNC) || - src_exprs.at(i)->has_flag(CNT_AGG)) { - if (OB_FAIL(min_set.push_back(src_exprs.at(i)))) { - LOG_WARN("fail to push back expr", K(ret)); - } - } else { - bool calculable = true; - for (int64_t j = 0; OB_SUCC(ret) && calculable && j < src_exprs.at(i)->get_param_count(); ++j) { - if (OB_FAIL(ObOptimizerUtil::expr_calculable_by_exprs(src_exprs.at(i)->get_param_expr(j), src_exprs, calculable))) { - LOG_WARN("fail to check if candi expr is calculable", K(ret)); - } - } - if (OB_SUCC(ret) && !calculable) { - if (OB_FAIL(min_set.push_back(src_exprs.at(i)))) { - LOG_WARN("failed to push back expr", K(ret)); - } - } + LOG_WARN("unexpected null", K(ret), K(expr)); + } else if (OB_FAIL(expr_calculable_by_exprs(expr, src_exprs, false, true, is_calculable))) { + LOG_WARN("fail to check expr is calculable by other exprs", K(ret)); + } else if (is_calculable) { + // do nothing + } else if (OB_FAIL(min_set.push_back(expr))) { + LOG_WARN("fail to push back expr", K(ret)); } } return ret; diff --git a/src/sql/optimizer/ob_optimizer_util.h b/src/sql/optimizer/ob_optimizer_util.h index b1d6bd4bd8..d9feb7d112 100644 --- a/src/sql/optimizer/ob_optimizer_util.h +++ b/src/sql/optimizer/ob_optimizer_util.h @@ -1392,8 +1392,17 @@ public: static int check_contain_batch_stmt_parameter(ObRawExpr* expr, bool &contain); - static int expr_calculable_by_exprs(const ObRawExpr *src_expr, + static int expr_calculable_by_exprs(ObRawExpr *src_expr, const ObIArray &dst_exprs, + const bool need_check_contain, + const bool used_in_compare, + bool &is_calculable); + + static int expr_calculable_by_exprs(ObRawExpr *src_expr, + const ObIArray &dst_exprs, + ObIArray &parent_exprs, + const bool need_check_contain, + const bool used_in_compare, bool &is_calculable); static int get_minset_of_exprs(const ObIArray &src_exprs, ObIArray &min_set); diff --git a/src/sql/resolver/expr/ob_raw_expr_info_extractor.cpp b/src/sql/resolver/expr/ob_raw_expr_info_extractor.cpp index cf17e2bac8..ab8e265fef 100644 --- a/src/sql/resolver/expr/ob_raw_expr_info_extractor.cpp +++ b/src/sql/resolver/expr/ob_raw_expr_info_extractor.cpp @@ -39,17 +39,6 @@ int ObRawExprInfoExtractor::visit(ObConstRawExpr &expr) int ret = OB_SUCCESS; ObItemType type = expr.get_expr_type(); switch (type) { - case T_USER_VARIABLE_IDENTIFIER: { - ObUserVarIdentRawExpr &var_expr = static_cast(expr); - if (var_expr.get_is_contain_assign() || var_expr.get_query_has_udf()) { - if (OB_FAIL(var_expr.add_flag(IS_DYNAMIC_USER_VARIABLE))) { - LOG_WARN("add flag to user var ident raw expr failed", KR(ret)); - } - } else if (OB_FAIL(var_expr.add_flag(IS_CONST))) { - LOG_WARN("failed to add flag IS_CONST", K(ret)); - } - break; - } case T_SYSTEM_VARIABLE: case T_QUESTIONMARK: { if (OB_FAIL(expr.add_flag(IS_STATIC_PARAM))) { @@ -68,7 +57,7 @@ int ObRawExprInfoExtractor::visit(ObConstRawExpr &expr) default: break; } - if (OB_SUCC(ret) && T_USER_VARIABLE_IDENTIFIER != type) { + if (OB_SUCC(ret)) { if (OB_FAIL(expr.add_flag(IS_CONST))) { LOG_WARN("failed to add flag IS_CONST", K(ret)); } diff --git a/src/sql/rewrite/ob_transform_simplify_groupby.cpp b/src/sql/rewrite/ob_transform_simplify_groupby.cpp index fc3b132184..ef1a0802a1 100644 --- a/src/sql/rewrite/ob_transform_simplify_groupby.cpp +++ b/src/sql/rewrite/ob_transform_simplify_groupby.cpp @@ -1900,6 +1900,8 @@ int ObTransformSimplifyGroupby::check_can_convert_to_distinct(ObSelectStmt *stmt LOG_WARN("unexpected null", K(ret), K(select_exprs.at(i))); } else if (OB_FAIL(ObOptimizerUtil::expr_calculable_by_exprs(select_exprs.at(i), stmt->get_group_exprs(), + true, // need_check_contain + true, // used_in_compare is_calculable))) { LOG_WARN("fail to check if select expr is const or exist", K(ret)); } else { @@ -1912,6 +1914,8 @@ int ObTransformSimplifyGroupby::check_can_convert_to_distinct(ObSelectStmt *stmt LOG_WARN("unexpected null", K(ret), K(stmt->get_having_exprs().at(i))); } else if (OB_FAIL(ObOptimizerUtil::expr_calculable_by_exprs(stmt->get_having_exprs().at(i), stmt->get_group_exprs(), + true, // need_check_contain + true, // used_in_compare is_calculable))) { LOG_WARN("fail to check if having expr is const or exist", K(ret)); } else { diff --git a/src/sql/rewrite/ob_transform_simplify_groupby.h b/src/sql/rewrite/ob_transform_simplify_groupby.h index e13c35a8a9..a73d0b1029 100644 --- a/src/sql/rewrite/ob_transform_simplify_groupby.h +++ b/src/sql/rewrite/ob_transform_simplify_groupby.h @@ -110,9 +110,6 @@ private: ObIArray &vaild_having_exprs); int convert_group_by_to_distinct(ObDMLStmt *stmt, bool &trans_happened); int check_can_convert_to_distinct(ObSelectStmt *stmt, bool &can_convert); - int expr_calculable_by_exprs(const ObRawExpr *src_expr, - const ObIArray &dst_exprs, - bool &is_calculable); }; }