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)
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 |