Re: [HACKERS] Hash support for grouping sets

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