From 17564346242fdc2cf1481a54adcf78e0e5501803 Mon Sep 17 00:00:00 2001 From: JinmaoLi Date: Mon, 17 Jun 2024 12:48:43 +0000 Subject: [PATCH] [CP] fix deduce NOT NULL attribute by aggr filter incorrectly --- .../rewrite/ob_transform_simplify_expr.cpp | 12 +++-- src/sql/rewrite/ob_transform_utils.cpp | 44 +++++++++++++++++-- src/sql/rewrite/ob_transform_utils.h | 4 ++ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/sql/rewrite/ob_transform_simplify_expr.cpp b/src/sql/rewrite/ob_transform_simplify_expr.cpp index 60f6e134e..9c582e06f 100644 --- a/src/sql/rewrite/ob_transform_simplify_expr.cpp +++ b/src/sql/rewrite/ob_transform_simplify_expr.cpp @@ -268,12 +268,14 @@ int ObTransformSimplifyExpr::replace_is_null_condition(ObDMLStmt *stmt, bool &tr } ObSelectStmt *sel_stmt = static_cast(stmt); for (int64_t i = 0; OB_SUCC(ret) && i < sel_stmt->get_having_expr_size(); ++i) { - if (OB_FAIL(not_null_ctx.remove_having_filter(sel_stmt->get_having_exprs().at(i)))){ + bool exist_in_ctx = ObOptimizerUtil::find_item(not_null_ctx.having_filters_, + sel_stmt->get_having_exprs().at(i)); + if (exist_in_ctx && OB_FAIL(not_null_ctx.remove_having_filter(sel_stmt->get_having_exprs().at(i)))){ LOG_WARN("failed to remove filter", K(ret)); } else if (OB_FAIL(inner_replace_is_null_condition( sel_stmt, sel_stmt->get_having_exprs().at(i), not_null_ctx, is_happened))) { LOG_WARN("failed to replace is null expr", K(ret)); - } else if (OB_FAIL(not_null_ctx.add_having_filter(sel_stmt->get_having_exprs().at(i)))) { + } else if (exist_in_ctx && OB_FAIL(not_null_ctx.add_having_filter(sel_stmt->get_having_exprs().at(i)))) { LOG_WARN("failed to add filter", K(ret)); } else { trans_happened |= is_happened; @@ -1309,7 +1311,9 @@ int ObTransformSimplifyExpr::remove_dummy_nvl(ObDMLStmt *stmt, if (OB_SUCC(ret) && stmt->is_select_stmt() && !static_cast(stmt)->is_scala_group_by()) { ObSelectStmt *sel_stmt = static_cast(stmt); for (int64_t i = 0; OB_SUCC(ret) && i < sel_stmt->get_having_expr_size(); ++i) { - if (OB_FAIL(not_null_ctx.remove_having_filter(sel_stmt->get_having_exprs().at(i)))){ + bool exist_in_ctx = ObOptimizerUtil::find_item(not_null_ctx.having_filters_, + sel_stmt->get_having_exprs().at(i)); + if (exist_in_ctx && OB_FAIL(not_null_ctx.remove_having_filter(sel_stmt->get_having_exprs().at(i)))){ LOG_WARN("failed to remove filter", K(ret)); } else if (OB_FAIL(inner_remove_dummy_nvl(stmt, sel_stmt->get_having_exprs().at(i), @@ -1317,7 +1321,7 @@ int ObTransformSimplifyExpr::remove_dummy_nvl(ObDMLStmt *stmt, ignore_exprs, trans_happened))) { LOG_WARN("failed to remove dummy nvl", K(ret)); - } else if (OB_FAIL(not_null_ctx.add_having_filter(sel_stmt->get_having_exprs().at(i)))) { + } else if (exist_in_ctx && OB_FAIL(not_null_ctx.add_having_filter(sel_stmt->get_having_exprs().at(i)))) { LOG_WARN("failed to add filter", K(ret)); } } diff --git a/src/sql/rewrite/ob_transform_utils.cpp b/src/sql/rewrite/ob_transform_utils.cpp index 75342f7e9..381b30196 100644 --- a/src/sql/rewrite/ob_transform_utils.cpp +++ b/src/sql/rewrite/ob_transform_utils.cpp @@ -2065,12 +2065,48 @@ int ObNotNullContext::generate_stmt_context(int64_t stmt_context) } } - if (OB_SUCC(ret) && stmt_context >= NULLABLE_SCOPE::NS_TOP && - stmt->is_select_stmt()) { - if (OB_FAIL(having_filters_.assign( - static_cast(stmt)->get_having_exprs()))) { + if (OB_SUCC(ret) && stmt_context >= NULLABLE_SCOPE::NS_TOP && stmt->is_select_stmt()) { + // Exprs in "in_group_clause" mean different things when used in basic expressions with added NULLs versus in aggregate functions without NULLs. + // Thus, aggregate functions with such group exprs cannot be used to infer a not-null attribute. + // e.g. select c1, max(c2) from t1 group by rollup(c1) having max(c1) > 0 and nvl(c1, 0) is null); + // can't deduce `c1 is not null` by `max(c1) > 0` to simplify nvl(c1, 0) + const ObSelectStmt *sel_stmt = static_cast(stmt); + if (OB_FAIL(ObTransformUtils::get_having_filters_for_deduce(sel_stmt, + sel_stmt->get_having_exprs(), + group_clause_exprs_, + having_filters_))) { + LOG_WARN("failed to get having filters for deduce", K(ret)); + } + } + return ret; +} + +int ObTransformUtils::get_having_filters_for_deduce(const ObSelectStmt *sel_stmt, + const ObIArray &raw_having_exprs, + const ObIArray &group_clause_exprs, + ObIArray &having_exprs_for_deduce) +{ + int ret = OB_SUCCESS; + bool has_target = false; + if (OB_ISNULL(sel_stmt)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("get unexpected null", K(ret)); + } else if (sel_stmt->get_rollup_expr_size() == 0 && sel_stmt->get_cube_items_size() == 0 && + sel_stmt->get_grouping_sets_items_size() == 0) { + if (OB_FAIL(having_exprs_for_deduce.assign(raw_having_exprs))) { LOG_WARN("failed to assign having exprs", K(ret)); } + } else { + for (int64_t i = 0; OB_SUCC(ret) && i < raw_having_exprs.count(); ++i) { + if (OB_ISNULL(raw_having_exprs.at(i))) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("get unexpected null", K(ret)); + } else if (raw_having_exprs.at(i)->has_flag(CNT_AGG)) { + // do nothing + } else if (OB_FAIL(having_exprs_for_deduce.push_back(raw_having_exprs.at(i)))) { + LOG_WARN("failed to push back having expr", K(ret)); + } + } } return ret; } diff --git a/src/sql/rewrite/ob_transform_utils.h b/src/sql/rewrite/ob_transform_utils.h index 946506b95..188b69388 100644 --- a/src/sql/rewrite/ob_transform_utils.h +++ b/src/sql/rewrite/ob_transform_utils.h @@ -1955,6 +1955,10 @@ public: ObRawExpr *expr, bool &eq_zero, ObIArray &constraints); + static int get_having_filters_for_deduce(const ObSelectStmt* sel_stmt, + const ObIArray &raw_having_exprs, + const ObIArray &group_clause_exprs, + ObIArray &having_exprs_for_deduce); private: static int inner_get_lazy_left_join(ObDMLStmt *stmt, TableItem *table,