From d43c80d4d857fce9d5407738ad80c73824eaa704 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Thu, 30 Jan 2020 19:37:28 +0000 Subject: [PATCH] pullUpIntermediateRows should not be true when groupedByDisjointPartitionColumn is true This was causing 'SELECT id, stdev(y_int) FROM tbl GROUP BY id' to push down stddev without group by --- .../planner/extended_op_node_utils.c | 3 ++- .../regress/expected/aggregate_support.out | 19 +++++++++++++++++++ src/test/regress/sql/aggregate_support.sql | 4 ++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/planner/extended_op_node_utils.c b/src/backend/distributed/planner/extended_op_node_utils.c index 1281d1879..882034e08 100644 --- a/src/backend/distributed/planner/extended_op_node_utils.c +++ b/src/backend/distributed/planner/extended_op_node_utils.c @@ -76,7 +76,8 @@ BuildExtendedOpNodeProperties(MultiExtendedOp *extendedOpNode, bool hasNonPartitionColumnDistinctAgg; extendedOpNodeProperties.pullDistinctColumns = pullDistinctColumns; extendedOpNodeProperties.pushDownWindowFunctions = pushDownWindowFunctions; - extendedOpNodeProperties.pullUpIntermediateRows = pullUpIntermediateRows; + extendedOpNodeProperties.pullUpIntermediateRows = + !groupedByDisjointPartitionColumn && pullUpIntermediateRows; return extendedOpNodeProperties; } diff --git a/src/test/regress/expected/aggregate_support.out b/src/test/regress/expected/aggregate_support.out index 470721d2e..bf4ddb41d 100644 --- a/src/test/regress/expected/aggregate_support.out +++ b/src/test/regress/expected/aggregate_support.out @@ -127,6 +127,25 @@ select key, stddev(valf) from aggdata group by key having stddev(val::float8) > 2 | 1.01500410508201 (1 row) +-- Test https://github.com/citusdata/citus/issues/3446 +set citus.coordinator_aggregation_strategy to 'row-gather'; +select id, stddev(val) from aggdata group by id order by 1; + id | stddev +--------------------------------------------------------------------- + 1 | + 2 | + 3 | + 4 | + 5 | + 6 | + 7 | + 8 | + 9 | + 10 | + 11 | +(11 rows) + +set citus.coordinator_aggregation_strategy to 'disabled'; -- test polymorphic aggregates from https://github.com/citusdata/citus/issues/2397 -- we do not currently support pseudotypes for transition types, so this errors for now CREATE OR REPLACE FUNCTION first_agg(anyelement, anyelement) diff --git a/src/test/regress/sql/aggregate_support.sql b/src/test/regress/sql/aggregate_support.sql index 4241e3c77..016f9979b 100644 --- a/src/test/regress/sql/aggregate_support.sql +++ b/src/test/regress/sql/aggregate_support.sql @@ -68,6 +68,10 @@ select sum2(val), sum2_strict(val) from aggdata where valf = 0; select key, stddev(valf) from aggdata group by key having stddev(valf) > 2 order by key; select key, stddev(valf) from aggdata group by key having stddev(val::float8) > 1 order by key; +-- Test https://github.com/citusdata/citus/issues/3446 +set citus.coordinator_aggregation_strategy to 'row-gather'; +select id, stddev(val) from aggdata group by id order by 1; +set citus.coordinator_aggregation_strategy to 'disabled'; -- test polymorphic aggregates from https://github.com/citusdata/citus/issues/2397 -- we do not currently support pseudotypes for transition types, so this errors for now