> On Mar 8, 2017, at 9:40 AM, Andrew Gierth <and...@tao11.riddles.org.uk> wrote:
> 
>>>>>> "Mark" == Mark Dilger <hornschnor...@gmail.com> writes:
> 
> Mark> Hi Andrew,
> 
> Mark> Reviewing the patch a bit more, I find it hard to understand the
> Mark> comment about passing -1 as a flag for finalize_aggregates.  Any
> Mark> chance you can spend a bit more time word-smithing that code
> Mark> comment?
> 
> Sure.
> 
> How does this look for wording (I'll incorporate it into an updated
> patch later):

Much better.  Thanks!

> /*
> * Compute the final value of all aggregates for one group.
> *
> * This function handles only one grouping set at a time.  But in the hash
> * case, it's the caller's responsibility to have selected the set already, and
> * we pass in -1 as currentSet here to flag that; this also changes how we
> * handle the indexing into AggStatePerGroup as explained below.
> *
> * Results are stored in the output econtext aggvalues/aggnulls.
> */
> static void
> finalize_aggregates(AggState *aggstate,
>                                       AggStatePerAgg peraggs,
>                                       AggStatePerGroup pergroup,
>                                       int currentSet)
> {
>       ExprContext *econtext = aggstate->ss.ps.ps_ExprContext;
>       Datum      *aggvalues = econtext->ecxt_aggvalues;
>       bool       *aggnulls = econtext->ecxt_aggnulls;
>       int                     aggno;
>       int                     transno;
> 
>       /*
>        * If currentSet >= 0, then we're doing sorted grouping, and pergroup 
> is an
>        * array of size numTrans*numSets which we have to index into using
>        * currentSet in addition to transno. The caller may not have selected 
> the
>        * set, so we do that.
>        *
>        * If currentSet < 0, then we're doing hashed grouping, and pergroup is 
> an
>        * array of only numTrans items (since for hashed grouping, each 
> grouping
>        * set is in a separate hashtable).  We rely on the caller having done
>        * select_current_set, and we fudge currentSet to 0 in order to make the
>        * same indexing calculations work as for the grouping case.
>        */
>       if (currentSet >= 0)
>               select_current_set(aggstate, currentSet, false);
>       else
>               currentSet = 0;
> 
> 
> -- 
> Andrew (irc:RhodiumToad)



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to