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

/*
 * 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