From 2f1ff9bfdfa5c0f4a5bb7fd64bd48968d5e55f42 Mon Sep 17 00:00:00 2001 From: jingtaoye35 <1255153887@qq.com> Date: Tue, 16 Jan 2024 08:42:38 +0000 Subject: [PATCH] fix an exec_param and a expr copy_on_replace bugs --- src/sql/optimizer/ob_log_subplan_filter.cpp | 2 + .../dml/ob_aggr_expr_push_up_analyzer.cpp | 133 ++++++++++++++++++ .../dml/ob_aggr_expr_push_up_analyzer.h | 19 ++- src/sql/resolver/dml/ob_stmt_expr_visitor.cpp | 2 +- src/sql/resolver/expr/ob_raw_expr_replacer.h | 4 +- src/sql/resolver/expr/ob_raw_expr_util.cpp | 51 +++++-- src/sql/resolver/expr/ob_raw_expr_util.h | 9 +- 7 files changed, 196 insertions(+), 24 deletions(-) diff --git a/src/sql/optimizer/ob_log_subplan_filter.cpp b/src/sql/optimizer/ob_log_subplan_filter.cpp index 937efdf90e..7b08000b0f 100644 --- a/src/sql/optimizer/ob_log_subplan_filter.cpp +++ b/src/sql/optimizer/ob_log_subplan_filter.cpp @@ -637,6 +637,7 @@ int ObLogSubPlanFilter::replace_nested_subquery_exprs(ObRawExprReplacer &replace } for (int64_t i = 0; OB_SUCC(ret) && i < subquery_exprs_.count(); ++i) { ObRawExpr *expr = subquery_exprs_.at(i); + int64_t ref_id = subquery_exprs_.at(i)->get_ref_id(); if (ObOptimizerUtil::find_item(plan->get_onetime_query_refs(), expr)) { // do not replace onetime expr ref query, only adjust nested subquery } else if (OB_FAIL(replace_expr_action(replacer, expr))) { @@ -648,6 +649,7 @@ int ObLogSubPlanFilter::replace_nested_subquery_exprs(ObRawExprReplacer &replace LOG_WARN("unexpected expr type", K(ret)); } else { subquery_exprs_.at(i) = static_cast(expr); + subquery_exprs_.at(i)->set_ref_id(ref_id); } } return ret; diff --git a/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.cpp b/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.cpp index bcd184863d..fad91e98bc 100644 --- a/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.cpp +++ b/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.cpp @@ -64,6 +64,8 @@ int ObAggrExprPushUpAnalyzer::analyze_and_push_up_aggr_expr(ObRawExprFactory &ex } else if (OB_FAIL(ObTransformUtils::decorrelate(reinterpret_cast(aggr_expr), final_exec_params))) { LOG_WARN("failed to decorrelate exec params", K(ret)); + } else if (OB_FAIL(replace_final_exec_param_in_aggr(final_exec_params, param_query_refs, expr_factory))) { + LOG_WARN("failed to replace real exec param in aggr", K(ret)); } if (OB_SUCC(ret)) { if (OB_FAIL(final_aggr_resolver->add_aggr_expr(aggr_expr))) { @@ -444,5 +446,136 @@ int ObAggrExprPushUpAnalyzer::get_exec_params(ObDMLResolver *resolver, return ret; } +int ObAggrExprPushUpAnalyzer::replace_final_exec_param_in_aggr(const ObIArray &exec_params, + ObIArray ¶m_query_refs, + ObRawExprFactory &expr_factory) +{ + int ret = OB_SUCCESS; + ObSEArray new_execs; + for (int64_t i = 0; OB_SUCC(ret) && i < exec_params.count(); ++i) { + ObExecParamRawExpr *new_expr = NULL; + if (OB_FAIL(ObRawExprUtils::create_new_exec_param(expr_factory, + exec_params.at(i)->get_ref_expr(), + new_expr, + false))) { + LOG_WARN("failed to create new exec param", K(ret)); + } else if (OB_FAIL(new_execs.push_back(static_cast(new_expr)))) { + LOG_WARN("failed to push back", K(ret)); + } + } + if (OB_SUCC(ret)) { + ObStmtExecParamReplacer replacer; + replacer.set_relation_scope(); + if (OB_FAIL(replacer.add_replace_exprs(exec_params, new_execs))) { + LOG_WARN("failed to add replace exprs", K(ret)); + } else { + for (int64_t i = 0; OB_SUCC(ret) && i < param_query_refs.count(); i++) { + if (OB_FAIL(replacer.do_visit(reinterpret_cast(param_query_refs.at(i))))) { + LOG_WARN("failed to replace exec param", K(ret)); + } + } + } + } + return ret; +} + +int ObStmtExecParamReplacer::check_need_replace(const ObRawExpr *old_expr, + ObRawExpr *&new_expr, + bool &need_replace) +{ + int ret = OB_SUCCESS; + uint64_t key = reinterpret_cast(old_expr); + uint64_t val = 0; + need_replace = false; + if (OB_UNLIKELY(!expr_replace_map_.created())) { + // do nothing + } else if (OB_FAIL(expr_replace_map_.get_refactored(key, val))) { + if (OB_HASH_NOT_EXIST == ret) { + ret = OB_SUCCESS; + } else { + LOG_WARN("failed to get expr from hash map", K(ret)); + } + } else { + need_replace = true; + new_expr = reinterpret_cast(val); + } + return ret; +} + +int ObStmtExecParamReplacer::add_replace_exprs(const ObIArray &from_exprs, + const ObIArray &to_exprs) +{ + int ret = OB_SUCCESS; + int64_t bucket_size = MAX(from_exprs.count(), 64); + if (OB_UNLIKELY(from_exprs.count() != to_exprs.count())) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("expr size mismatch", K(from_exprs.count()), K(to_exprs.count()), K(ret)); + } else if (expr_replace_map_.created()) { + /* do nothing */ + } else if (OB_FAIL(expr_replace_map_.create(bucket_size, ObModIds::OB_SQL_COMPILE))) { + LOG_WARN("failed to create expr map", K(ret)); + } else if (OB_FAIL(to_exprs_.create(bucket_size))) { + LOG_WARN("failed to create expr set", K(ret)); + } + const ObRawExpr *from_expr = NULL; + const ObRawExpr *to_expr = NULL; + for (int64_t i = 0; OB_SUCC(ret) && i < from_exprs.count(); ++i) { + bool is_existed = false; + ObRawExpr *new_expr = NULL; + if (OB_ISNULL(from_expr = from_exprs.at(i)) || OB_ISNULL(to_expr = to_exprs.at(i))) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("unexpected null expr", KP(from_expr), KP(to_expr), K(ret)); + } else if (OB_FAIL(check_need_replace(from_expr, new_expr, is_existed))) { + LOG_WARN("failed to check need replace", K(ret)); + } else if (is_existed) { + /* do nothing */ + } else if (OB_FAIL(expr_replace_map_.set_refactored(reinterpret_cast(from_expr), + reinterpret_cast(to_expr)))) { + LOG_WARN("failed to add replace expr into map", K(ret)); + } else if (OB_FAIL(to_exprs_.set_refactored(reinterpret_cast(to_expr)))) { + LOG_WARN("failed to add replace expr into set", K(ret)); + } + } + return ret; +} + +int ObStmtExecParamReplacer::do_visit(ObRawExpr *&expr) +{ + int ret = OB_SUCCESS; + bool is_happended = false; + if (OB_ISNULL(expr)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("expr is null", K(ret), K(expr)); + } else if (expr->is_exec_param_expr()) { + bool need_replace = false; + ObRawExpr *to_expr; + if (OB_FAIL(check_need_replace(expr, to_expr, need_replace))) { + LOG_WARN("failed to check need replace", K(ret)); + } else if (need_replace) { + expr = to_expr; + } + } else if (expr->is_query_ref_expr()) { + ObQueryRefRawExpr *query_ref_expr = static_cast(expr); + for (int64_t i = 0; OB_SUCC(ret) && i < query_ref_expr->get_param_count(); ++i) { + if (OB_FAIL(SMART_CALL(do_visit(reinterpret_cast( + query_ref_expr->get_exec_params().at(i)))))) { + LOG_WARN("failed to remove const exec param", K(ret)); + } + } + if (NULL == query_ref_expr->get_ref_stmt()) { + /* ref_stmt may has not been resolve yet */ + } else if (OB_FAIL(SMART_CALL(query_ref_expr->get_ref_stmt()->iterate_stmt_expr(*this)))) { + LOG_WARN("failed to iterator stmt expr", K(ret)); + } + } else if (expr->has_flag(CNT_SUB_QUERY)) { + for (int64_t i = 0; OB_SUCC(ret) && i < expr->get_param_count(); i++) { + if (OB_FAIL(SMART_CALL(do_visit(expr->get_param_expr(i))))) { + LOG_WARN("failed to do replace exec param", K(ret)); + } + } + } + return ret; +} + } // namespace sql } // namespace oceanbase diff --git a/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.h b/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.h index d548ee161a..38ec245f18 100644 --- a/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.h +++ b/src/sql/resolver/dml/ob_aggr_expr_push_up_analyzer.h @@ -15,6 +15,7 @@ #include "lib/container/ob_array.h" #include "sql/ob_sql_utils.h" #include "sql/resolver/expr/ob_raw_expr.h" +#include "sql/resolver/dml/ob_stmt_expr_visitor.h" namespace oceanbase { namespace sql @@ -23,6 +24,20 @@ class ObDMLResolver; class ObAggFunRawExpr; class ObSelectResolver; class ObSelectStmt; + +class ObStmtExecParamReplacer : public ObStmtExprVisitor +{ +public: + ObStmtExecParamReplacer() {} + virtual int do_visit(ObRawExpr *&expr) override; + int add_replace_exprs(const ObIArray &from_exprs, + const ObIArray &to_exprs); + int check_need_replace(const ObRawExpr *old_expr, ObRawExpr *&new_expr, bool &need_replace); +private: + hash::ObHashSet to_exprs_; + hash::ObHashMap expr_replace_map_; +}; + class ObAggrExprPushUpAnalyzer { public: @@ -63,7 +78,9 @@ private: int remove_alias_exprs(); int remove_alias_exprs(ObRawExpr* &expr); - + int replace_final_exec_param_in_aggr(const ObIArray &exec_params, + ObIArray ¶m_query_refs, + ObRawExprFactory &expr_factory); private: // contain current layer column (a real column or a alias select item) bool has_cur_layer_column_; diff --git a/src/sql/resolver/dml/ob_stmt_expr_visitor.cpp b/src/sql/resolver/dml/ob_stmt_expr_visitor.cpp index 28c9ff9ad0..ddfad4c49b 100644 --- a/src/sql/resolver/dml/ob_stmt_expr_visitor.cpp +++ b/src/sql/resolver/dml/ob_stmt_expr_visitor.cpp @@ -262,4 +262,4 @@ int ObStmtExecParamFormatter::do_formalize_exec_param(ObRawExpr *&expr, bool &is } } return ret; -} \ No newline at end of file +} diff --git a/src/sql/resolver/expr/ob_raw_expr_replacer.h b/src/sql/resolver/expr/ob_raw_expr_replacer.h index 1a1fd07e27..d00e46f407 100644 --- a/src/sql/resolver/expr/ob_raw_expr_replacer.h +++ b/src/sql/resolver/expr/ob_raw_expr_replacer.h @@ -65,6 +65,7 @@ public: const ObIArray &to_exprs); int add_replace_exprs(const ObIArray> &to_replace_exprs); int append_replace_exprs(const ObRawExprReplacer &other); + int check_need_replace(const ObRawExpr *old_expr, ObRawExpr *&new_expr, bool &need_replace); private: // types and constants @@ -76,9 +77,6 @@ private: const bool overwrite, bool &is_existed); int check_skip_expr(const ObRawExpr &expr, bool &skip_expr); - int check_need_replace(const ObRawExpr *old_expr, - ObRawExpr *&new_expr, - bool &need_replace); // disallow copy DISALLOW_COPY_AND_ASSIGN(ObRawExprReplacer); // function members diff --git a/src/sql/resolver/expr/ob_raw_expr_util.cpp b/src/sql/resolver/expr/ob_raw_expr_util.cpp index e03e91cd0c..e5c2cad7d2 100644 --- a/src/sql/resolver/expr/ob_raw_expr_util.cpp +++ b/src/sql/resolver/expr/ob_raw_expr_util.cpp @@ -4576,29 +4576,52 @@ int ObRawExprUtils::get_exec_param_expr(ObRawExprFactory &expr_factory, // we create a new one here if (OB_SUCC(ret) && NULL == param_expr) { ObExecParamRawExpr *exec_param = NULL; - if (OB_FAIL(expr_factory.create_raw_expr(T_QUESTIONMARK, exec_param))) { - LOG_WARN("failed to create raw expr", K(ret)); - } else if (OB_ISNULL(exec_param)) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("exec param is null", K(ret), K(exec_param)); + if (OB_FAIL(ObRawExprUtils::create_new_exec_param(expr_factory, + outer_val_expr, + exec_param, + false))) { + LOG_WARN("failed to create new exec param", K(ret)); } else if (OB_FAIL(query_ref->add_exec_param_expr(exec_param))) { LOG_WARN("failed to add exec param expr", K(ret)); - } else if (OB_FAIL(exec_param->set_enum_set_values(outer_val_expr->get_enum_set_values()))) { - LOG_WARN("failed to set enum set values", K(ret)); - } else if (OB_FAIL(exec_param->add_flag(IS_CONST))) { - LOG_WARN("failed to add flag", K(ret)); - } else if (OB_FAIL(exec_param->add_flag(IS_DYNAMIC_PARAM))) { - LOG_WARN("failed to add flag", K(ret)); } else { - exec_param->set_ref_expr(outer_val_expr); - exec_param->set_param_index(-1); - exec_param->set_result_type(outer_val_expr->get_result_type()); param_expr = exec_param; } } return ret; } +int ObRawExprUtils::create_new_exec_param(ObRawExprFactory &expr_factory, + ObRawExpr *ref_expr, + ObExecParamRawExpr *&exec_param, + bool is_onetime /*=false*/) +{ + int ret = OB_SUCCESS; + exec_param = NULL; + if (OB_ISNULL(ref_expr)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("expr is null", K(ret), K(ref_expr)); + } else if (OB_FAIL(expr_factory.create_raw_expr(T_QUESTIONMARK, exec_param))) { + LOG_WARN("failed to create exec param expr", K(ret)); + } else if (OB_ISNULL(exec_param)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("exec param is null", K(ret), K(exec_param)); + } else if (OB_FAIL(exec_param->set_enum_set_values(ref_expr->get_enum_set_values()))) { + LOG_WARN("failed to set enum set values", K(ret)); + } else if (OB_FAIL(exec_param->add_flag(IS_CONST))) { + LOG_WARN("failed to add flag", K(ret)); + } else if (OB_FAIL(exec_param->add_flag(IS_DYNAMIC_PARAM))) { + LOG_WARN("failed to add flag", K(ret)); + } else { + exec_param->set_ref_expr(ref_expr, is_onetime); + exec_param->set_param_index(-1); + exec_param->set_result_type(ref_expr->get_result_type()); + if (is_onetime) { + exec_param->add_flag(IS_ONETIME); + } + } + return ret; +} + int ObRawExprUtils::create_new_exec_param(ObQueryCtx *query_ctx, ObRawExprFactory &expr_factory, ObRawExpr *&expr, diff --git a/src/sql/resolver/expr/ob_raw_expr_util.h b/src/sql/resolver/expr/ob_raw_expr_util.h index ed1046a947..e1106a0634 100644 --- a/src/sql/resolver/expr/ob_raw_expr_util.h +++ b/src/sql/resolver/expr/ob_raw_expr_util.h @@ -557,11 +557,10 @@ public: ObRawExprFactory &expr_factory, ObRawExpr *&expr, bool is_onetime = false); - - static int create_exec_param_expr(ObQueryCtx *query_ctx, - ObRawExprFactory &expr_factory, - ObRawExpr *&src_expr, - std::pair &init_expr); + static int create_new_exec_param(ObRawExprFactory &expr_factory, + ObRawExpr *ref_expr, + ObExecParamRawExpr *&exec_param, + bool is_onetime = false); static int create_param_expr(ObRawExprFactory &expr_factory, int64_t param_idx, ObRawExpr *&expr); static int build_trim_expr(const share::schema::ObColumnSchemaV2 *column_schema, ObRawExprFactory &expr_factory,