From 25510c8333be998e820677c749a4c25112ba913e Mon Sep 17 00:00:00 2001 From: zs0 Date: Thu, 10 Mar 2022 17:55:40 +0800 Subject: [PATCH] fix bug: create fd use invalid unique index --- src/sql/optimizer/ob_logical_operator.h | 9 +- .../expr/ob_raw_expr_canonicalizer_impl.cpp | 2 + src/sql/rewrite/ob_transform_utils.cpp | 142 +++++++++--------- src/sql/rewrite/ob_transform_utils.h | 7 +- .../ob_transform_where_subquery_pullup.cpp | 13 +- .../expr/test_raw_expr_canonicalizer.result | 28 ++-- 6 files changed, 113 insertions(+), 88 deletions(-) diff --git a/src/sql/optimizer/ob_logical_operator.h b/src/sql/optimizer/ob_logical_operator.h index a66ed7aac3..378a9b6313 100644 --- a/src/sql/optimizer/ob_logical_operator.h +++ b/src/sql/optimizer/ob_logical_operator.h @@ -261,9 +261,12 @@ static const char OPCOST[] = "OP_COST"; * these operator never generate expr */ -#define IS_EXPR_PASSBY_OPER(type) \ - (log_op_def::LOG_GRANULE_ITERATOR == (type) || log_op_def::LOG_LINK == (type) || \ - log_op_def::LOG_EXCHANGE == (type) || log_op_def::LOG_MONITORING_DUMP == (type)) +#define IS_EXPR_PASSBY_OPER(type) (log_op_def::LOG_GRANULE_ITERATOR == (type) \ + || log_op_def::LOG_LINK == (type) \ + || log_op_def::LOG_EXCHANGE == (type) \ + || log_op_def::LOG_MONITORING_DUMP == (type) \ + || log_op_def::LOG_JOIN_FILTER == (type)) + struct FilterCompare { FilterCompare(common::ObIArray& predicate_selectivities) diff --git a/src/sql/resolver/expr/ob_raw_expr_canonicalizer_impl.cpp b/src/sql/resolver/expr/ob_raw_expr_canonicalizer_impl.cpp index 90dc83fbff..6fc2e5ab72 100644 --- a/src/sql/resolver/expr/ob_raw_expr_canonicalizer_impl.cpp +++ b/src/sql/resolver/expr/ob_raw_expr_canonicalizer_impl.cpp @@ -33,6 +33,8 @@ int ObRawExprCanonicalizerImpl::canonicalize(ObRawExpr*& expr) LOG_WARN("cluster and or failed", K(ret)); } else if (OB_FAIL(pull_similar_expr(expr))) { LOG_WARN("pull similar expr failed", K(ret)); + } else if (OB_FAIL(expr->extract_info())) { + LOG_WARN("failed to extract info", K(ret), K(*expr)); } return ret; } diff --git a/src/sql/rewrite/ob_transform_utils.cpp b/src/sql/rewrite/ob_transform_utils.cpp index 1bc2730aed..6d4f88cd7a 100644 --- a/src/sql/rewrite/ob_transform_utils.cpp +++ b/src/sql/rewrite/ob_transform_utils.cpp @@ -2533,7 +2533,44 @@ int ObTransformUtils::is_match_index(const ObDMLStmt* stmt, const ObIArray& exprs, ObIArray& subqueries) +int ObTransformUtils::extract_inseparable_query_ref_expr(ObIArray &exprs, + ObIArray &target_exprs) +{ + int ret = OB_SUCCESS; + for (int64_t i = 0; OB_SUCC(ret) && i < exprs.count(); i++) { + if (OB_FAIL(extract_inseparable_query_ref_expr(exprs.at(i), target_exprs))) { + LOG_WARN("failed to extract query ref exprs", K(ret)); + } + } + return ret; +} + +int ObTransformUtils::extract_inseparable_query_ref_expr(ObRawExpr *expr, ObIArray &target_exprs) +{ + int ret = OB_SUCCESS; + if (OB_ISNULL(expr)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("expr is null", K(ret)); + } else if (expr->is_query_ref_expr() + || T_OP_EXISTS == expr->get_expr_type() + || T_OP_NOT_EXISTS == expr->get_expr_type() + || expr->has_flag(IS_WITH_ALL) + || expr->has_flag(IS_WITH_ANY)) { + if (OB_FAIL(add_var_to_array_no_dup(target_exprs, expr))) { + LOG_WARN("failed to add var to array no dup", 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(extract_inseparable_query_ref_expr(expr->get_param_expr(i), + target_exprs)))) { + LOG_WARN("failed to extract query ref expr", K(ret)); + } + } + } + return ret; +} + +int ObTransformUtils::extract_query_ref_expr(ObIArray &exprs, ObIArray &subqueries) { int ret = OB_SUCCESS; for (int64_t i = 0; OB_SUCC(ret) && i < exprs.count(); i++) { @@ -3465,13 +3502,14 @@ int ObTransformUtils::compute_basic_table_property(ObDMLStmt* stmt, UniqueCheckH ObIArray& cond_exprs, UniqueCheckInfo& res_info) { int ret = OB_SUCCESS; - ObSEArray cur_cond_exprs; - const ObTableSchema* table_schema = NULL; - ObSEArray index_infos; - ObSchemaChecker* schema_checker = NULL; + ObSEArray cur_cond_exprs; ObSqlBitSet<> table_set; + ObSqlSchemaGuard *schema_guard = NULL; + uint64_t index_tids[OB_MAX_INDEX_PER_TABLE]; + int64_t index_count = OB_MAX_INDEX_PER_TABLE; if (OB_ISNULL(stmt) || OB_ISNULL(table) || OB_ISNULL(check_helper.alloc_) || OB_ISNULL(check_helper.fd_factory_) || - OB_ISNULL(schema_checker = check_helper.schema_checker_)) { + OB_ISNULL(check_helper.schema_checker_) || + OB_ISNULL(schema_guard = check_helper.schema_checker_->get_sql_schema_guard())) { ret = OB_ERR_UNEXPECTED; LOG_WARN("unexpected null", K(ret)); } else if (!table->is_basic_table()) { @@ -3488,43 +3526,31 @@ int ObTransformUtils::compute_basic_table_property(ObDMLStmt* stmt, UniqueCheckH LOG_WARN("failed to compute const exprs", K(ret)); } else if (OB_FAIL(ObEqualAnalysis::compute_equal_set(check_helper.alloc_, cur_cond_exprs, res_info.equal_sets_))) { LOG_WARN("failed to compute compute equal set", K(ret)); - } else if (OB_FAIL(schema_checker->get_table_schema(table->ref_id_, table_schema))) { - LOG_WARN("failed to get table schema", K(ret), K(table->ref_id_)); - } else if (OB_ISNULL(table_schema)) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("table schema should not be null", K(ret), K(table)); - } else if (OB_FAIL(ObOptimizerUtil::try_add_fd_item(stmt, - *check_helper.fd_factory_, - table->table_id_, - res_info.table_set_, - table_schema, - cur_cond_exprs, - res_info.not_null_, - res_info.fd_sets_, - res_info.candi_fd_sets_))) { - LOG_WARN("failed to try add fd item", K(ret)); - } else if (OB_FAIL(table_schema->get_simple_index_infos_without_delay_deleted_tid(index_infos, false))) { - LOG_WARN("get simple_index_infos without delay_deleted_tid failed", K(ret)); - } - for (int64_t i = 0; OB_SUCC(ret) && i < index_infos.count(); ++i) { - const ObTableSchema* index_schema = NULL; - if (OB_FAIL(schema_checker->get_table_schema(index_infos.at(i).table_id_, index_schema))) { - LOG_WARN("failed to get table schema", K(ret)); - } else if (OB_ISNULL(index_schema)) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("get null index schema", K(ret)); - } else if (!index_schema->is_unique_index()) { - /*do nothing*/ - } else if (OB_FAIL(ObOptimizerUtil::try_add_fd_item(stmt, - *check_helper.fd_factory_, - table->table_id_, - res_info.table_set_, - index_schema, - cur_cond_exprs, - res_info.not_null_, - res_info.fd_sets_, - res_info.candi_fd_sets_))) { - LOG_WARN("failed to try add fd item", K(ret)); + } else if (OB_FAIL(schema_guard->get_can_read_index_array( + table->ref_id_, index_tids, index_count, false, true /*global index*/, false /*domain index*/))) { + LOG_WARN("failed to get can read index", K(ret), K(table->ref_id_)); + } else { + for (int64_t i = -1; OB_SUCC(ret) && i < index_count; ++i) { + const ObTableSchema *index_schema = NULL; + uint64_t index_id = (i == -1 ? table->ref_id_ : index_tids[i]); + if (OB_FAIL(schema_guard->get_table_schema(index_id, index_schema))) { + LOG_WARN("failed to get table schema", K(ret)); + } else if (OB_ISNULL(index_schema)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("get null index schema", K(ret)); + } else if (-1 != i && !index_schema->is_unique_index()) { + // do nothing + } else if (OB_FAIL(ObOptimizerUtil::try_add_fd_item(stmt, + *check_helper.fd_factory_, + table->table_id_, + res_info.table_set_, + index_schema, + cur_cond_exprs, + res_info.not_null_, + res_info.fd_sets_, + res_info.candi_fd_sets_))) { + LOG_WARN("failed to try add fd item", K(ret)); + } } } return ret; @@ -5359,7 +5385,6 @@ int ObTransformUtils::create_simple_view( if (OB_SUCC(ret)) { // create select list ObSEArray columns; - ObSEArray query_refs; ObSqlBitSet<> from_tables; ObSEArray shared_exprs; if (OB_FAIL(view_stmt->get_from_tables(from_tables))) { @@ -5368,10 +5393,8 @@ int ObTransformUtils::create_simple_view( LOG_WARN("failed to get column exprs", K(ret)); } else if (OB_FAIL(extract_table_exprs(*view_stmt, columns, from_tables, select_list))) { LOG_WARN("failed to extract table exprs", K(ret)); - } else if (OB_FAIL(extract_query_ref_expr(post_join_exprs, query_refs))) { + } else if (OB_FAIL(extract_inseparable_query_ref_expr(post_join_exprs, select_list))) { LOG_WARN("failed to extract query refs", K(ret)); - } else if (OB_FAIL(append(select_list, query_refs))) { - LOG_WARN("failed to append query ref exprs", K(ret)); } else if (OB_FAIL(extract_shared_expr(stmt, view_stmt, shared_exprs))) { LOG_WARN("failed to extract shared expr", K(ret)); } else if (OB_FAIL(append(select_list, shared_exprs))) { @@ -6156,30 +6179,7 @@ int ObTransformUtils::rebuild_select_items(ObSelectStmt& stmt, ObRelIds& output_ return ret; } -int ObTransformUtils::create_select_item_for_subquery( - ObSelectStmt& stmt, ObSelectStmt*& child_stmt, ObIAllocator& alloc, ObIArray& query_ref_exprs) -{ - int ret = OB_SUCCESS; - ObSEArray tmp_query_ref_exprs; - for (int64_t i = 0; OB_SUCC(ret) && i < stmt.get_select_item_size(); ++i) { - ObRawExpr* cur_expr = stmt.get_select_item(i).expr_; - if (OB_FAIL(ObTransformUtils::extract_query_ref_expr(cur_expr, tmp_query_ref_exprs))) { - LOG_WARN("failed to extract query ref expr", K(ret)); - } - } - if (OB_FAIL(ret)) { - } else if (OB_FAIL(append(query_ref_exprs, tmp_query_ref_exprs))) { - LOG_WARN("failed to append exprs", K(ret)); - } - for (int64_t i = 0; OB_SUCC(ret) && i < query_ref_exprs.count(); ++i) { - if (OB_FAIL(create_select_item(alloc, query_ref_exprs.at(i), child_stmt))) { - LOG_WARN("failed to create select item", K(ret)); - } - } - return ret; -} - -int ObTransformUtils::right_join_to_left(ObDMLStmt* stmt) +int ObTransformUtils::right_join_to_left(ObDMLStmt *stmt) { int ret = OB_SUCCESS; if (OB_ISNULL(stmt)) { diff --git a/src/sql/rewrite/ob_transform_utils.h b/src/sql/rewrite/ob_transform_utils.h index 71e6022dff..d7da7d3369 100644 --- a/src/sql/rewrite/ob_transform_utils.h +++ b/src/sql/rewrite/ob_transform_utils.h @@ -313,6 +313,10 @@ public: const ObColumnRefRawExpr* col_expr, bool& is_match, EqualSets* equal_sets = NULL, ObIArray* col_exprs = NULL); + static int extract_inseparable_query_ref_expr(ObIArray &exprs, ObIArray &target_exprs); + + static int extract_inseparable_query_ref_expr(ObRawExpr *expr, ObIArray &target_exprs); + static int extract_query_ref_expr(ObIArray& exprs, ObIArray& subqueries); static int extract_query_ref_expr(ObRawExpr* expr, ObIArray& subqueries); @@ -753,9 +757,6 @@ public: static int replace_with_groupby_exprs(ObSelectStmt* select_stmt, ObRawExpr*& expr); private: - static int create_select_item_for_subquery( - ObSelectStmt& stmt, ObSelectStmt*& child_stmt, ObIAllocator& alloc, ObIArray& query_ref_exprs); - static int add_non_duplicated_select_expr( ObIArray& add_select_exprs, ObIArray& org_select_exprs); }; diff --git a/src/sql/rewrite/ob_transform_where_subquery_pullup.cpp b/src/sql/rewrite/ob_transform_where_subquery_pullup.cpp index 0fe9e748bf..a790b1a4c8 100644 --- a/src/sql/rewrite/ob_transform_where_subquery_pullup.cpp +++ b/src/sql/rewrite/ob_transform_where_subquery_pullup.cpp @@ -2166,6 +2166,8 @@ int ObWhereSubQueryPullup::transform_single_set_query(ObDMLStmt* stmt, bool& tra OB_ISNULL(queries.at(j)->get_ref_stmt())) { ret = OB_ERR_UNEXPECTED; LOG_WARN("invalid subquery", K(ret)); + } else if (queries.at(j)->get_ref_stmt()->is_eliminated()) { + // do nothing } else if (is_vector_query(queries.at(j))) { // not necessary limitation } else if (OB_FAIL(tmp.push_back(queries.at(j)))) { @@ -2196,7 +2198,13 @@ int ObWhereSubQueryPullup::transform_single_set_query(ObDMLStmt* stmt, bool& tra } } for (int64_t j = 0; OB_SUCC(ret) && j < queries.count(); ++j) { - if (OB_FAIL(unnest_single_set_subquery(stmt, queries.at(j), true, is_vector_assign, is_select_expr))) { + ObSelectStmt *subquery = NULL; + if (OB_ISNULL(queries.at(j)) || OB_ISNULL(subquery = queries.at(j)->get_ref_stmt())) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("get unexpected null", K(ret)); + } else if (subquery->is_eliminated()) { + //do nothing + } else if (OB_FAIL(unnest_single_set_subquery(stmt, queries.at(j), true, is_vector_assign, is_select_expr))) { LOG_WARN("failed to unnest single set subquery", K(ret)); } else { trans_happened = true; @@ -2260,7 +2268,8 @@ int ObWhereSubQueryPullup::check_subquery_validity(ObSelectStmt* subquery, bool& if (OB_ISNULL(subquery)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("subquery is null", K(ret), K(subquery)); - } else if (!subquery->is_spj() || subquery->has_subquery() || subquery->get_stmt_hint().enable_no_unnest()) { + } else if (!subquery->is_spj() || subquery->has_subquery() || subquery->get_stmt_hint().enable_no_unnest() || + subquery->is_eliminated()) { is_valid = false; } else if (OB_FAIL(subquery->get_column_exprs(columns))) { LOG_WARN("failed to get column exprs", K(ret)); diff --git a/unittest/sql/resolver/expr/test_raw_expr_canonicalizer.result b/unittest/sql/resolver/expr/test_raw_expr_canonicalizer.result index 7f701aeabc..2fb3f4386b 100644 --- a/unittest/sql/resolver/expr/test_raw_expr_canonicalizer.result +++ b/unittest/sql/resolver/expr/test_raw_expr_canonicalizer.result @@ -20,9 +20,11 @@ not (10 > 5 and 100 < 9) } }, "expr_info": [ + "IS_OR", "IS_CALCULABLE_EXPR", "IS_CONST_EXPR", "CNT_CONST", + "CNT_OR", "CNT_CALCULABLE_EXPR" ], "rel_id": [ @@ -474,8 +476,7 @@ not a = b } }, "expr_info": [ - "CNT_COLUMN", - "IS_JOIN_COND" + "CNT_COLUMN" ], "rel_id": [ ], @@ -709,10 +710,8 @@ not a is true } }, "expr_info": [ - "IS_IS_EXPR", "CNT_CONST", - "CNT_COLUMN", - "CNT_IS_EXPR" + "CNT_COLUMN" ], "rel_id": [ ], @@ -847,8 +846,7 @@ not a between 1 and 100 }, "expr_info": [ "CNT_CONST", - "CNT_COLUMN", - "IS_RANGE_COND" + "CNT_COLUMN" ], "rel_id": [ ], @@ -1166,11 +1164,9 @@ not a between 1 and 100 } }, "expr_info": [ - "IS_OR", "IS_CALCULABLE_EXPR", "IS_CONST_EXPR", "CNT_CONST", - "CNT_OR", "CNT_CALCULABLE_EXPR" ], "rel_id": [ @@ -1352,6 +1348,11 @@ not a between 1 and 100 } }, "expr_info": [ + "IS_CALCULABLE_EXPR", + "IS_CONST_EXPR", + "CNT_CONST", + "CNT_OR", + "CNT_CALCULABLE_EXPR" ], "rel_id": [ ], @@ -1615,6 +1616,10 @@ A or (A And B) or (A And C) } }, "expr_info": [ + "IS_OR", + "CNT_CONST", + "CNT_COLUMN", + "CNT_OR" ], "rel_id": [ ], @@ -1642,6 +1647,8 @@ A or (A And B) or (A And C) } }, "expr_info": [ + "IS_CONST", + "CNT_CONST" ], "rel_id": [ ], @@ -1716,6 +1723,7 @@ A or (A And B) or (A And C) } }, "expr_info": [ + "CNT_COLUMN" ], "rel_id": [ ], @@ -1832,6 +1840,7 @@ A or (A And B) or (A And C) } }, "expr_info": [ + "CNT_COLUMN" ], "rel_id": [ ], @@ -1948,6 +1957,7 @@ A or (A And B) or (A And C) } }, "expr_info": [ + "CNT_COLUMN" ], "rel_id": [ ], -- GitLab