From fd64ae3d6c6b8e50c0f745b981f704e6af046e2a Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Thu, 7 Nov 2024 19:10:02 +0000 Subject: [PATCH] Backport #71538 to 24.9: Analyzer materialized view IN with CTE fix --- src/Analyzer/QueryNode.h | 12 ++++ src/Analyzer/Resolve/QueryAnalyzer.cpp | 49 ++++++++++----- src/Analyzer/UnionNode.cpp | 21 +++++++ src/Analyzer/UnionNode.h | 3 + ...er_materialized_view_in_with_cte.reference | 1 + ...analyzer_materialized_view_in_with_cte.sql | 63 +++++++++++++++++++ ...zer_materialized_view_cte_nested.reference | 1 + ..._analyzer_materialized_view_cte_nested.sql | 19 ++++++ 8 files changed, 152 insertions(+), 17 deletions(-) create mode 100644 tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.reference create mode 100644 tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.sql create mode 100644 tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.reference create mode 100644 tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.sql diff --git a/src/Analyzer/QueryNode.h b/src/Analyzer/QueryNode.h index aef0c8805bb..2333fc56218 100644 --- a/src/Analyzer/QueryNode.h +++ b/src/Analyzer/QueryNode.h @@ -602,9 +602,21 @@ public: return projection_columns; } + /// Returns true if query node is resolved, false otherwise + bool isResolved() const + { + return !projection_columns.empty(); + } + /// Resolve query node projection columns void resolveProjectionColumns(NamesAndTypes projection_columns_value); + /// Clear query node projection columns + void clearProjectionColumns() + { + projection_columns.clear(); + } + /// Remove unused projection columns void removeUnusedProjectionColumns(const std::unordered_set & used_projection_columns); diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index ba1ef850118..f235c35fa0f 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -2928,27 +2928,29 @@ ProjectionNames QueryAnalyzer::resolveFunction(QueryTreeNodePtr & node, Identifi /// Replace storage with values storage of insertion block if (StoragePtr storage = scope.context->getViewSource()) { - QueryTreeNodePtr table_expression; - /// Process possibly nested sub-selects - for (auto * query_node = in_second_argument->as(); query_node; query_node = table_expression->as()) - table_expression = extractLeftTableExpression(query_node->getJoinTree()); + QueryTreeNodePtr table_expression = in_second_argument; - if (table_expression) + /// Process possibly nested sub-selects + while (table_expression) { - if (auto * query_table_node = table_expression->as()) - { - if (query_table_node->getStorageID().getFullNameNotQuoted() == storage->getStorageID().getFullNameNotQuoted()) - { - auto replacement_table_expression = std::make_shared(storage, scope.context); - if (std::optional table_expression_modifiers = query_table_node->getTableExpressionModifiers()) - replacement_table_expression->setTableExpressionModifiers(*table_expression_modifiers); - in_second_argument = in_second_argument->cloneAndReplace(table_expression, std::move(replacement_table_expression)); - } - } + if (auto * query_node = table_expression->as()) + table_expression = extractLeftTableExpression(query_node->getJoinTree()); + else if (auto * union_node = table_expression->as()) + table_expression = union_node->getQueries().getNodes().at(0); + else + break; + } + + TableNode * table_expression_table_node = table_expression ? table_expression->as() : nullptr; + + if (table_expression_table_node && + table_expression_table_node->getStorageID().getFullNameNotQuoted() == storage->getStorageID().getFullNameNotQuoted()) + { + auto replacement_table_expression_table_node = table_expression_table_node->clone(); + replacement_table_expression_table_node->as().updateStorage(storage, scope.context); + in_second_argument = in_second_argument->cloneAndReplace(table_expression, std::move(replacement_table_expression_table_node)); } } - - resolveExpressionNode(in_second_argument, scope, false /*allow_lambda_expression*/, true /*allow_table_expression*/); } /// Edge case when the first argument of IN is scalar subquery. @@ -5283,6 +5285,16 @@ void QueryAnalyzer::resolveQuery(const QueryTreeNodePtr & query_node, Identifier auto & query_node_typed = query_node->as(); + /** It is unsafe to call resolveQuery on already resolved query node, because during identifier resolution process + * we replace identifiers with expressions without aliases, also at the end of resolveQuery all aliases from all nodes will be removed. + * For subsequent resolveQuery executions it is possible to have wrong projection header, because for nodes + * with aliases projection name is alias. + * + * If for client it is necessary to resolve query node after clone, client must clear projection columns from query node before resolve. + */ + if (query_node_typed.isResolved()) + return; + if (query_node_typed.isCTE()) ctes_in_resolve_process.insert(query_node_typed.getCTEName()); @@ -5648,6 +5660,9 @@ void QueryAnalyzer::resolveUnion(const QueryTreeNodePtr & union_node, Identifier { auto & union_node_typed = union_node->as(); + if (union_node_typed.isResolved()) + return; + if (union_node_typed.isCTE()) ctes_in_resolve_process.insert(union_node_typed.getCTEName()); diff --git a/src/Analyzer/UnionNode.cpp b/src/Analyzer/UnionNode.cpp index 77f6921f939..47f6462fb16 100644 --- a/src/Analyzer/UnionNode.cpp +++ b/src/Analyzer/UnionNode.cpp @@ -35,6 +35,7 @@ namespace ErrorCodes { extern const int TYPE_MISMATCH; extern const int BAD_ARGUMENTS; + extern const int LOGICAL_ERROR; } UnionNode::UnionNode(ContextMutablePtr context_, SelectUnionMode union_mode_) @@ -50,6 +51,26 @@ UnionNode::UnionNode(ContextMutablePtr context_, SelectUnionMode union_mode_) children[queries_child_index] = std::make_shared(); } +bool UnionNode::isResolved() const +{ + for (const auto & query_node : getQueries().getNodes()) + { + bool is_resolved = false; + + if (auto * query_node_typed = query_node->as()) + is_resolved = query_node_typed->isResolved(); + else if (auto * union_node_typed = query_node->as()) + is_resolved = union_node_typed->isResolved(); + else + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected query tree node type in UNION node"); + + if (!is_resolved) + return false; + } + + return true; +} + NamesAndTypes UnionNode::computeProjectionColumns() const { if (recursive_cte_table) diff --git a/src/Analyzer/UnionNode.h b/src/Analyzer/UnionNode.h index 40baad1ad57..85d6afb1e47 100644 --- a/src/Analyzer/UnionNode.h +++ b/src/Analyzer/UnionNode.h @@ -163,6 +163,9 @@ public: return children[queries_child_index]; } + /// Returns true if union node is resolved, false otherwise + bool isResolved() const; + /// Compute union node projection columns NamesAndTypes computeProjectionColumns() const; diff --git a/tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.reference b/tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.reference new file mode 100644 index 00000000000..5ddf8439af5 --- /dev/null +++ b/tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.reference @@ -0,0 +1 @@ +1 2 \N test diff --git a/tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.sql b/tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.sql new file mode 100644 index 00000000000..4543d336d14 --- /dev/null +++ b/tests/queries/0_stateless/03262_analyzer_materialized_view_in_with_cte.sql @@ -0,0 +1,63 @@ +SET allow_experimental_analyzer = 1; + +DROP TABLE IF EXISTS mv_test; +DROP TABLE IF EXISTS mv_test_target; +DROP VIEW IF EXISTS mv_test_mv; + +CREATE TABLE mv_test +( + `id` UInt64, + `ref_id` UInt64, + `final_id` Nullable(UInt64), + `display` String +) +ENGINE = Log; + +CREATE TABLE mv_test_target +( + `id` UInt64, + `ref_id` UInt64, + `final_id` Nullable(UInt64), + `display` String +) +ENGINE = Log; + +CREATE MATERIALIZED VIEW mv_test_mv TO mv_test_target +( + `id` UInt64, + `ref_id` UInt64, + `final_id` Nullable(UInt64), + `display` String +) +AS WITH + tester AS + ( + SELECT + id, + ref_id, + final_id, + display + FROM mv_test + ), + id_set AS + ( + SELECT + display, + max(id) AS max_id + FROM mv_test + GROUP BY display + ) +SELECT * +FROM tester +WHERE id IN ( + SELECT max_id + FROM id_set +); + +INSERT INTO mv_test ( id, ref_id, display) values ( 1, 2, 'test'); + +SELECT * FROM mv_test_target; + +DROP VIEW mv_test_mv; +DROP TABLE mv_test_target; +DROP TABLE mv_test; diff --git a/tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.reference b/tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.reference new file mode 100644 index 00000000000..0cfbf08886f --- /dev/null +++ b/tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.reference @@ -0,0 +1 @@ +2 diff --git a/tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.sql b/tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.sql new file mode 100644 index 00000000000..4ea853a7c22 --- /dev/null +++ b/tests/queries/0_stateless/03263_analyzer_materialized_view_cte_nested.sql @@ -0,0 +1,19 @@ +SET allow_experimental_analyzer = 1; + +DROP TABLE IF EXISTS test_table; +DROP VIEW IF EXISTS test_mv; + +CREATE TABLE test_table ENGINE = MergeTree ORDER BY tuple() AS SELECT 1 as col1; + +CREATE MATERIALIZED VIEW test_mv ENGINE = MergeTree ORDER BY tuple() AS +WITH + subquery_on_source AS (SELECT col1 AS aliased FROM test_table), + output AS (SELECT * FROM test_table WHERE col1 IN (SELECT aliased FROM subquery_on_source)) +SELECT * FROM output; + +INSERT INTO test_table VALUES (2); + +SELECT * FROM test_mv; + +DROP VIEW test_mv; +DROP TABLE test_table;