Aggregate function API versus grouping sets

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Atri Sharma <atri(dot)jiit(at)gmail(dot)com>
Subject: Aggregate function API versus grouping sets
Date: 2014-07-02 09:32:31
Message-ID: 87r424i24w.fsf@news-spur.riddles.org.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I've been assisting Atri with development of an implementation of
GROUPING SETS, beginning with a one-pass implementation of ROLLUP.
Two issues have arisen regarding the API for calling aggregate
transition and final functions that I think need answering now,
since they relate to changes in 9.4.

1. Assumptions about the relationship between aggcontext and fn_extra

Tom's version of the ordered-set aggregate code makes the assumption
that it is safe to store the aggcontext returned by AggCheckCallContext
in a structure hung off flinfo->fn_extra. This implicitly assumes that
the function will be called in only one aggcontext, or at least only one
per flinfo.

Doing rollup via GroupAggregate by maintaining multiple transition
values at a time (one per grouping set) means that the transfn is being
called interleaved for transition values in different contexts. So the
question becomes: is it wrong for the transition function to assume that
aggcontext can be cached this way, or is it necessary for the executor
to use a separate flinfo for each concurrent grouping set?

2. AggGetPerAggEContext

The comment documents this as returning the per-output-tuple econtext,
and the ordered-set code assumes that the rescan of this context implies
that the aggcontext is also about to be reset (so cleanup functions can
be called).

Since it seems that the cleanup callback is the sole reason for this
function to exist, is it acceptable to remove any implication that the
context returned is the overall per-output-tuple one, and simply state
that it is a context whose cleanup functions are called at the
appropriate time before aggcontext is reset? (In fact what I'm
considering is making aggcontext be the per-tuple memory context of an
ExprContext created for each grouping set, and having
AggGetPerAggEContext return this - but it won't be the actual context in
which the result projection happens.)

--
Andrew (irc:RhodiumToad)

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2014-07-02 09:44:12 Re: Allowing NOT IN to use ANTI joins
Previous Message Jeevan Chalke 2014-07-02 09:25:33 Re: Allowing NOT IN to use ANTI joins