From 60617e80c3d9bd21d528de45d46351418313a726 Mon Sep 17 00:00:00 2001 From: obdev Date: Thu, 8 Feb 2024 23:25:32 +0000 Subject: [PATCH] [CP] fix check constraint bugs --- .../rewrite/ob_transform_const_propagate.cpp | 95 +++++++++++++++++-- .../rewrite/ob_transform_const_propagate.h | 9 +- .../ob_transform_predicate_move_around.cpp | 10 +- 3 files changed, 98 insertions(+), 16 deletions(-) diff --git a/src/sql/rewrite/ob_transform_const_propagate.cpp b/src/sql/rewrite/ob_transform_const_propagate.cpp index 00cf629284..c622417023 100644 --- a/src/sql/rewrite/ob_transform_const_propagate.cpp +++ b/src/sql/rewrite/ob_transform_const_propagate.cpp @@ -2037,10 +2037,9 @@ int ObTransformConstPropagate::replace_check_constraint_exprs(ObDMLStmt *stmt, part_column_expr, old_column_exprs, new_const_exprs, - complex_cst_info_idx))) { + complex_cst_info_idx, + trans_happened))) { LOG_WARN("failed to do replace check constraint expr", K(ret)); - } else { - trans_happened = true; } } } @@ -2144,6 +2143,9 @@ int ObTransformConstPropagate::do_check_constraint_param_expr_vaildity( LOG_WARN("failed to check all const propagate column", K(ret)); } else if (!is_valid) { //do nothing + } else if (complex_cst_info_idx >= 0 + && expr_const_infos.at(complex_cst_info_idx).multi_const_exprs_.count() > 10) { + is_valid = false; //do not deduce big in //check rule 3 } else if (OB_FAIL(recursive_check_non_column_param_expr_validity(non_column_param_expr, parent_exprs, @@ -2159,6 +2161,33 @@ int ObTransformConstPropagate::do_check_constraint_param_expr_vaildity( return ret; } +int ObTransformConstPropagate::check_constraint_value_validity(ObRawExpr *value_expr, bool &reject) +{ + int ret = OB_SUCCESS; + reject = false; + ObObj result; + bool got_result = false; + if (OB_ISNULL(value_expr)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("unexpected error", K(ret)); + } else if (OB_FAIL(value_expr->extract_info())) { + LOG_WARN("extract info failed", K(ret)); + } else if (OB_FAIL(ObSQLUtils::calc_const_or_calculable_expr(ctx_->exec_ctx_, + value_expr, + result, + got_result, + *ctx_->allocator_))) { + LOG_WARN("calc const or calculable expr failed", K(ret)); + } else if (!got_result) { + reject = true; + //uncalculable + } else if (result.is_null()) { + reject = true; + //violate null reject, can not add new condition + } + return ret; +} + int ObTransformConstPropagate::check_all_const_propagate_column(ObIArray &column_exprs, ObIArray &expr_const_infos, ObIArray &new_const_exprs, @@ -2238,7 +2267,8 @@ int ObTransformConstPropagate::do_replace_check_constraint_expr(ObDMLStmt *stmt, ObRawExpr *part_column_expr, ObIArray &old_column_exprs, ObIArray &new_const_exprs, - int64_t &complex_cst_info_idx) + int64_t &complex_cst_info_idx, + bool &trans_happened) { int ret = OB_SUCCESS; if (OB_ISNULL(stmt) || OB_ISNULL(check_constraint_expr) || OB_ISNULL(part_column_expr) || @@ -2248,8 +2278,11 @@ int ObTransformConstPropagate::do_replace_check_constraint_expr(ObDMLStmt *stmt, LOG_WARN("get unexpected error", K(ret), K(check_constraint_expr), K(stmt), K(part_column_expr), K(ctx_), K(old_column_exprs), K(new_const_exprs)); } else { + bool reject = false; ObRawExpr *new_check_cst_expr = NULL; + ObRawExpr *value_expr = NULL; ObRawExprCopier copier(*ctx_->expr_factory_); + ObSEArray not_null_values; if (complex_cst_info_idx >= 0) {//need generate in condition for complex const(or/in expr) if (OB_UNLIKELY(complex_cst_info_idx > expr_const_infos.count() || !expr_const_infos.at(complex_cst_info_idx).is_complex_const_info_)) { @@ -2260,8 +2293,11 @@ int ObTransformConstPropagate::do_replace_check_constraint_expr(ObDMLStmt *stmt, part_column_expr, old_column_exprs, new_const_exprs, - new_check_cst_expr))) { + new_check_cst_expr, + not_null_values, + reject))) { LOG_WARN("failed to build new in condition expr", K(ret)); + } else if (reject) { } else { expr_const_infos.at(complex_cst_info_idx).is_used_ = true; } @@ -2271,14 +2307,47 @@ int ObTransformConstPropagate::do_replace_check_constraint_expr(ObDMLStmt *stmt, LOG_WARN("failed to add skipped expr", K(ret)); } else if (OB_FAIL(copier.copy(check_constraint_expr, new_check_cst_expr))) { LOG_WARN("failed to copy expr", K(ret)); + } else if (OB_ISNULL(new_check_cst_expr) || OB_UNLIKELY(new_check_cst_expr->get_param_count() < 2)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("unexpected error", K(ret), KPC(new_check_cst_expr)); + } else if (OB_ISNULL(value_expr = (part_column_expr == new_check_cst_expr->get_param_expr(0) ? + new_check_cst_expr->get_param_expr(1) : + new_check_cst_expr->get_param_expr(0)))) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("value expr is null", K(ret)); + } else if (OB_FAIL(not_null_values.push_back(value_expr))) { + LOG_WARN("push back failed", K(ret)); + } else if (OB_FAIL(check_constraint_value_validity(value_expr, reject))) { + LOG_WARN("ensure check cst failed", K(ret)); } else {/*do nothing*/} - if (OB_SUCC(ret)) { - if (OB_FAIL(batch_mark_expr_const_infos_used(old_column_exprs, expr_const_infos))) { + if (OB_SUCC(ret) && !reject) { + ObRawExpr *or_expr = NULL; + ObRawExpr *part_col_is_null = NULL; + ObSEArray or_expr_children; + if (OB_FAIL(ObRawExprUtils::build_is_not_null_expr(*ctx_->expr_factory_, part_column_expr, false, part_col_is_null))) { + LOG_WARN("build is null failed", K(ret)); + } else if (OB_FAIL(or_expr_children.push_back(part_col_is_null))) { + LOG_WARN("push back failed", K(ret)); + } else if (OB_FAIL(or_expr_children.push_back(new_check_cst_expr))) { + LOG_WARN("push back failed", K(ret)); + } else if (OB_FAIL(ObRawExprUtils::build_or_exprs(*ctx_->expr_factory_, or_expr_children, or_expr))) { + LOG_WARN("build or exprs failed", K(ret)); + } else { + new_check_cst_expr = or_expr; + } + if (OB_FAIL(ret)) { + } else if (OB_FAIL(batch_mark_expr_const_infos_used(old_column_exprs, expr_const_infos))) { LOG_WARN("failed to batch mark_expr_const_infos_used", K(ret)); } else if (OB_FAIL(stmt->get_condition_exprs().push_back(new_check_cst_expr))) { LOG_WARN("failed to push back", K(ret)); } else { + for (int64_t i = 0; OB_SUCC(ret) && i < not_null_values.count(); i++) { + if (OB_FAIL(ObTransformUtils::add_param_not_null_constraint(*ctx_, not_null_values.at(i)))) { + LOG_WARN("add not null cst failed", K(ret)); + } + } + trans_happened = true; LOG_TRACE("Succeed to do replace check constraint expr", KPC(new_check_cst_expr)); } } @@ -2291,7 +2360,9 @@ int ObTransformConstPropagate::build_new_in_condition_expr(ObRawExpr *check_cons ObRawExpr *part_column_expr, ObIArray &old_column_exprs, ObIArray &new_const_exprs, - ObRawExpr *&new_condititon_expr) + ObRawExpr *&new_condititon_expr, + ObIArray ¬_null_values, + bool &reject) { int ret = OB_SUCCESS; new_condititon_expr = NULL; @@ -2327,7 +2398,7 @@ int ObTransformConstPropagate::build_new_in_condition_expr(ObRawExpr *check_cons } else if (OB_FAIL(in_expr->add_param_expr(row_expr))) { LOG_WARN("failed to add param expr", K(ret)); } else { - for (int64_t i = 0; OB_SUCC(ret) && i < expr_const_info.multi_const_exprs_.count(); ++i) { + for (int64_t i = 0; OB_SUCC(ret) && !reject && i < expr_const_info.multi_const_exprs_.count(); ++i) { ObRawExpr *new_param_expr = NULL; ObRawExprCopier copier(*ctx_->expr_factory_); if (OB_FAIL(old_column_exprs.push_back(expr_const_info.column_expr_))) { @@ -2340,12 +2411,16 @@ int ObTransformConstPropagate::build_new_in_condition_expr(ObRawExpr *check_cons LOG_WARN("failed to copy expr", K(ret)); } else if (OB_FAIL(row_expr->add_param_expr(new_param_expr))) { LOG_WARN("failed to add param expr", K(ret)); + } else if (OB_FAIL(not_null_values.push_back(new_param_expr))) { + LOG_WARN("failed to add param expr", K(ret)); + } else if (OB_FAIL(check_constraint_value_validity(new_param_expr, reject))) { + LOG_WARN("ensure check cst failed", K(ret)); } else { old_column_exprs.pop_back(); new_const_exprs.pop_back(); } } - if (OB_SUCC(ret)) { + if (OB_SUCC(ret) && !reject) { if (OB_FAIL(in_expr->formalize(ctx_->session_info_))) { LOG_WARN("failed to formalize", K(ret)); } else { diff --git a/src/sql/rewrite/ob_transform_const_propagate.h b/src/sql/rewrite/ob_transform_const_propagate.h index 5de3c87b44..cbcc97cf36 100644 --- a/src/sql/rewrite/ob_transform_const_propagate.h +++ b/src/sql/rewrite/ob_transform_const_propagate.h @@ -336,14 +336,17 @@ private: ObRawExpr *part_column_expr, ObIArray &old_column_exprs, ObIArray &new_const_exprs, - int64_t &complex_cst_info_idx); + int64_t &complex_cst_info_idx, + bool &trans_happened); int build_new_in_condition_expr(ObRawExpr *check_constraint_expr, ExprConstInfo &expr_const_info, ObRawExpr *part_column_expr, ObIArray &old_column_exprs, ObIArray &new_const_exprs, - ObRawExpr *&new_condititon_expr); + ObRawExpr *&new_condititon_expr, + ObIArray ¬_null_values, + bool &reject); int batch_mark_expr_const_infos_used(ObIArray &column_exprs, ObIArray &expr_const_infos); @@ -359,6 +362,8 @@ private: ObRawExpr *expr, ExprConstInfo &equal_info); + int check_constraint_value_validity(ObRawExpr *value_expr, bool &reject); + private: typedef ObSEArray PullupConstInfos; ObArenaAllocator allocator_; diff --git a/src/sql/rewrite/ob_transform_predicate_move_around.cpp b/src/sql/rewrite/ob_transform_predicate_move_around.cpp index 98360401d0..a4ba3a883b 100644 --- a/src/sql/rewrite/ob_transform_predicate_move_around.cpp +++ b/src/sql/rewrite/ob_transform_predicate_move_around.cpp @@ -371,8 +371,8 @@ int ObTransformPredicateMoveAround::pullup_predicates(ObDMLStmt *stmt, if (OB_FAIL(pullup_predicates_from_set_stmt(stmt, sel_ids, output_pullup_preds))) { LOG_WARN("process set stmt failed", K(ret)); } - } else if (OB_FAIL(generate_basic_table_pullup_preds(stmt, *input_pullup_preds))) { - LOG_WARN("add stmt check constraints", K(ret)); + // } else if (OB_FAIL(generate_basic_table_pullup_preds(stmt, *input_pullup_preds))) { + // LOG_WARN("add stmt check constraints", K(ret)); } else if (OB_FAIL(pullup_predicates_from_view(*stmt, sel_ids, *input_pullup_preds))) { LOG_WARN("failed to pullup predicates from view", K(ret)); } else if (OB_FAIL(generate_pullup_predicates_for_subquery(*stmt, *input_pullup_preds))) { @@ -599,6 +599,9 @@ int ObTransformPredicateMoveAround::generate_pullup_predicates_for_subquery(ObDM return ret; } +//Be careful, this function may cause bugs. +//it should make sure that the check constraint result is not null or calculable, but now we can not guarantee it. +//may reopen it in future by case. int ObTransformPredicateMoveAround::generate_basic_table_pullup_preds(ObDMLStmt *stmt, ObIArray &preds) { int ret = OB_SUCCESS; @@ -1099,8 +1102,7 @@ int ObTransformPredicateMoveAround::recursive_check_expr_pullup_validity( for (int64_t i = 0; OB_SUCC(ret) && state >= 0 && i < expr->get_param_count(); ++i) { if (OB_FAIL(parent_exprs.push_back(expr))) { LOG_WARN("failed to push back", K(ret)); - } - if (OB_FAIL(SMART_CALL(recursive_check_expr_pullup_validity(expr->get_param_expr(i), + } else if (OB_FAIL(SMART_CALL(recursive_check_expr_pullup_validity(expr->get_param_expr(i), pullup_list, parent_exprs, state)))) {