mirror of
https://github.com/postgres/postgres.git
synced 2025-07-08 11:42:09 +03:00
Fix handling of argument and result datatypes for partial aggregation.
When doing partial aggregation, the args list of the upper (combining) Aggref node is replaced by a Var representing the output of the partial aggregation steps, which has either the aggregate's transition data type or a serialized representation of that. However, nodeAgg.c blindly continued to use the args list as an indication of the user-level argument types. This broke resolution of polymorphic transition datatypes at executor startup (though it accidentally failed to fail for the ANYARRAY case, which is likely the only one anyone had tested). Moreover, the constructed FuncExpr passed to the finalfunc contained completely wrong information, which would have led to bogus answers or crashes for any case where the finalfunc examined that information (which is only likely to be with polymorphic aggregates using a non-polymorphic transition type). As an independent bug, apply_partialaggref_adjustment neglected to resolve a polymorphic transition datatype before assigning it as the output type of the lower-level Aggref node. This again accidentally failed to fail for ANYARRAY but would be unlikely to work in other cases. To fix the first problem, record the user-level argument types in a separate OID-list field of Aggref, and look to that rather than the args list when asking what the argument types were. (It turns out to be convenient to include any "direct" arguments in this list too, although those are not currently subject to being overwritten.) Rather than adding yet another resolve_aggregate_transtype() call to fix the second problem, add an aggtranstype field to Aggref, and store the resolved transition type OID there when the planner first computes it. (By doing this in the planner and not the parser, we can allow the aggregate's transition type to change from time to time, although no DDL support yet exists for that.) This saves nothing of consequence for simple non-polymorphic aggregates, but for polymorphic transition types we save a catalog lookup during executor startup as well as several planner lookups that are new in 9.6 due to parallel query planning. In passing, fix an error that was introduced into count_agg_clauses_walker some time ago: it was applying exprTypmod() to something that wasn't an expression node at all, but a TargetEntry. exprTypmod silently returned -1 so that there was not an obvious failure, but this broke the intended sensitivity of aggregate space consumption estimates to the typmod of varchar and similar data types. This part needs to be back-patched. Catversion bump due to change of stored Aggref nodes. Discussion: <8229.1466109074@sss.pgh.pa.us>
This commit is contained in:
@ -2085,6 +2085,7 @@ search_indexed_tlist_for_partial_aggref(Aggref *aggref, indexed_tlist *itlist,
|
||||
continue;
|
||||
if (aggref->inputcollid != tlistaggref->inputcollid)
|
||||
continue;
|
||||
/* ignore aggtranstype and aggargtypes, should be redundant */
|
||||
if (!equal(aggref->aggdirectargs, tlistaggref->aggdirectargs))
|
||||
continue;
|
||||
if (!equal(aggref->args, tlistaggref->args))
|
||||
|
@ -523,7 +523,7 @@ contain_agg_clause_walker(Node *node, void *context)
|
||||
/*
|
||||
* count_agg_clauses
|
||||
* Recursively count the Aggref nodes in an expression tree, and
|
||||
* accumulate other cost information about them too.
|
||||
* accumulate other information about them too.
|
||||
*
|
||||
* Note: this also checks for nested aggregates, which are an error.
|
||||
*
|
||||
@ -532,6 +532,10 @@ contain_agg_clause_walker(Node *node, void *context)
|
||||
* values if all are evaluated in parallel (as would be done in a HashAgg
|
||||
* plan). See AggClauseCosts for the exact set of statistics collected.
|
||||
*
|
||||
* In addition, we mark Aggref nodes with the correct aggtranstype, so
|
||||
* that that doesn't need to be done repeatedly. (That makes this function's
|
||||
* name a bit of a misnomer.)
|
||||
*
|
||||
* NOTE that the counts/costs are ADDED to those already in *costs ... so
|
||||
* the caller is responsible for zeroing the struct initially.
|
||||
*
|
||||
@ -572,8 +576,6 @@ count_agg_clauses_walker(Node *node, count_agg_clauses_context *context)
|
||||
Oid aggtranstype;
|
||||
int32 aggtransspace;
|
||||
QualCost argcosts;
|
||||
Oid inputTypes[FUNC_MAX_ARGS];
|
||||
int numArguments;
|
||||
|
||||
Assert(aggref->agglevelsup == 0);
|
||||
|
||||
@ -597,6 +599,28 @@ count_agg_clauses_walker(Node *node, count_agg_clauses_context *context)
|
||||
aggtransspace = aggform->aggtransspace;
|
||||
ReleaseSysCache(aggTuple);
|
||||
|
||||
/*
|
||||
* Resolve the possibly-polymorphic aggregate transition type, unless
|
||||
* already done in a previous pass over the expression.
|
||||
*/
|
||||
if (OidIsValid(aggref->aggtranstype))
|
||||
aggtranstype = aggref->aggtranstype;
|
||||
else
|
||||
{
|
||||
Oid inputTypes[FUNC_MAX_ARGS];
|
||||
int numArguments;
|
||||
|
||||
/* extract argument types (ignoring any ORDER BY expressions) */
|
||||
numArguments = get_aggregate_argtypes(aggref, inputTypes);
|
||||
|
||||
/* resolve actual type of transition state, if polymorphic */
|
||||
aggtranstype = resolve_aggregate_transtype(aggref->aggfnoid,
|
||||
aggtranstype,
|
||||
inputTypes,
|
||||
numArguments);
|
||||
aggref->aggtranstype = aggtranstype;
|
||||
}
|
||||
|
||||
/* count it; note ordered-set aggs always have nonempty aggorder */
|
||||
costs->numAggs++;
|
||||
if (aggref->aggorder != NIL || aggref->aggdistinct != NIL)
|
||||
@ -668,15 +692,6 @@ count_agg_clauses_walker(Node *node, count_agg_clauses_context *context)
|
||||
costs->finalCost += argcosts.per_tuple;
|
||||
}
|
||||
|
||||
/* extract argument types (ignoring any ORDER BY expressions) */
|
||||
numArguments = get_aggregate_argtypes(aggref, inputTypes);
|
||||
|
||||
/* resolve actual type of transition state, if polymorphic */
|
||||
aggtranstype = resolve_aggregate_transtype(aggref->aggfnoid,
|
||||
aggtranstype,
|
||||
inputTypes,
|
||||
numArguments);
|
||||
|
||||
/*
|
||||
* If the transition type is pass-by-value then it doesn't add
|
||||
* anything to the required size of the hashtable. If it is
|
||||
@ -698,14 +713,15 @@ count_agg_clauses_walker(Node *node, count_agg_clauses_context *context)
|
||||
* This works for cases like MAX/MIN and is probably somewhat
|
||||
* reasonable otherwise.
|
||||
*/
|
||||
int numdirectargs = list_length(aggref->aggdirectargs);
|
||||
int32 aggtranstypmod;
|
||||
int32 aggtranstypmod = -1;
|
||||
|
||||
if (numArguments > numdirectargs &&
|
||||
aggtranstype == inputTypes[numdirectargs])
|
||||
aggtranstypmod = exprTypmod((Node *) linitial(aggref->args));
|
||||
else
|
||||
aggtranstypmod = -1;
|
||||
if (aggref->args)
|
||||
{
|
||||
TargetEntry *tle = (TargetEntry *) linitial(aggref->args);
|
||||
|
||||
if (aggtranstype == exprType((Node *) tle->expr))
|
||||
aggtranstypmod = exprTypmod((Node *) tle->expr);
|
||||
}
|
||||
|
||||
avgwidth = get_typavgwidth(aggtranstype, aggtranstypmod);
|
||||
}
|
||||
|
@ -797,11 +797,20 @@ apply_partialaggref_adjustment(PathTarget *target)
|
||||
|
||||
newaggref = (Aggref *) copyObject(aggref);
|
||||
|
||||
/* use the serialization type, if one exists */
|
||||
/*
|
||||
* Use the serialization type, if one exists. Note that we don't
|
||||
* support it being a polymorphic type. (XXX really we ought to
|
||||
* hardwire this as INTERNAL -> BYTEA, and avoid a catalog lookup
|
||||
* here altogether?)
|
||||
*/
|
||||
if (OidIsValid(aggform->aggserialtype))
|
||||
newaggref->aggoutputtype = aggform->aggserialtype;
|
||||
else
|
||||
newaggref->aggoutputtype = aggform->aggtranstype;
|
||||
{
|
||||
/* Otherwise, we return the aggregate's transition type */
|
||||
Assert(OidIsValid(newaggref->aggtranstype));
|
||||
newaggref->aggoutputtype = newaggref->aggtranstype;
|
||||
}
|
||||
|
||||
/* flag it as partial */
|
||||
newaggref->aggpartial = true;
|
||||
|
Reference in New Issue
Block a user