diff --git a/src/backend/distributed/planner/extended_op_node_utils.c b/src/backend/distributed/planner/extended_op_node_utils.c index 1a4c55554..393175312 100644 --- a/src/backend/distributed/planner/extended_op_node_utils.c +++ b/src/backend/distributed/planner/extended_op_node_utils.c @@ -79,6 +79,9 @@ BuildExtendedOpNodeProperties(MultiExtendedOp *extendedOpNode, hasNonPartitionColumnDistinctAgg, extendedOpNode->onlyPushableWindowFunctions); + extendedOpNodeProperties.hasGroupBy = extendedOpNode->groupClauseList != NIL; + extendedOpNodeProperties.hasAggregate = TargetListHasAggregates(targetList); + extendedOpNodeProperties.groupedByDisjointPartitionColumn = groupedByDisjointPartitionColumn; extendedOpNodeProperties.repartitionSubquery = repartitionSubquery; diff --git a/src/backend/distributed/planner/multi_logical_optimizer.c b/src/backend/distributed/planner/multi_logical_optimizer.c index 2d7a9875e..8e158e27c 100644 --- a/src/backend/distributed/planner/multi_logical_optimizer.c +++ b/src/backend/distributed/planner/multi_logical_optimizer.c @@ -228,7 +228,6 @@ static Expr * AddTypeConversion(Node *originalAggregate, Node *newExpression); static MultiExtendedOp * WorkerExtendedOpNode(MultiExtendedOp *originalOpNode, ExtendedOpNodeProperties * extendedOpNodeProperties); -static bool TargetListHasAggregates(List *targetEntryList); static void ProcessTargetListForWorkerQuery(List *targetEntryList, ExtendedOpNodeProperties * extendedOpNodeProperties, @@ -2830,7 +2829,7 @@ BuildOrderByLimitReference(bool hasDistinctOn, bool groupedByDisjointPartitionCo * target list contain aggregates that are not inside the window functions. * This function should not be called if window functions are being pulled up. */ -static bool +bool TargetListHasAggregates(List *targetEntryList) { TargetEntry *targetEntry = NULL; @@ -3840,7 +3839,24 @@ CanPushDownExpression(Node *expression, bool hasWindowFunction = contain_window_function(expression); if (!hasAggregate && !hasWindowFunction) { - return true; + /* + * If the query has the form SELECT expression, agg(..) FROM table; + * then expression should be evaluated on the coordinator. + * + * Other than the efficiency part of this, we could also crash if + * we pushed down the expression to the workers. When pushing down + * expressions to workers we create a Var reference to the worker + * tuples. If the result from worker is empty, but we need to have + * at least a row in coordinator result, postgres will crash when + * trying to evaluate the Var. + * + * For details, see https://github.com/citusdata/citus/pull/3961 + */ + if (!extendedOpNodeProperties->hasAggregate || + extendedOpNodeProperties->hasGroupBy) + { + return true; + } } /* aggregates inside pushed down window functions can be pushed down */ diff --git a/src/include/distributed/extended_op_node_utils.h b/src/include/distributed/extended_op_node_utils.h index 5d2c7cead..e36c35a72 100644 --- a/src/include/distributed/extended_op_node_utils.h +++ b/src/include/distributed/extended_op_node_utils.h @@ -31,6 +31,12 @@ typedef struct ExtendedOpNodeProperties bool onlyPushableWindowFunctions; bool pullUpIntermediateRows; bool pushDownGroupingAndHaving; + + /* indicates whether the MultiExtendedOp has a GROUP BY */ + bool hasGroupBy; + + /* indicates whether the MultiExtendedOp has an aggregate on the target list */ + bool hasAggregate; } ExtendedOpNodeProperties; diff --git a/src/include/distributed/multi_logical_optimizer.h b/src/include/distributed/multi_logical_optimizer.h index 6fc42f8a2..9e6167959 100644 --- a/src/include/distributed/multi_logical_optimizer.h +++ b/src/include/distributed/multi_logical_optimizer.h @@ -176,5 +176,6 @@ extern void FindReferencedTableColumn(Expr *columnExpression, List *parentQueryL Query *query, Oid *relationId, Var **column); extern char * WorkerColumnName(AttrNumber resno); extern bool IsGroupBySubsetOfDistinct(List *groupClauses, List *distinctClauses); +extern bool TargetListHasAggregates(List *targetEntryList); #endif /* MULTI_LOGICAL_OPTIMIZER_H */