diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index 4df4a9b..8f240ae 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -4517,6 +4517,24 @@ ExecInitExpr(Expr *node, PlanState *parent) AggState *aggstate = (AggState *) parent; Aggref *aggref = (Aggref *) node; + if (aggstate->finalizeAggs == aggref->aggpartial) + { + /* planner messed up */ + if (aggref->aggpartial) + elog(ERROR, "partial Aggref found in finalized aggregate node"); + else + elog(ERROR, "non-partial Aggref found in partial aggregate node"); + } + + if (aggstate->combineStates != aggref->aggcombine) + { + /* planner messed up */ + if (aggref->aggcombine) + elog(ERROR, "combine Aggref found in non-combine aggregate node"); + else + elog(ERROR, "non-combine Aggref found in combine aggregate node"); + } + if (aggstate->finalizeAggs && aggref->aggoutputtype != aggref->aggtype) { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index a21928b..08ed990 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1244,6 +1244,8 @@ _copyAggref(const Aggref *from) COPY_NODE_FIELD(aggfilter); COPY_SCALAR_FIELD(aggstar); COPY_SCALAR_FIELD(aggvariadic); + COPY_SCALAR_FIELD(aggcombine); + COPY_SCALAR_FIELD(aggpartial); COPY_SCALAR_FIELD(aggkind); COPY_SCALAR_FIELD(agglevelsup); COPY_LOCATION_FIELD(location); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 3c6c567..c5ccc42 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -202,6 +202,8 @@ _equalAggref(const Aggref *a, const Aggref *b) COMPARE_NODE_FIELD(aggfilter); COMPARE_SCALAR_FIELD(aggstar); COMPARE_SCALAR_FIELD(aggvariadic); + COMPARE_SCALAR_FIELD(aggcombine); + COMPARE_SCALAR_FIELD(aggpartial); COMPARE_SCALAR_FIELD(aggkind); COMPARE_SCALAR_FIELD(agglevelsup); COMPARE_LOCATION_FIELD(location); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 5ac7446..c2f0e0f 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1040,6 +1040,8 @@ _outAggref(StringInfo str, const Aggref *node) WRITE_NODE_FIELD(aggfilter); WRITE_BOOL_FIELD(aggstar); WRITE_BOOL_FIELD(aggvariadic); + WRITE_BOOL_FIELD(aggcombine); + WRITE_BOOL_FIELD(aggpartial); WRITE_CHAR_FIELD(aggkind); WRITE_UINT_FIELD(agglevelsup); WRITE_LOCATION_FIELD(location); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 8059594..6f28047 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -556,6 +556,8 @@ _readAggref(void) READ_NODE_FIELD(aggfilter); READ_BOOL_FIELD(aggstar); READ_BOOL_FIELD(aggvariadic); + READ_BOOL_FIELD(aggcombine); + READ_BOOL_FIELD(aggpartial); READ_CHAR_FIELD(aggkind); READ_UINT_FIELD(agglevelsup); READ_LOCATION_FIELD(location); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 5537c14..bd6ad9cc 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -2463,6 +2463,14 @@ fix_combine_agg_expr_mutator(Node *node, fix_upper_expr_context *context) newtle = makeTargetEntry((Expr *) newvar, 1, NULL, false); newaggref = (Aggref *) copyObject(aggref); newaggref->args = list_make1(newtle); + newaggref->aggcombine = true; + + /* + * A combine aggregate node does not perform FILTER, so we'd + * better remove any filter clauses so they're not shown in + * EXPLAIN. + */ + newaggref->aggfilter = NULL; return (Node *) newaggref; } diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 4c8c83d..c7872b6 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -785,6 +785,7 @@ apply_partialaggref_adjustment(PathTarget *target) aggform = (Form_pg_aggregate) GETSTRUCT(aggTuple); newaggref = (Aggref *) copyObject(aggref); + newaggref->aggpartial = true; /* use the serialization type, if one exists */ if (OidIsValid(aggform->aggserialtype)) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1b8f0ae..6ed316e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8233,14 +8233,63 @@ get_agg_expr(Aggref *aggref, deparse_context *context) /* Extract the argument types as seen by the parser */ nargs = get_aggregate_argtypes(aggref, argtypes); - /* Print the aggregate name, schema-qualified if needed */ - appendStringInfo(buf, "%s(%s", - generate_function_name(aggref->aggfnoid, nargs, - NIL, argtypes, - aggref->aggvariadic, - &use_variadic, - context->special_exprkind), - (aggref->aggdistinct != NIL) ? "DISTINCT " : ""); + /* + * We prefix partial aggregates which have final functions with the + * keyword "PARTIAL". This avoids printing confusing things like + * avg(avg(x)). We'll happily print max(max(x)), since aggregates like max + * have no final function. + */ + if (aggref->aggpartial) + { + HeapTuple aggTuple; + Form_pg_aggregate aggform; + + /* check if the aggregate has a final function */ + aggTuple = SearchSysCache1(AGGFNOID, + ObjectIdGetDatum(aggref->aggfnoid)); + if (!HeapTupleIsValid(aggTuple)) + elog(ERROR, "cache lookup failed for aggregate %u", + aggref->aggfnoid); + aggform = (Form_pg_aggregate) GETSTRUCT(aggTuple); + + if (OidIsValid(aggform->aggfinalfn)) + appendStringInfoString(buf, "PARTIAL "); + + ReleaseSysCache(aggTuple); + } + + /* + * Combine Aggrefs should never be generated from the parser, so it's OK + * to just display the proname as the function name, and not bother + * looking it up in generate_function_name(). This gets us around the + * problem that the args of a combine Aggref are not suitable for + * generate_function_name() to find the correct aggregate function. + */ + if (aggref->aggcombine) + { + HeapTuple proctup; + Form_pg_proc procform; + Oid funcid = aggref->aggfnoid; + + proctup = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid)); + if (!HeapTupleIsValid(proctup)) + elog(ERROR, "cache lookup failed for function %u", funcid); + procform = (Form_pg_proc) GETSTRUCT(proctup); + appendStringInfo(buf, "%s(%s", NameStr(procform->proname), + (aggref->aggdistinct != NIL) ? "DISTINCT " : ""); + ReleaseSysCache(proctup); + } + else + { + /* Print the aggregate name, schema-qualified if needed */ + appendStringInfo(buf, "%s(%s", + generate_function_name(aggref->aggfnoid, nargs, + NIL, argtypes, + aggref->aggvariadic, + &use_variadic, + context->special_exprkind), + (aggref->aggdistinct != NIL) ? "DISTINCT " : ""); + } if (AGGKIND_IS_ORDERED_SET(aggref->aggkind)) { diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 1ffc0a1..c4ed544 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -261,6 +261,10 @@ typedef struct Param * interesting happens in the Aggref itself, but we must set the output data * type to whatever type is used for transition values. * + * 'aggcombine' and 'aggpartial' should never be true for Aggrefs which come + * from the parser. Such Aggrefs are only ever generated inside the planner + * in order to support multi-phase aggregation. + * * Note: If you are adding fields here you may also need to add a comparison * in search_indexed_tlist_for_partial_aggref() */ @@ -280,6 +284,8 @@ typedef struct Aggref bool aggstar; /* TRUE if argument list was really '*' */ bool aggvariadic; /* true if variadic arguments have been * combined into an array last argument */ + bool aggcombine; /* true if belongs to combine agg node */ + bool aggpartial; /* true if belongs to partial agg node */ char aggkind; /* aggregate kind (see pg_aggregate.h) */ Index agglevelsup; /* > 0 if agg belongs to outer query */ int location; /* token location, or -1 if unknown */