From 4b68ee12c6344f417e5f5f218857524c4af15f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Tue, 10 Mar 2020 21:20:00 +0000 Subject: [PATCH] Also check aggregates in havingQual when scanning for non pushdownable aggregates Came across this while coming up with test cases, 'result "68_1" does not exist' I'll seek to address in a future PR, for now avoid segfault --- .../planner/multi_logical_optimizer.c | 6 +++++ src/test/regress/expected/multi_subquery.out | 22 +++++++++++++++++++ src/test/regress/sql/multi_subquery.sql | 14 ++++++++++++ 3 files changed, 42 insertions(+) diff --git a/src/backend/distributed/planner/multi_logical_optimizer.c b/src/backend/distributed/planner/multi_logical_optimizer.c index ee32c50c0..0f57c6a89 100644 --- a/src/backend/distributed/planner/multi_logical_optimizer.c +++ b/src/backend/distributed/planner/multi_logical_optimizer.c @@ -3504,6 +3504,7 @@ RequiresIntermediateRowPullUp(MultiNode *logicalPlanNode) MultiExtendedOp *extendedOpNode = (MultiExtendedOp *) linitial(opNodeList); List *targetList = extendedOpNode->targetList; + Node *havingQual = extendedOpNode->havingQual; /* * PVC_REJECT_PLACEHOLDERS is implicit if PVC_INCLUDE_PLACEHOLDERS isn't @@ -3511,6 +3512,8 @@ RequiresIntermediateRowPullUp(MultiNode *logicalPlanNode) */ List *expressionList = pull_var_clause((Node *) targetList, PVC_INCLUDE_AGGREGATES | PVC_INCLUDE_WINDOWFUNCS); + expressionList = list_concat(expressionList, + pull_var_clause(havingQual, PVC_INCLUDE_AGGREGATES)); Node *expression = NULL; foreach_ptr(expression, expressionList) @@ -3547,6 +3550,7 @@ DeferErrorIfContainsNonPushdownableAggregate(MultiNode *logicalPlanNode) MultiExtendedOp *extendedOpNode = (MultiExtendedOp *) linitial(opNodeList); List *targetList = extendedOpNode->targetList; + Node *havingQual = extendedOpNode->havingQual; /* * PVC_REJECT_PLACEHOLDERS is implicit if PVC_INCLUDE_PLACEHOLDERS isn't @@ -3554,6 +3558,8 @@ DeferErrorIfContainsNonPushdownableAggregate(MultiNode *logicalPlanNode) */ List *expressionList = pull_var_clause((Node *) targetList, PVC_INCLUDE_AGGREGATES | PVC_INCLUDE_WINDOWFUNCS); + expressionList = list_concat(expressionList, + pull_var_clause(havingQual, PVC_INCLUDE_AGGREGATES)); ListCell *expressionCell = NULL; foreach(expressionCell, expressionList) diff --git a/src/test/regress/expected/multi_subquery.out b/src/test/regress/expected/multi_subquery.out index d387ded3e..4a0afb2bb 100644 --- a/src/test/regress/expected/multi_subquery.out +++ b/src/test/regress/expected/multi_subquery.out @@ -903,6 +903,28 @@ SELECT t1.event_type FROM events_table t1 GROUP BY t1.event_type HAVING t1.event_type > avg(2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) ORDER BY 1; ERROR: cannot handle unplanned sub-select +SELECT t1.event_type FROM events_table t1 +GROUP BY t1.event_type HAVING t1.event_type > avg(t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) +ORDER BY 1; +ERROR: cannot handle unplanned sub-select +RESET citus.coordinator_aggregation_strategy; +SELECT t1.event_type FROM events_table t1 +GROUP BY t1.event_type HAVING t1.event_type > corr(t1.value_3, t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) +ORDER BY 1; +ERROR: result "68_1" does not exist +CONTEXT: while executing command on localhost:xxxxx +SELECT t1.event_type FROM events_table t1 +GROUP BY t1.event_type HAVING t1.event_type * 5 > sum(distinct t1.value_3) +ORDER BY 1; + event_type +--------------------------------------------------------------------- + 3 + 4 + 5 + 6 +(4 rows) + +SET citus.coordinator_aggregation_strategy TO 'disabled'; -- Test https://github.com/citusdata/citus/issues/3433 CREATE TABLE keyval1 (key int, value int); SELECT create_distributed_table('keyval1', 'key'); diff --git a/src/test/regress/sql/multi_subquery.sql b/src/test/regress/sql/multi_subquery.sql index dae0b6aee..cf4c47a93 100644 --- a/src/test/regress/sql/multi_subquery.sql +++ b/src/test/regress/sql/multi_subquery.sql @@ -641,6 +641,20 @@ SELECT t1.event_type FROM events_table t1 GROUP BY t1.event_type HAVING t1.event_type > avg(2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) ORDER BY 1; +SELECT t1.event_type FROM events_table t1 +GROUP BY t1.event_type HAVING t1.event_type > avg(t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) +ORDER BY 1; + +RESET citus.coordinator_aggregation_strategy; +SELECT t1.event_type FROM events_table t1 +GROUP BY t1.event_type HAVING t1.event_type > corr(t1.value_3, t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) +ORDER BY 1; + +SELECT t1.event_type FROM events_table t1 +GROUP BY t1.event_type HAVING t1.event_type * 5 > sum(distinct t1.value_3) +ORDER BY 1; +SET citus.coordinator_aggregation_strategy TO 'disabled'; + -- Test https://github.com/citusdata/citus/issues/3433 CREATE TABLE keyval1 (key int, value int); SELECT create_distributed_table('keyval1', 'key');