From fd436db3f80299c9180e15c41712bdb5168a434e Mon Sep 17 00:00:00 2001 From: st0 Date: Wed, 23 Mar 2022 14:01:09 +0800 Subject: [PATCH] fix nullif/least calc_resultN bug in static engine --- .../ob_static_engine_expr_cg.cpp | 52 ++++++------------- .../code_generator/ob_static_engine_expr_cg.h | 2 +- .../engine/expr/ob_expr_from_unix_time.cpp | 3 -- src/sql/engine/expr/ob_expr_greatest.cpp | 12 ++--- src/sql/engine/expr/ob_expr_greatest.h | 2 - src/sql/engine/expr/ob_expr_least.cpp | 16 +++--- src/sql/engine/expr/ob_expr_least.h | 2 - src/sql/engine/expr/ob_expr_nullif.cpp | 6 +-- src/sql/engine/expr/ob_expr_operator.cpp | 13 ++++- src/sql/engine/expr/ob_expr_operator.h | 3 +- .../expr/ob_raw_expr_wrap_enum_set.cpp | 8 +-- 11 files changed, 49 insertions(+), 70 deletions(-) diff --git a/src/sql/code_generator/ob_static_engine_expr_cg.cpp b/src/sql/code_generator/ob_static_engine_expr_cg.cpp index a8540c1e5f..d088111176 100644 --- a/src/sql/code_generator/ob_static_engine_expr_cg.cpp +++ b/src/sql/code_generator/ob_static_engine_expr_cg.cpp @@ -972,47 +972,21 @@ int ObStaticEngineExprCG::add_so_check_expr_above(ObIArray& exprs, ObExp return ret; } -int ObStaticEngineExprCG::replace_var_rt_expr(ObExpr* origin_expr, const ObRawExpr *origin_raw_expr, +int ObStaticEngineExprCG::replace_var_rt_expr(ObExpr* origin_expr, ObExpr* var_expr, ObExpr* parent_expr, int32_t var_idx) // child pos of parent_expr { int ret = OB_SUCCESS; if (OB_ISNULL(origin_expr)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("expr is null", K(ret), K(origin_expr)); - } else { - while (OB_SUCC(ret)) { - if (OB_ISNULL(origin_raw_expr)) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("origin raw expr is null", K(ret)); - // from_unixtime may add implicit cast above origin param expr, find origin param. - } else if (T_FUN_SYS_CAST == origin_expr->type_ - && CM_IS_IMPLICIT_CAST(origin_raw_expr->get_extra())) { - if (OB_UNLIKELY(origin_expr->arg_cnt_ < 1) || OB_ISNULL(origin_expr->args_[0])) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("invalid param", KPC(origin_expr)); - } else { - origin_expr = origin_expr->args_[0]; - origin_raw_expr = origin_raw_expr->get_param_expr(0); - } - } else if (T_FUN_ENUM_TO_STR == origin_expr->type_ || T_FUN_SET_TO_STR == origin_expr->type_) { - if (OB_UNLIKELY(origin_expr->arg_cnt_ < 2) || OB_ISNULL(origin_expr->args_[1])) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("invalid param", K(ret), KPC(origin_expr)); - } else { - origin_expr = origin_expr->args_[1]; - origin_raw_expr = origin_raw_expr->get_param_expr(1); - } - } else { - break; - } - } - } - - if (OB_FAIL(ret)) { } else if (T_EXEC_VAR == var_expr->type_) { if (OB_ISNULL(parent_expr) || OB_UNLIKELY(parent_expr->arg_cnt_ < var_idx + 1)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("invalid param", KPC(parent_expr)); + } else if (OB_UNLIKELY(var_expr->datum_meta_.type_ != origin_expr->datum_meta_.type_ + || var_expr->datum_meta_.cs_type_ != origin_expr->datum_meta_.cs_type_)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("exec var meta diff from origin expr meta", K(ret), KPC(origin_expr), KPC(var_expr)); } else { parent_expr->args_[var_idx] = origin_expr; } @@ -1022,13 +996,15 @@ int ObStaticEngineExprCG::replace_var_rt_expr(ObExpr* origin_expr, const ObRawEx // May two exprs above ObVarRawExpr: // 1. implicit cast add in type deduce // 2. ENUM_TO_STR/SET_TO_STR for enum/set - if (T_FUN_ENUM_TO_STR == parent_expr->type_ || T_FUN_SET_TO_STR == parent_expr->type_) { + if (T_FUN_ENUM_TO_STR == parent_expr->type_ + || T_FUN_SET_TO_STR == parent_expr->type_ + || T_FUN_ENUM_TO_INNER_TYPE == parent_expr->type_ + || T_FUN_SET_TO_INNER_TYPE == parent_expr->type_) { var_idx = 1; } else if (T_FUN_SYS_CAST == parent_expr->type_) { var_idx = 0; } else { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("unexpected expr added above var expr", K(ret), K(parent_expr->type_)); + break; } if (OB_FAIL(ret)) { } else if (OB_UNLIKELY(parent_expr->arg_cnt_ < var_idx + 1) || @@ -1036,7 +1012,13 @@ int ObStaticEngineExprCG::replace_var_rt_expr(ObExpr* origin_expr, const ObRawEx ret = OB_ERR_UNEXPECTED; LOG_WARN("expr is null", K(ret), KPC(parent_expr)); } else if (T_EXEC_VAR == var_expr->type_) { - parent_expr->args_[var_idx] = origin_expr; + if (OB_UNLIKELY(var_expr->datum_meta_.type_ != origin_expr->datum_meta_.type_ + || var_expr->datum_meta_.cs_type_ != origin_expr->datum_meta_.cs_type_)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("exec var meta diff from origin expr meta", K(ret), KPC(origin_expr), KPC(var_expr)); + } else { + parent_expr->args_[var_idx] = origin_expr; + } break; } else { parent_expr = var_expr; diff --git a/src/sql/code_generator/ob_static_engine_expr_cg.h b/src/sql/code_generator/ob_static_engine_expr_cg.h index b05fa38fca..4d8a6fd41c 100644 --- a/src/sql/code_generator/ob_static_engine_expr_cg.h +++ b/src/sql/code_generator/ob_static_engine_expr_cg.h @@ -63,7 +63,7 @@ public: static int generate_rt_expr(const ObRawExpr& src, common::ObIArray& exprs, ObExpr*& dst); - static int replace_var_rt_expr(ObExpr* origin_expr, const ObRawExpr *origin_raw_expr, ObExpr* var_expr, + static int replace_var_rt_expr(ObExpr* origin_expr, ObExpr* var_expr, ObExpr* parent_expr, int32_t var_idx); // Attention : Please think over before you have to use this function. diff --git a/src/sql/engine/expr/ob_expr_from_unix_time.cpp b/src/sql/engine/expr/ob_expr_from_unix_time.cpp index c2aa4a0832..7cfca4a8fb 100644 --- a/src/sql/engine/expr/ob_expr_from_unix_time.cpp +++ b/src/sql/engine/expr/ob_expr_from_unix_time.cpp @@ -212,9 +212,6 @@ int ObExprFromUnixTime::cg_expr(ObExprCGCtx& op_cg_ctx, const ObRawExpr& raw_exp if (OB_ISNULL(rt_expr.args_[0]) || OB_ISNULL(rt_expr.args_[1]) || OB_ISNULL(rt_expr.args_[2])) { ret = OB_INVALID_ARGUMENT; LOG_WARN("invalid null args", K(ret), K(rt_expr.args_[0]), K(rt_expr.args_[1]), K(rt_expr.args_[2])); - } else if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr(rt_expr.args_[1], raw_expr.get_param_expr(1), - rt_expr.args_[2], &rt_expr, 2))) { - LOG_WARN("replace var rt expr failed", K(ret), K(rt_expr)); } else if (ob_is_string_tc(rt_expr.args_[2]->datum_meta_.type_)) { rt_expr.eval_func_ = &eval_fromtime_normal; } else { diff --git a/src/sql/engine/expr/ob_expr_greatest.cpp b/src/sql/engine/expr/ob_expr_greatest.cpp index 30fd4a63fc..0c96b1166f 100644 --- a/src/sql/engine/expr/ob_expr_greatest.cpp +++ b/src/sql/engine/expr/ob_expr_greatest.cpp @@ -47,13 +47,7 @@ int ObExprBaseGreatest::calc_result_typeN( int ObExprBaseGreatest::calc_resultN( ObObj& result, const ObObj* objs_stack, int64_t param_num, ObExprCtx& expr_ctx) const { - return ObMinMaxExprOperator::calc_(result, objs_stack, param_num, result_type_, expr_ctx, CO_GT, need_cast_); -} - -int ObExprBaseGreatest::calc( - ObObj& result, const ObObj* objs_stack, int64_t param_num, const ObExprResType& result_type, ObExprCtx& expr_ctx) -{ - return ObMinMaxExprOperator::calc_(result, objs_stack, param_num, result_type, expr_ctx, CO_GT, true); + return ObMinMaxExprOperator::calc_(result, objs_stack, param_num, result_type_, expr_ctx, CO_GT, need_cast_, get_type()); } ObExprGreatestMySQL::ObExprGreatestMySQL(ObIAllocator& alloc) : ObExprBaseGreatest(alloc, MORE_THAN_ONE) @@ -124,11 +118,11 @@ int ObExprBaseGreatest::cg_expr(ObExprCGCtx& op_cg_ctx, const ObRawExpr& raw_exp const uint32_t real_param_num = param_num / 3; for (int64_t i = 0; OB_SUCC(ret) && i < real_param_num; i++) { if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr( - rt_expr.args_[i], raw_expr.get_param_expr(i), rt_expr.args_[i + real_param_num], + rt_expr.args_[i], rt_expr.args_[i + real_param_num], &rt_expr, i + real_param_num))) { LOG_WARN("replace var rt expr failed", K(ret)); } else if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr( - rt_expr.args_[i], raw_expr.get_param_expr(i), rt_expr.args_[i + 2 * real_param_num], + rt_expr.args_[i], rt_expr.args_[i + 2 * real_param_num], &rt_expr, i + 2 * real_param_num))) { LOG_WARN("replace var rt expr failed", K(ret)); } diff --git a/src/sql/engine/expr/ob_expr_greatest.h b/src/sql/engine/expr/ob_expr_greatest.h index 1da1cd903f..300d1bc28e 100644 --- a/src/sql/engine/expr/ob_expr_greatest.h +++ b/src/sql/engine/expr/ob_expr_greatest.h @@ -27,8 +27,6 @@ public: ObExprResType& type, ObExprResType* types_stack, int64_t param_num, common::ObExprTypeCtx& type_ctx) const; virtual int calc_resultN( common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, common::ObExprCtx& expr_ctx) const; - static int calc(common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, - const ObExprResType& expected_type, common::ObExprCtx& expr_ctx); virtual int cg_expr(ObExprCGCtx& op_cg_ctx, const ObRawExpr& raw_expr, ObExpr& rt_expr) const override; static int calc_greatest(const ObExpr& expr, ObEvalCtx& ctx, ObDatum& expr_datum); diff --git a/src/sql/engine/expr/ob_expr_least.cpp b/src/sql/engine/expr/ob_expr_least.cpp index 12247530d4..d66a8c9377 100644 --- a/src/sql/engine/expr/ob_expr_least.cpp +++ b/src/sql/engine/expr/ob_expr_least.cpp @@ -150,6 +150,10 @@ int ObExprBaseLeastGreatest::calc_result_typeN_mysql( if (OB_FAIL(calc_result_meta_for_comparison( type, types, real_param_num, type_ctx.get_coll_type(), default_length_semantics))) { LOG_WARN("calc result meta for comparison failed"); + } + // can't cast origin parameters. + for (int64_t i = 0; i < real_param_num; i++) { + types[i].set_calc_meta(types[i].get_obj_meta()); } if (!all_integer || !type.is_integer_type()) { // compatible with MySQL. compare type and result type may be different. @@ -284,13 +288,7 @@ int ObExprBaseLeast::calc_result_typeN( int ObExprBaseLeast::calc_resultN(ObObj& result, const ObObj* objs_stack, int64_t param_num, ObExprCtx& expr_ctx) const { - return ObMinMaxExprOperator::calc_(result, objs_stack, param_num, result_type_, expr_ctx, CO_LT, need_cast_); -} - -int ObExprBaseLeast::calc( - ObObj& result, const ObObj* objs_stack, int64_t param_num, const ObExprResType& expected_type, ObExprCtx& expr_ctx) -{ - return ObMinMaxExprOperator::calc_(result, objs_stack, param_num, expected_type, expr_ctx, CO_LT, true); + return ObMinMaxExprOperator::calc_(result, objs_stack, param_num, result_type_, expr_ctx, CO_LT, need_cast_, get_type()); } ObExprLeastMySQL::ObExprLeastMySQL(ObIAllocator& alloc) : ObExprBaseLeast(alloc, MORE_THAN_ONE) @@ -350,10 +348,10 @@ int ObExprBaseLeast::cg_expr(ObExprCGCtx& op_cg_ctx, const ObRawExpr& raw_expr, const uint32_t real_param_num = param_num / 3; for (int64_t i = 0; OB_SUCC(ret) && i < real_param_num; i++) { if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr( - rt_expr.args_[i], raw_expr.get_param_expr(i), rt_expr.args_[i + real_param_num], &rt_expr, i + real_param_num))) { + rt_expr.args_[i], rt_expr.args_[i + real_param_num], &rt_expr, i + real_param_num))) { LOG_WARN("replace var rt expr failed", K(ret)); } else if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr( - rt_expr.args_[i], raw_expr.get_param_expr(i), rt_expr.args_[i + 2 * real_param_num], &rt_expr, i + 2 * real_param_num))) { + rt_expr.args_[i], rt_expr.args_[i + 2 * real_param_num], &rt_expr, i + 2 * real_param_num))) { LOG_WARN("replace var rt expr failed", K(ret)); } } diff --git a/src/sql/engine/expr/ob_expr_least.h b/src/sql/engine/expr/ob_expr_least.h index deaf2f747f..551704e4a5 100644 --- a/src/sql/engine/expr/ob_expr_least.h +++ b/src/sql/engine/expr/ob_expr_least.h @@ -64,8 +64,6 @@ public: ObExprResType& type, ObExprResType* types_stack, int64_t param_num, common::ObExprTypeCtx& type_ctx) const override; virtual int calc_resultN( common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, common::ObExprCtx& expr_ctx) const override; - static int calc(common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, - const ObExprResType& expected_type, common::ObExprCtx& expr_ctx); virtual int cg_expr(ObExprCGCtx& op_cg_ctx, const ObRawExpr& raw_expr, ObExpr& rt_expr) const override; static int calc_least(const ObExpr& expr, ObEvalCtx& ctx, ObDatum& expr_datum); diff --git a/src/sql/engine/expr/ob_expr_nullif.cpp b/src/sql/engine/expr/ob_expr_nullif.cpp index 65d1b6fa3b..159c651396 100644 --- a/src/sql/engine/expr/ob_expr_nullif.cpp +++ b/src/sql/engine/expr/ob_expr_nullif.cpp @@ -149,7 +149,7 @@ int ObExprNullif::calc_resultN(ObObj& result, const ObObj* objs_stack, int64_t p EXPR_DEFINE_CMP_CTX(result_type_.get_calc_meta(), true, expr_ctx); EXPR_DEFINE_CAST_CTX(expr_ctx, CM_NONE); ObObj cmp; - if (OB_UNLIKELY(2 != param_num) || OB_ISNULL(objs_stack)) { + if (OB_UNLIKELY(2 != param_num && 6 != param_num) || OB_ISNULL(objs_stack)) { ret = OB_INVALID_ARGUMENT; LOG_WARN("invalid argument", K(ret), K(param_num), K(objs_stack)); } else { @@ -229,10 +229,10 @@ int ObExprNullif::cg_expr(ObExprCGCtx& expr_cg_ctx, const ObRawExpr& raw_expr, O const uint32_t real_param_num = param_num / 3; for (int64_t i = 0; OB_SUCC(ret) && i < real_param_num; i++) { if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr( - rt_expr.args_[i], raw_expr.get_param_expr(i), rt_expr.args_[i + real_param_num], &rt_expr, i + real_param_num))) { + rt_expr.args_[i], rt_expr.args_[i + real_param_num], &rt_expr, i + real_param_num))) { LOG_WARN("replace var rt expr failed", K(ret)); } else if (OB_FAIL(ObStaticEngineExprCG::replace_var_rt_expr( - rt_expr.args_[i], raw_expr.get_param_expr(i), rt_expr.args_[i + 2 * real_param_num], &rt_expr, i + 2 * real_param_num))) { + rt_expr.args_[i], rt_expr.args_[i + 2 * real_param_num], &rt_expr, i + 2 * real_param_num))) { LOG_WARN("replace var rt expr failed", K(ret)); } } diff --git a/src/sql/engine/expr/ob_expr_operator.cpp b/src/sql/engine/expr/ob_expr_operator.cpp index e5dd47ba6c..47a644b295 100644 --- a/src/sql/engine/expr/ob_expr_operator.cpp +++ b/src/sql/engine/expr/ob_expr_operator.cpp @@ -3812,13 +3812,22 @@ int ObMinMaxExprOperator::calc_result_meta_for_comparison(ObExprResType& type, O } int ObMinMaxExprOperator::calc_(ObObj& result, const ObObj* objs_stack, int64_t param_num, - const ObExprResType& result_type, ObExprCtx& expr_ctx, ObCmpOp cmp_op, bool need_cast) + const ObExprResType& result_type, ObExprCtx& expr_ctx, ObCmpOp cmp_op, bool need_cast, ObExprOperatorType expr_type) { // todo: // we assume that result type / result collation / compare type / compare collation are CORRECT. // but who knowns? we should check it ASAP. int ret = OB_SUCCESS; - if (OB_UNLIKELY(need_cast)) { + if (T_FUN_SYS_LEAST_INNER == expr_type || T_FUN_SYS_GREATEST_INNER == expr_type) { + if (0 != param_num % 3) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("unexpected param num", K(ret), K(param_num)); + } else { + param_num = param_num / 3; + } + } + if (OB_FAIL(ret)) { + } else if (OB_UNLIKELY(need_cast)) { ret = calc_with_cast(result, objs_stack, param_num, result_type, expr_ctx, cmp_op); } else { ret = calc_without_cast(result, objs_stack, param_num, result_type, expr_ctx, cmp_op); diff --git a/src/sql/engine/expr/ob_expr_operator.h b/src/sql/engine/expr/ob_expr_operator.h index 17031b5e0f..7d7b86d665 100644 --- a/src/sql/engine/expr/ob_expr_operator.h +++ b/src/sql/engine/expr/ob_expr_operator.h @@ -1554,7 +1554,8 @@ protected: // least should set cmp_op to CO_LT. // greatest should set cmp_op to CO_GT. static int calc_(common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, - const ObExprResType& result_type, common::ObExprCtx& expr_ctx, common::ObCmpOp cmp_op, bool need_cast); + const ObExprResType& result_type, common::ObExprCtx& expr_ctx, common::ObCmpOp cmp_op, bool need_cast, + ObExprOperatorType expr_type); OB_INLINE static int calc_without_cast(common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, const ObExprResType& result_type, common::ObExprCtx& expr_ctx, common::ObCmpOp cmp_op); OB_INLINE static int calc_with_cast(common::ObObj& result, const common::ObObj* objs_stack, int64_t param_num, diff --git a/src/sql/resolver/expr/ob_raw_expr_wrap_enum_set.cpp b/src/sql/resolver/expr/ob_raw_expr_wrap_enum_set.cpp index 5b5873f4bd..bf20090683 100644 --- a/src/sql/resolver/expr/ob_raw_expr_wrap_enum_set.cpp +++ b/src/sql/resolver/expr/ob_raw_expr_wrap_enum_set.cpp @@ -767,14 +767,16 @@ int ObRawExprWrapEnumSet::wrap_nullif_expr(ObSysFunRawExpr& expr) int ret = OB_SUCCESS; int64_t param_count = expr.get_param_count(); int64_t input_types_count = expr.get_input_types().count(); + bool param_copied = false; if (OB_UNLIKELY(OB_ISNULL(my_session_))) { ret = OB_NOT_INIT; LOG_WARN("session is null", K(ret)); } else if (OB_UNLIKELY(T_FUN_SYS_NULLIF != expr.get_expr_type())) { ret = OB_INVALID_ARGUMENT; LOG_WARN("invalid argument", K(expr), K(ret)); - } else if (OB_UNLIKELY((my_session_->use_static_typing_engine() && 6 != param_count) || - (!my_session_->use_static_typing_engine() && 2 != param_count))) { + } else if (FALSE_IT(param_copied = my_session_->use_static_typing_engine() && !is_oracle_mode())) { + } else if (OB_UNLIKELY((param_copied && 6 != param_count) + || (!param_copied && 2 != param_count))) { // For the new engine, due to the copy of the nullif parameter, // there are actually 3 sets of parameters: the original input parameter, // the calc type parameter and the result parameter, so there are 6 in total @@ -784,7 +786,7 @@ int ObRawExprWrapEnumSet::wrap_nullif_expr(ObSysFunRawExpr& expr) ret = OB_ERR_UNEXPECTED; LOG_WARN("param_count is not equal to input_types_count", K(param_count), K(input_types_count), K(ret)); } else { - for (int i = 0; i < param_count / 2; ++i) { + for (int i = 0; i < param_count && OB_SUCC(ret); i += 2) { ObRawExpr* left_param = expr.get_param_expr(i); ObRawExpr* right_param = expr.get_param_expr(i + 1); if (OB_ISNULL(left_param) || OB_ISNULL(right_param)) {