From 951ed7ceca809380572530b89d3e70e3e1b8fa5d Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Thu, 27 Jun 2024 07:08:07 +0000 Subject: [PATCH] Backport #64517 to 24.3: Fixing aliases for GLOBAL IN --- src/Analyzer/Passes/QueryAnalysisPass.cpp | 19 +++++++++++++++--- src/Storages/buildQueryTreeForShard.cpp | 20 ++++++++++++------- .../03164_analyzer_global_in_alias.reference | 4 ++++ .../03164_analyzer_global_in_alias.sql | 6 ++++++ 4 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 tests/queries/0_stateless/03164_analyzer_global_in_alias.reference create mode 100644 tests/queries/0_stateless/03164_analyzer_global_in_alias.sql diff --git a/src/Analyzer/Passes/QueryAnalysisPass.cpp b/src/Analyzer/Passes/QueryAnalysisPass.cpp index 3c4c1100c7d..3a86de8a4ba 100644 --- a/src/Analyzer/Passes/QueryAnalysisPass.cpp +++ b/src/Analyzer/Passes/QueryAnalysisPass.cpp @@ -6523,7 +6523,8 @@ ProjectionNames QueryAnalyzer::resolveExpressionNode(QueryTreeNodePtr & node, Id scope.scope_node->formatASTForErrorMessage()); auto & table_node = node->as(); - result_projection_names.push_back(table_node.getStorageID().getFullNameNotQuoted()); + if (result_projection_names.empty()) + result_projection_names.push_back(table_node.getStorageID().getFullNameNotQuoted()); break; } @@ -8224,7 +8225,10 @@ void QueryAnalyzer::resolveQuery(const QueryTreeNodePtr & query_node, Identifier { auto node = node_with_duplicated_alias; auto node_alias = node->getAlias(); - resolveExpressionNode(node, scope, true /*allow_lambda_expression*/, false /*allow_table_expression*/); + + /// Add current alias to non cached set, because in case of cyclic alias identifier should not be substituted from cache. + /// See 02896_cyclic_aliases_crash. + resolveExpressionNode(node, scope, true /*allow_lambda_expression*/, true /*allow_table_expression*/); bool has_node_in_alias_table = false; @@ -8233,7 +8237,16 @@ void QueryAnalyzer::resolveQuery(const QueryTreeNodePtr & query_node, Identifier { has_node_in_alias_table = true; - if (!it->second->isEqual(*node)) + bool matched = it->second->isEqual(*node); + if (!matched) + /// Table expression could be resolved as scalar subquery, + /// but for duplicating alias we allow table expression to be returned. + /// So, check constant node source expression as well. + if (const auto * constant_node = it->second->as()) + if (const auto & source_expression = constant_node->getSourceExpression()) + matched = source_expression->isEqual(*node); + + if (!matched) throw Exception(ErrorCodes::MULTIPLE_EXPRESSIONS_FOR_ALIAS, "Multiple expressions {} and {} for alias {}. In scope {}", node->formatASTForErrorMessage(), diff --git a/src/Storages/buildQueryTreeForShard.cpp b/src/Storages/buildQueryTreeForShard.cpp index 5bbdbe487b0..131712e750a 100644 --- a/src/Storages/buildQueryTreeForShard.cpp +++ b/src/Storages/buildQueryTreeForShard.cpp @@ -320,6 +320,8 @@ QueryTreeNodePtr buildQueryTreeForShard(const PlannerContextPtr & planner_contex auto replacement_map = visitor.getReplacementMap(); const auto & global_in_or_join_nodes = visitor.getGlobalInOrJoinNodes(); + QueryTreeNodePtrWithHashMap global_in_temporary_tables; + for (const auto & global_in_or_join_node : global_in_or_join_nodes) { if (auto * join_node = global_in_or_join_node.query_node->as()) @@ -364,15 +366,19 @@ QueryTreeNodePtr buildQueryTreeForShard(const PlannerContextPtr & planner_contex if (in_function_node_type != QueryTreeNodeType::QUERY && in_function_node_type != QueryTreeNodeType::UNION && in_function_node_type != QueryTreeNodeType::TABLE) continue; - auto subquery_to_execute = in_function_subquery_node; - if (subquery_to_execute->as()) - subquery_to_execute = buildSubqueryToReadColumnsFromTableExpression(std::move(subquery_to_execute), planner_context->getQueryContext()); + auto & temporary_table_expression_node = global_in_temporary_tables[in_function_subquery_node]; + if (!temporary_table_expression_node) + { + auto subquery_to_execute = in_function_subquery_node; + if (subquery_to_execute->as()) + subquery_to_execute = buildSubqueryToReadColumnsFromTableExpression(subquery_to_execute, planner_context->getQueryContext()); - auto temporary_table_expression_node = executeSubqueryNode(subquery_to_execute, - planner_context->getMutableQueryContext(), - global_in_or_join_node.subquery_depth); + temporary_table_expression_node = executeSubqueryNode(subquery_to_execute, + planner_context->getMutableQueryContext(), + global_in_or_join_node.subquery_depth); + } - in_function_subquery_node = std::move(temporary_table_expression_node); + replacement_map.emplace(in_function_subquery_node.get(), temporary_table_expression_node); } else { diff --git a/tests/queries/0_stateless/03164_analyzer_global_in_alias.reference b/tests/queries/0_stateless/03164_analyzer_global_in_alias.reference new file mode 100644 index 00000000000..459605fc1db --- /dev/null +++ b/tests/queries/0_stateless/03164_analyzer_global_in_alias.reference @@ -0,0 +1,4 @@ +1 1 +1 +1 1 +1 diff --git a/tests/queries/0_stateless/03164_analyzer_global_in_alias.sql b/tests/queries/0_stateless/03164_analyzer_global_in_alias.sql new file mode 100644 index 00000000000..00c293334ee --- /dev/null +++ b/tests/queries/0_stateless/03164_analyzer_global_in_alias.sql @@ -0,0 +1,6 @@ +SET allow_experimental_analyzer=1; +SELECT 1 GLOBAL IN (SELECT 1) AS s, s FROM remote('127.0.0.{2,3}', system.one) GROUP BY 1; +SELECT 1 GLOBAL IN (SELECT 1) AS s FROM remote('127.0.0.{2,3}', system.one) GROUP BY 1; + +SELECT 1 GLOBAL IN (SELECT 1) AS s, s FROM remote('127.0.0.{1,3}', system.one) GROUP BY 1; +SELECT 1 GLOBAL IN (SELECT 1) AS s FROM remote('127.0.0.{1,3}', system.one) GROUP BY 1;