From d8748259523cd39175486a9465256be38bf4a3eb Mon Sep 17 00:00:00 2001 From: Larry955 <1412857955@qq.com> Date: Mon, 28 Nov 2022 03:22:43 +0000 Subject: [PATCH] fix failed cases caused by udf expr compare --- src/sql/resolver/expr/ob_raw_expr.cpp | 83 ++++++++++--------- src/sql/resolver/expr/ob_raw_expr.h | 16 +++- src/sql/rewrite/ob_stmt_comparer.cpp | 6 +- src/sql/rewrite/ob_stmt_comparer.h | 42 ++++++++++ .../ob_transform_subquery_coalesce.cpp | 27 ++++-- .../rewrite/ob_transform_subquery_coalesce.h | 3 +- 6 files changed, 125 insertions(+), 52 deletions(-) diff --git a/src/sql/resolver/expr/ob_raw_expr.cpp b/src/sql/resolver/expr/ob_raw_expr.cpp index 2935913058..dc05fd3b3d 100644 --- a/src/sql/resolver/expr/ob_raw_expr.cpp +++ b/src/sql/resolver/expr/ob_raw_expr.cpp @@ -4304,46 +4304,49 @@ bool ObUDFRawExpr::inner_same_as(const ObRawExpr &expr, ObExprEqualCheckContext *check_context) const { - bool bool_ret = is_deterministic_; - if (bool_ret) { - bool_ret = ObSysFunRawExpr::inner_same_as(expr, check_context); - if (bool_ret) { - const ObUDFRawExpr *other = static_cast(&expr); - bool_ret = udf_id_ == other->get_udf_id() && - pkg_id_ == other->get_pkg_id() && - type_id_ == other->get_type_id() && - pls_type_ == other->get_pls_type() && - database_name_.compare(other->get_database_name()) == 0 && - package_name_.compare(other->get_package_name()) == 0 && - is_deterministic_ == other->is_deterministic() && - is_parallel_enable_ == other->is_parallel_enable() && - is_udt_udf_ == other->get_is_udt_udf() && - is_pkg_body_udf_ == other->is_pkg_body_udf() && - is_return_sys_cursor_ == other->get_is_return_sys_cursor() && - is_aggregate_udf_ == other->get_is_aggregate_udf() && - is_aggr_udf_distinct_ == other->get_is_aggr_udf_distinct() && - loc_ == other->get_loc() && - is_udt_cons_ == other->get_is_udt_cons() && - subprogram_path_.count() == other->get_subprogram_path().count() && - params_type_.count() == other->get_params_type().count() && - nocopy_params_.count() == other->get_nocopy_params().count() && - params_name_.count() == other->get_params_name().count() && - params_desc_v2_.count() == other->get_params_desc().count(); - for (int64_t i = 0; bool_ret && i < subprogram_path_.count(); ++i) { - bool_ret = subprogram_path_.at(i) == other->get_subprogram_path().at(i); - } - for (int64_t i = 0; bool_ret && i < params_type_.count(); ++i) { - bool_ret = params_type_.at(i) == other->get_params_type().at(i); - } - for (int64_t i = 0; bool_ret && i < nocopy_params_.count(); ++i) { - bool_ret = nocopy_params_.at(i) == other->get_nocopy_params().at(i); - } - for (int64_t i = 0; bool_ret && i < params_name_.count(); ++i) { - bool_ret = params_name_.at(i).compare(other->get_params_name().at(i)) == 0; - } - for (int64_t i = 0; bool_ret && i < params_desc_v2_.count(); ++i) { - bool_ret = params_desc_v2_.at(i) == other->get_params_desc().at(i); - } + bool bool_ret = true; + if (this == &expr) { + // do nothing + } else if (NULL != check_context && check_context->need_check_deterministic_ && !is_deterministic_) { + bool_ret = false; + } else if (!ObSysFunRawExpr::inner_same_as(expr, check_context)) { + bool_ret = false; + } else { + const ObUDFRawExpr *other = static_cast(&expr); + bool_ret = udf_id_ == other->get_udf_id() && + pkg_id_ == other->get_pkg_id() && + type_id_ == other->get_type_id() && + pls_type_ == other->get_pls_type() && + database_name_.compare(other->get_database_name()) == 0 && + package_name_.compare(other->get_package_name()) == 0 && + is_deterministic_ == other->is_deterministic() && + is_parallel_enable_ == other->is_parallel_enable() && + is_udt_udf_ == other->get_is_udt_udf() && + is_pkg_body_udf_ == other->is_pkg_body_udf() && + is_return_sys_cursor_ == other->get_is_return_sys_cursor() && + is_aggregate_udf_ == other->get_is_aggregate_udf() && + is_aggr_udf_distinct_ == other->get_is_aggr_udf_distinct() && + loc_ == other->get_loc() && + is_udt_cons_ == other->get_is_udt_cons() && + subprogram_path_.count() == other->get_subprogram_path().count() && + params_type_.count() == other->get_params_type().count() && + nocopy_params_.count() == other->get_nocopy_params().count() && + params_name_.count() == other->get_params_name().count() && + params_desc_v2_.count() == other->get_params_desc().count(); + for (int64_t i = 0; bool_ret && i < subprogram_path_.count(); ++i) { + bool_ret = subprogram_path_.at(i) == other->get_subprogram_path().at(i); + } + for (int64_t i = 0; bool_ret && i < params_type_.count(); ++i) { + bool_ret = params_type_.at(i) == other->get_params_type().at(i); + } + for (int64_t i = 0; bool_ret && i < nocopy_params_.count(); ++i) { + bool_ret = nocopy_params_.at(i) == other->get_nocopy_params().at(i); + } + for (int64_t i = 0; bool_ret && i < params_name_.count(); ++i) { + bool_ret = params_name_.at(i).compare(other->get_params_name().at(i)) == 0; + } + for (int64_t i = 0; bool_ret && i < params_desc_v2_.count(); ++i) { + bool_ret = params_desc_v2_.at(i) == other->get_params_desc().at(i); } } return bool_ret; diff --git a/src/sql/resolver/expr/ob_raw_expr.h b/src/sql/resolver/expr/ob_raw_expr.h index 620a4f3b92..a278dd67ed 100644 --- a/src/sql/resolver/expr/ob_raw_expr.h +++ b/src/sql/resolver/expr/ob_raw_expr.h @@ -1190,7 +1190,19 @@ struct ObExprEqualCheckContext recursion_level_(0), override_set_op_compare_(false), err_code_(common::OB_SUCCESS), - param_expr_() + param_expr_(), + need_check_deterministic_(false) + { } + ObExprEqualCheckContext(bool need_check_deterministic) + : override_const_compare_(false), + override_column_compare_(false), + override_query_compare_(false), + ignore_implicit_cast_(false), + recursion_level_(0), + override_set_op_compare_(false), + err_code_(common::OB_SUCCESS), + param_expr_(), + need_check_deterministic_(need_check_deterministic) { } virtual ~ObExprEqualCheckContext() {} struct ParamExprPair @@ -1235,6 +1247,7 @@ struct ObExprEqualCheckContext override_set_op_compare_ = false; err_code_ = OB_SUCCESS; param_expr_.reset(); + need_check_deterministic_ = false; } bool override_const_compare_; bool override_column_compare_; @@ -1245,6 +1258,7 @@ struct ObExprEqualCheckContext int err_code_; //when compare with T_QUESTIONMARK, as T_QUESTIONMARK is unkown, record this first. common::ObSEArray param_expr_; + bool need_check_deterministic_; }; struct ObExprParamCheckContext : ObExprEqualCheckContext diff --git a/src/sql/rewrite/ob_stmt_comparer.cpp b/src/sql/rewrite/ob_stmt_comparer.cpp index f1c215d962..0269be2989 100644 --- a/src/sql/rewrite/ob_stmt_comparer.cpp +++ b/src/sql/rewrite/ob_stmt_comparer.cpp @@ -699,7 +699,7 @@ int ObStmtComparer::compute_conditions_map(const ObDMLStmt *first, { int ret = OB_SUCCESS; ObSqlBitSet<> matched_items; - ObStmtCompareContext context; + ObStmtCompareContext context(first, second, map_info, &first->get_query_ctx()->calculable_items_); match_count = 0; if (OB_ISNULL(first) || OB_ISNULL(second) || OB_ISNULL(first->get_query_ctx())) { ret = OB_ERR_UNEXPECTED; @@ -707,7 +707,6 @@ int ObStmtComparer::compute_conditions_map(const ObDMLStmt *first, } else if (OB_FAIL(condition_map.prepare_allocate(first_exprs.count()))) { LOG_WARN("failed to preallocate array", K(ret)); } else { - context.init(first, second, map_info, &first->get_query_ctx()->calculable_items_); for (int64_t i = 0; OB_SUCC(ret) && i < first_exprs.count(); ++i) { bool is_match = false; condition_map.at(i) = OB_INVALID_ID; @@ -894,7 +893,7 @@ int ObStmtComparer::compute_tables_map(const ObDMLStmt *first, { int ret = OB_SUCCESS; ObSqlBitSet<> matched_items; - ObStmtCompareContext context; + ObStmtCompareContext context(first, second, map_info, &first->get_query_ctx()->calculable_items_); match_count = 0; if (OB_ISNULL(first) || OB_ISNULL(second) || OB_ISNULL(first->get_query_ctx())) { ret = OB_ERR_UNEXPECTED; @@ -902,7 +901,6 @@ int ObStmtComparer::compute_tables_map(const ObDMLStmt *first, } else if (OB_FAIL(table_map.prepare_allocate(first_table_ids.count()))) { LOG_WARN("failed to preallocate array", K(ret)); } else { - context.init(first, second, map_info, &first->get_query_ctx()->calculable_items_); for (int64_t i = 0; OB_SUCC(ret) && i < first_table_ids.count(); ++i) { bool is_match = false; table_map.at(i) = OB_INVALID_ID; diff --git a/src/sql/rewrite/ob_stmt_comparer.h b/src/sql/rewrite/ob_stmt_comparer.h index bd8e1b8781..3ef1dc15cd 100644 --- a/src/sql/rewrite/ob_stmt_comparer.h +++ b/src/sql/rewrite/ob_stmt_comparer.h @@ -115,6 +115,46 @@ struct ObStmtCompareContext : ObExprEqualCheckContext outer_(NULL), map_info_(), equal_param_info_() + { + init_override_params(); + } + ObStmtCompareContext(bool need_check_deterministic) : + ObExprEqualCheckContext(need_check_deterministic), + calculable_items_(NULL), + inner_(NULL), + outer_(NULL), + map_info_(), + equal_param_info_() + { + init_override_params(); + } + // for common expression extraction + ObStmtCompareContext(const ObIArray *calculable_items, + bool need_check_deterministic = false) : + ObExprEqualCheckContext(need_check_deterministic), + calculable_items_(calculable_items), + inner_(NULL), + outer_(NULL), + map_info_(), + equal_param_info_() + { + init_override_params(); + } + ObStmtCompareContext(const ObDMLStmt *inner, + const ObDMLStmt *outer, + const ObStmtMapInfo &map_info, + const ObIArray *calculable_items, + bool need_check_deterministic = false) : + ObExprEqualCheckContext(need_check_deterministic), + calculable_items_(calculable_items), + inner_(inner), + outer_(outer), + map_info_(map_info), + equal_param_info_() + { + init_override_params(); + } + inline void init_override_params() { override_column_compare_ = true; override_const_compare_ = true; @@ -123,6 +163,8 @@ struct ObStmtCompareContext : ObExprEqualCheckContext } virtual ~ObStmtCompareContext() {} + // since the init() func only initialize the class members, + // it is better to use constructor // for common expression extraction void init(const ObIArray *calculable_items); diff --git a/src/sql/rewrite/ob_transform_subquery_coalesce.cpp b/src/sql/rewrite/ob_transform_subquery_coalesce.cpp index 1f16681d68..caa716e34e 100644 --- a/src/sql/rewrite/ob_transform_subquery_coalesce.cpp +++ b/src/sql/rewrite/ob_transform_subquery_coalesce.cpp @@ -1350,8 +1350,7 @@ int ObTransformSubqueryCoalesce::transform_or_expr(ObDMLStmt *stmt, ObRawExpr *first_expr_param = NULL; ObQueryRefRawExpr *first_subquery_expr = NULL; ObSEArray subqueries; - ObStmtCompareContext compare_ctx; - compare_ctx.init(&stmt->get_query_ctx()->calculable_items_); + ObStmtCompareContext compare_ctx(&stmt->get_query_ctx()->calculable_items_); //Check whether each independent or condition can pull up related conditions for (int64_t i = 0; OB_SUCC(ret) && can_be_transform && i < expr->get_param_count(); ++i) { ObRawExpr *expr_param = expr->get_param_expr(i); @@ -1837,7 +1836,8 @@ int ObTransformSubqueryCoalesce::coalesce_subquery(StmtCompareHelper &helper, helper.stmt_map_infos_.at(i), select_exprs, index_map, - coalesce_query))) { + coalesce_query, + i == 0))) { LOG_WARN("failed to inner coalesce subquery", K(ret)); } else if (OB_FAIL(append(query_ctx->all_equal_param_constraints_, helper.stmt_map_infos_.at(i).equal_param_map_))) { @@ -1852,10 +1852,10 @@ int ObTransformSubqueryCoalesce::inner_coalesce_subquery(ObSelectStmt *subquery, ObStmtMapInfo &map_info, ObIArray &select_exprs, ObIArray &index_map, - ObSelectStmt *coalesce_query) + ObSelectStmt *coalesce_query, + const bool is_first_subquery) { int ret = OB_SUCCESS; - ObStmtCompareContext context; //select items in subquery ObSEArray subquery_select_list; //select items in coalesce query @@ -1867,12 +1867,20 @@ int ObTransformSubqueryCoalesce::inner_coalesce_subquery(ObSelectStmt *subquery, //column items in subquery trans to column items in coalesce query ObSEArray new_column_list; ObSEArray new_column_items; + + // if the select items have same udf, need check if they are deterministic + // eg: select func(c1), func(c1) from t1; + bool need_check_deterministic = true; + ObStmtCompareContext context(coalesce_query, + subquery, + map_info, + &query_ctx->calculable_items_, + need_check_deterministic); if (OB_ISNULL(subquery) || OB_ISNULL(coalesce_query) || OB_ISNULL(ctx_) || OB_ISNULL(ctx_->allocator_) || OB_ISNULL(query_ctx)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("unexpect null param", K(ret)); - } else if (OB_FALSE_IT(context.init(coalesce_query, subquery, map_info, &query_ctx->calculable_items_))) { } else if (OB_FAIL(subquery->get_select_exprs(subquery_select_list))) { LOG_WARN("failed to get select exprs", K(ret)); } else if (OB_FAIL(subquery->get_column_exprs(subquery_column_list))) { @@ -1953,6 +1961,13 @@ int ObTransformSubqueryCoalesce::inner_coalesce_subquery(ObSelectStmt *subquery, LOG_WARN("unexpect null select expr", K(ret)); } else if (!coalesce_select->same_as(*subquery_select, &context)) { // do nothing + } else if (!is_first_subquery && + !coalesce_select->is_column_ref_expr() && + !coalesce_select->is_const_expr() && + OB_FAIL(ObTransformUtils::create_select_item(*ctx_->allocator_, + coalesce_select, + coalesce_query))) { + LOG_WARN("failed to create column for subquery", K(ret)); } else if (OB_FAIL(index_map.push_back(coalesce_query->get_select_item_size() - 1))) { LOG_WARN("failed to push back index", K(ret)); } else { diff --git a/src/sql/rewrite/ob_transform_subquery_coalesce.h b/src/sql/rewrite/ob_transform_subquery_coalesce.h index b3986b5974..9d4e139c07 100644 --- a/src/sql/rewrite/ob_transform_subquery_coalesce.h +++ b/src/sql/rewrite/ob_transform_subquery_coalesce.h @@ -168,7 +168,8 @@ private: ObStmtMapInfo &map_info, ObIArray &select_exprs, ObIArray &index_map, - ObSelectStmt *coalesce_query); + ObSelectStmt *coalesce_query, + const bool is_first_subquery); int get_map_table_id(ObSelectStmt *subquery, ObSelectStmt *coalesce_subquery,