From bc1b0aed8523fb23e3ae87a9a81079faa72f27c1 Mon Sep 17 00:00:00 2001 From: vdimir Date: Wed, 24 Mar 2021 15:48:29 +0300 Subject: [PATCH] Correctly handle table alias in PredicateRewriteVisitor --- src/Interpreters/IdentifierSemantic.cpp | 2 +- .../PredicateExpressionsOptimizer.cpp | 6 +++--- .../PredicateExpressionsOptimizer.h | 3 ++- src/Interpreters/PredicateRewriteVisitor.cpp | 19 ++++++++++++------- src/Interpreters/PredicateRewriteVisitor.h | 8 +++++--- .../00597_push_down_predicate.reference | 12 ++++++++++++ .../0_stateless/00597_push_down_predicate.sql | 4 ++++ 7 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/Interpreters/IdentifierSemantic.cpp b/src/Interpreters/IdentifierSemantic.cpp index a1fc533eb7f..81bd499ea2e 100644 --- a/src/Interpreters/IdentifierSemantic.cpp +++ b/src/Interpreters/IdentifierSemantic.cpp @@ -209,7 +209,7 @@ IdentifierSemantic::ColumnMatch IdentifierSemantic::canReferColumnToTable(const return canReferColumnToTable(identifier, table_with_columns.table); } -/// Strip qualificators from left side of column name. +/// Strip qualifications from left side of column name. /// Example: 'database.table.name' -> 'name'. void IdentifierSemantic::setColumnShortName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) { diff --git a/src/Interpreters/PredicateExpressionsOptimizer.cpp b/src/Interpreters/PredicateExpressionsOptimizer.cpp index 00b47be408a..476bdaaceea 100644 --- a/src/Interpreters/PredicateExpressionsOptimizer.cpp +++ b/src/Interpreters/PredicateExpressionsOptimizer.cpp @@ -146,7 +146,7 @@ bool PredicateExpressionsOptimizer::tryRewritePredicatesToTables(ASTs & tables_e break; /// Skip left and right table optimization is_rewrite_tables |= tryRewritePredicatesToTable(tables_element[table_pos], tables_predicates[table_pos], - tables_with_columns[table_pos].columns.getNames()); + tables_with_columns[table_pos]); if (table_element->table_join && isRight(table_element->table_join->as()->kind)) break; /// Skip left table optimization @@ -156,13 +156,13 @@ bool PredicateExpressionsOptimizer::tryRewritePredicatesToTables(ASTs & tables_e return is_rewrite_tables; } -bool PredicateExpressionsOptimizer::tryRewritePredicatesToTable(ASTPtr & table_element, const ASTs & table_predicates, Names && table_columns) const +bool PredicateExpressionsOptimizer::tryRewritePredicatesToTable(ASTPtr & table_element, const ASTs & table_predicates, const TableWithColumnNamesAndTypes & table_columns) const { if (!table_predicates.empty()) { auto optimize_final = enable_optimize_predicate_expression_to_final_subquery; auto optimize_with = allow_push_predicate_when_subquery_contains_with; - PredicateRewriteVisitor::Data data(context, table_predicates, std::move(table_columns), optimize_final, optimize_with); + PredicateRewriteVisitor::Data data(context, table_predicates, table_columns, optimize_final, optimize_with); PredicateRewriteVisitor(data).visit(table_element); return data.is_rewrite; diff --git a/src/Interpreters/PredicateExpressionsOptimizer.h b/src/Interpreters/PredicateExpressionsOptimizer.h index 8cceda93164..223ac1e8998 100644 --- a/src/Interpreters/PredicateExpressionsOptimizer.h +++ b/src/Interpreters/PredicateExpressionsOptimizer.h @@ -33,7 +33,8 @@ private: bool tryRewritePredicatesToTables(ASTs & tables_element, const std::vector & tables_predicates); - bool tryRewritePredicatesToTable(ASTPtr & table_element, const ASTs & table_predicates, Names && table_columns) const; + bool tryRewritePredicatesToTable( + ASTPtr & table_element, const ASTs & table_predicates, const TableWithColumnNamesAndTypes & table_columns) const; bool tryMovePredicatesFromHavingToWhere(ASTSelectQuery & select_query); }; diff --git a/src/Interpreters/PredicateRewriteVisitor.cpp b/src/Interpreters/PredicateRewriteVisitor.cpp index e1d80c1954a..d4083e84639 100644 --- a/src/Interpreters/PredicateRewriteVisitor.cpp +++ b/src/Interpreters/PredicateRewriteVisitor.cpp @@ -17,8 +17,8 @@ namespace DB { PredicateRewriteVisitorData::PredicateRewriteVisitorData( - const Context & context_, const ASTs & predicates_, Names && column_names_, bool optimize_final_, bool optimize_with_) - : context(context_), predicates(predicates_), column_names(column_names_), optimize_final(optimize_final_), optimize_with(optimize_with_) + const Context & context_, const ASTs & predicates_, const TableWithColumnNamesAndTypes & table_columns_, bool optimize_final_, bool optimize_with_) + : context(context_), predicates(predicates_), table_columns(table_columns_), optimize_final(optimize_final_), optimize_with(optimize_with_) { } @@ -42,7 +42,7 @@ void PredicateRewriteVisitorData::visit(ASTSelectWithUnionQuery & union_select_q void PredicateRewriteVisitorData::visitFirstInternalSelect(ASTSelectQuery & select_query, ASTPtr &) { - is_rewrite |= rewriteSubquery(select_query, column_names, column_names); + is_rewrite |= rewriteSubquery(select_query, {}); } void PredicateRewriteVisitorData::visitOtherInternalSelect(ASTSelectQuery & select_query, ASTPtr &) @@ -65,7 +65,7 @@ void PredicateRewriteVisitorData::visitOtherInternalSelect(ASTSelectQuery & sele const Names & internal_columns = InterpreterSelectQuery( temp_internal_select, context, SelectQueryOptions().analyze()).getSampleBlock().getNames(); - if (rewriteSubquery(*temp_select_query, column_names, internal_columns)) + if (rewriteSubquery(*temp_select_query, internal_columns)) { is_rewrite |= true; select_query.setExpression(ASTSelectQuery::Expression::SELECT, std::move(temp_select_query->refSelect())); @@ -89,7 +89,7 @@ static void cleanAliasAndCollectIdentifiers(ASTPtr & predicate, std::vector identifiers; @@ -106,13 +107,17 @@ bool PredicateRewriteVisitorData::rewriteSubquery(ASTSelectQuery & subquery, con for (const auto & identifier : identifiers) { + IdentifierSemantic::setColumnShortName(*identifier, table_columns.table); const auto & column_name = identifier->name(); - const auto & outer_column_iterator = std::find(outer_columns.begin(), outer_columns.end(), column_name); /// For lambda functions, we can't always find them in the list of columns /// For example: SELECT * FROM system.one WHERE arrayMap(x -> x, [dummy]) = [0] + const auto & outer_column_iterator = std::find(outer_columns.begin(), outer_columns.end(), column_name); if (outer_column_iterator != outer_columns.end()) - identifier->setShortName(inner_columns[outer_column_iterator - outer_columns.begin()]); + { + const Names & column_names = inner_columns.empty() ? outer_columns : inner_columns; + identifier->setShortName(column_names[outer_column_iterator - outer_columns.begin()]); + } } /// We only need to push all the predicates to subquery having diff --git a/src/Interpreters/PredicateRewriteVisitor.h b/src/Interpreters/PredicateRewriteVisitor.h index 02c8b9ca422..1132d93a5ec 100644 --- a/src/Interpreters/PredicateRewriteVisitor.h +++ b/src/Interpreters/PredicateRewriteVisitor.h @@ -4,6 +4,7 @@ #include #include #include +#include namespace DB { @@ -24,12 +25,13 @@ public: return true; } - PredicateRewriteVisitorData(const Context & context_, const ASTs & predicates_, Names && column_names_, bool optimize_final_, bool optimize_with_); + PredicateRewriteVisitorData(const Context & context_, const ASTs & predicates_, + const TableWithColumnNamesAndTypes & table_columns_, bool optimize_final_, bool optimize_with_); private: const Context & context; const ASTs & predicates; - const Names column_names; + const TableWithColumnNamesAndTypes & table_columns; bool optimize_final; bool optimize_with; @@ -37,7 +39,7 @@ private: void visitOtherInternalSelect(ASTSelectQuery & select_query, ASTPtr &); - bool rewriteSubquery(ASTSelectQuery & subquery, const Names & outer_columns, const Names & inner_columns); + bool rewriteSubquery(ASTSelectQuery & subquery, const Names & inner_columns); }; using PredicateRewriteMatcher = OneTypeMatcher; diff --git a/tests/queries/0_stateless/00597_push_down_predicate.reference b/tests/queries/0_stateless/00597_push_down_predicate.reference index bd1c4791df4..59313c35b81 100644 --- a/tests/queries/0_stateless/00597_push_down_predicate.reference +++ b/tests/queries/0_stateless/00597_push_down_predicate.reference @@ -585,3 +585,15 @@ SEMI LEFT JOIN ) AS r USING (id) WHERE r.id = 1 2000-01-01 1 test string 1 1 2000-01-01 test string 1 1 +SELECT value + t1.value AS expr +FROM +( + SELECT + value, + t1.value + FROM test_00597 AS t0 + ALL FULL OUTER JOIN test_00597 AS t1 USING (date) + WHERE (value + `t1.value`) < 3 +) +WHERE expr < 3 +2 diff --git a/tests/queries/0_stateless/00597_push_down_predicate.sql b/tests/queries/0_stateless/00597_push_down_predicate.sql index ec306ac6792..2e3357241ad 100644 --- a/tests/queries/0_stateless/00597_push_down_predicate.sql +++ b/tests/queries/0_stateless/00597_push_down_predicate.sql @@ -135,5 +135,9 @@ SELECT * FROM (SELECT * FROM (SELECT * FROM test_00597) AS a ANY LEFT JOIN (SELE EXPLAIN SYNTAX SELECT * FROM (SELECT * FROM test_00597) ANY INNER JOIN (SELECT * FROM (SELECT * FROM test_00597)) as r USING id WHERE r.id = 1; SELECT * FROM (SELECT * FROM test_00597) ANY INNER JOIN (SELECT * FROM (SELECT * FROM test_00597)) as r USING id WHERE r.id = 1; +-- issue 20497 +EXPLAIN SYNTAX SELECT value + t1.value AS expr FROM (SELECT t0.value, t1.value FROM test_00597 AS t0 FULL JOIN test_00597 AS t1 USING date) WHERE expr < 3; +SELECT value + t1.value AS expr FROM (SELECT t0.value, t1.value FROM test_00597 AS t0 FULL JOIN test_00597 AS t1 USING date) WHERE expr < 3; + DROP TABLE IF EXISTS test_00597; DROP TABLE IF EXISTS test_view_00597;