>>>>> "Mark" == Mark Dilger <hornschnorter@gmail.com> writes:
Mark> Hi Andrew,
Mark> Reviewing the patch a bit more, I find it hard to understand theMark> comment about passing -1 as a flag for
finalize_aggregates. AnyMark> chance you can spend a bit more time word-smithing that codeMark> 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.
Butin the hash* case, it's the caller's responsibility to have selected the set already, and* we pass in -1 as
currentSethere to flag that; this also changes how we* handle the indexing into AggStatePerGroup as explained below.**
Resultsare 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
haveto 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
hashedgrouping, 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
groupingcase. */if (currentSet >= 0) select_current_set(aggstate, currentSet, false);else currentSet = 0;
--
Andrew (irc:RhodiumToad)
