From 7f05ccaddf8c80c1c9d76d2a818c1751681c714b Mon Sep 17 00:00:00 2001 From: Nikolai Kochetov Date: Wed, 15 May 2024 16:20:42 +0000 Subject: [PATCH 1/3] Update lambda execution name. Fix query tree hash calculation in case of empty database. --- src/Planner/PlannerActionsVisitor.cpp | 20 +++++++++++++++++-- ...tion_memory_efficient_mix_levels.reference | 11 ++++++++++ ...ggregation_memory_efficient_mix_levels.sql | 2 ++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/Planner/PlannerActionsVisitor.cpp b/src/Planner/PlannerActionsVisitor.cpp index 2b369eaa593..49a9305b3ce 100644 --- a/src/Planner/PlannerActionsVisitor.cpp +++ b/src/Planner/PlannerActionsVisitor.cpp @@ -243,8 +243,24 @@ public: } case QueryTreeNodeType::LAMBDA: { - auto lambda_hash = node->getTreeHash(); - result = "__lambda_" + toString(lambda_hash); + WriteBufferFromOwnString buffer; + + const auto & lambda_node = node->as(); + const auto & lambda_arguments_nodes = lambda_node.getArguments().getNodes(); + + size_t lambda_arguments_nodes_size = lambda_arguments_nodes.size(); + for (size_t i = 0; i < lambda_arguments_nodes_size; ++i) + { + const auto & lambda_arguments_node = lambda_arguments_nodes[i]; + buffer << calculateActionNodeName(lambda_arguments_node); + + if (i + 1 != lambda_arguments_nodes_size) + buffer << ", "; + } + + buffer << " -> " << calculateActionNodeName(lambda_node.getExpression()); + + result = buffer.str(); break; } default: diff --git a/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.reference b/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.reference index ac13b3f193e..1d1e81fa5e2 100644 --- a/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.reference +++ b/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.reference @@ -8,3 +8,14 @@ 7 1 8 1 9 1 +[0] +[0] +[1] +[2] +[3] +[4] +[5] +[6] +[7] +[8] +[9] diff --git a/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.sql b/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.sql index e70652877e0..aef1d22372d 100644 --- a/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.sql +++ b/tests/queries/0_stateless/01231_distributed_aggregation_memory_efficient_mix_levels.sql @@ -23,6 +23,8 @@ set max_bytes_before_external_group_by = 16; select x, count() from ma_dist group by x order by x; +select arrayFilter(y -> y = x, [x]) as f from ma_dist order by f; + drop table if exists shard_0.shard_01231_distributed_aggregation_memory_efficient; drop table if exists shard_1.shard_01231_distributed_aggregation_memory_efficient; From 61acdc2bcb3d98c949748b15c29dcc58d0d73dce Mon Sep 17 00:00:00 2001 From: Nikolai Kochetov Date: Wed, 15 May 2024 17:38:33 +0000 Subject: [PATCH 2/3] Fix test. --- src/Planner/PlannerActionsVisitor.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Planner/PlannerActionsVisitor.cpp b/src/Planner/PlannerActionsVisitor.cpp index 49a9305b3ce..c2ab1a99910 100644 --- a/src/Planner/PlannerActionsVisitor.cpp +++ b/src/Planner/PlannerActionsVisitor.cpp @@ -251,8 +251,10 @@ public: size_t lambda_arguments_nodes_size = lambda_arguments_nodes.size(); for (size_t i = 0; i < lambda_arguments_nodes_size; ++i) { - const auto & lambda_arguments_node = lambda_arguments_nodes[i]; - buffer << calculateActionNodeName(lambda_arguments_node); + const auto & lambda_argument_node = lambda_arguments_nodes[i]; + buffer << calculateActionNodeName(lambda_argument_node); + buffer << ' '; + buffer << lambda_argument_node->as().getResultType()->getName(); if (i + 1 != lambda_arguments_nodes_size) buffer << ", "; From d34e06ce4275f9db0767137422e7caab4076fa9a Mon Sep 17 00:00:00 2001 From: Nikolai Kochetov Date: Thu, 16 May 2024 17:55:07 +0000 Subject: [PATCH 3/3] Add a comment. --- src/Planner/PlannerActionsVisitor.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Planner/PlannerActionsVisitor.cpp b/src/Planner/PlannerActionsVisitor.cpp index c2ab1a99910..a88c74d460b 100644 --- a/src/Planner/PlannerActionsVisitor.cpp +++ b/src/Planner/PlannerActionsVisitor.cpp @@ -243,6 +243,14 @@ public: } case QueryTreeNodeType::LAMBDA: { + /// Initially, the action name was `"__lambda_" + toString(node->getTreeHash());`. + /// This is not a good idea because: + /// * hash is different on initiator and shard if the default database is changed in cluster + /// * hash is reliable only within one node; any change will break queries in between versions + /// + /// Now, we calculate execution name as (names + types) for lambda arguments + action name (expression) + /// and this should be more reliable (as long as we trust the calculation of action name for functions) + WriteBufferFromOwnString buffer; const auto & lambda_node = node->as();