Re: Aggregate function API versus grouping sets

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Doing rollup via GroupAggregate by maintaining multiple transition>> values at a time (one per grouping set) means
thatthe transfn is>> being called interleaved for transition values in different>> contexts. So the question becomes:
isit wrong for the transition>> function to assume that aggcontext can be cached this way, or is>> it necessary for the
executorto use a separate flinfo for each>> concurrent grouping set?
 
Tom> Hm.  We could probably move gcontext into the per-group data.Tom> I'm not sure though if there are any other
dependenciesthere onTom> the groups being evaluated serially.  More generally, I wonderTom> whether user-defined
aggregatesare likely to be makingTom> assumptions that will be broken by this.
 

The thing is that almost everything _except_ aggcontext and
AggGetPerAggEContext that a transfn might want to hang off fn_extra
really is going to be constant across all groups.

The big question, as you say, is whether this is going to be an issue
for existing user-defined aggregates.
>> Since it seems that the cleanup callback is the sole reason for>> this function to exist, is it acceptable to remove
anyimplication>> that the context returned is the overall per-output-tuple one, and>> simply state that it is a context
whosecleanup functions are>> called at the appropriate time before aggcontext is reset?
 
Tom> Well, I think the implication is that it's the econtext in whichTom> the result of the aggregation will be used.

In the rollup case, though, it does not seem reasonable to have
multiple result-tuple econtexts (that would significantly complicate
the projection of rows, the handling of rescans, etc.).
Tom> Another approach would be to remove AggGetPerAggEContext as suchTom> from the API altogether, and instead offer an
interfacethatTom> says "register an aggregate cleanup callback", leaving it to theTom> agg/window core code to figure
outwhich context to hang thatTom> on.  I had thought that exposing this econtext and theTom> per-input-tuple one would
provideuseful generality, but maybeTom> we should rethink that.
 

Works for me.

-- 
Andrew (irc:RhodiumToad)