From 3b6ea659dfbfe25983bf1cdbdaac51ce38f6d73b Mon Sep 17 00:00:00 2001 From: Nikolai Kochetov Date: Tue, 26 Mar 2024 19:10:34 +0000 Subject: [PATCH] Fixing 02535_analyzer_group_by_use_nulls --- src/Analyzer/Passes/QueryAnalysisPass.cpp | 29 +++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Analyzer/Passes/QueryAnalysisPass.cpp b/src/Analyzer/Passes/QueryAnalysisPass.cpp index 87d943f2e24..c21a1546259 100644 --- a/src/Analyzer/Passes/QueryAnalysisPass.cpp +++ b/src/Analyzer/Passes/QueryAnalysisPass.cpp @@ -6678,45 +6678,48 @@ void QueryAnalyzer::resolveGroupByNode(QueryNode & query_node_typed, IdentifierR if (query_node_typed.isGroupByWithGroupingSets()) { + QueryTreeNodes nullable_group_by_keys; for (auto & grouping_sets_keys_list_node : query_node_typed.getGroupBy().getNodes()) { if (settings.enable_positional_arguments) replaceNodesWithPositionalArguments(grouping_sets_keys_list_node, query_node_typed.getProjection().getNodes(), scope); - resolveExpressionNodeList(grouping_sets_keys_list_node, scope, false /*allow_lambda_expression*/, false /*allow_table_expression*/); - // Remove redundant calls to `tuple` function. It simplifies checking if expression is an aggregation key. // It's required to support queries like: SELECT number FROM numbers(3) GROUP BY (number, number % 2) auto & group_by_list = grouping_sets_keys_list_node->as().getNodes(); expandTuplesInList(group_by_list); + + if (scope.group_by_use_nulls) + for (const auto & group_by_elem : group_by_list) + nullable_group_by_keys.push_back(group_by_elem->clone()); + + resolveExpressionNodeList(grouping_sets_keys_list_node, scope, false /*allow_lambda_expression*/, false /*allow_table_expression*/); } - if (scope.group_by_use_nulls) - { - for (const auto & grouping_set : query_node_typed.getGroupBy().getNodes()) - { - for (const auto & group_by_elem : grouping_set->as()->getNodes()) - scope.nullable_group_by_keys.insert(group_by_elem); - } - } + for (auto & nullable_group_by_key : nullable_group_by_keys) + scope.nullable_group_by_keys.insert(std::move(nullable_group_by_key)); } else { if (settings.enable_positional_arguments) replaceNodesWithPositionalArguments(query_node_typed.getGroupByNode(), query_node_typed.getProjection().getNodes(), scope); - resolveExpressionNodeList(query_node_typed.getGroupByNode(), scope, false /*allow_lambda_expression*/, false /*allow_table_expression*/); - // Remove redundant calls to `tuple` function. It simplifies checking if expression is an aggregation key. // It's required to support queries like: SELECT number FROM numbers(3) GROUP BY (number, number % 2) auto & group_by_list = query_node_typed.getGroupBy().getNodes(); expandTuplesInList(group_by_list); + QueryTreeNodes nullable_group_by_keys; if (scope.group_by_use_nulls) { for (const auto & group_by_elem : query_node_typed.getGroupBy().getNodes()) - scope.nullable_group_by_keys.insert(group_by_elem); + nullable_group_by_keys.push_back(group_by_elem->clone()); } + + resolveExpressionNodeList(query_node_typed.getGroupByNode(), scope, false /*allow_lambda_expression*/, false /*allow_table_expression*/); + + for (auto & nullable_group_by_key : nullable_group_by_keys) + scope.nullable_group_by_keys.insert(std::move(nullable_group_by_key)); } }