Re: Hash support for grouping sets

Hi Andrew,

Reviewing the patch a bit more, I find it hard to understand the comment about
passing -1 as a flag for finalize_aggregates. Any chance you can spend a bit
more time word-smithing that code comment?

@@ -1559,7 +1647,9 @@ prepare_projection_slot(AggState *aggstate, TupleTableSlot *slot, int currentSet
/*
* Compute the final value of all aggregates for one group.
*
- * This function handles only one grouping set at a time.
+ * 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 here to flag that and to control the indexing into pertrans.
*
* Results are stored in the output econtext aggvalues/aggnulls.
*/
@@ -1575,10 +1665,11 @@ finalize_aggregates(AggState *aggstate,
int aggno;
int transno;

- Assert(currentSet == 0 ||
- ((Agg *) aggstate->ss.ps.plan)->aggstrategy != AGG_HASHED);
-
- aggstate->current_set = currentSet;
+ /* ugly hack for hash */
+ if (currentSet >= 0)
+ select_current_set(aggstate, currentSet, false);
+ else
+ currentSet = 0;

> On Mar 8, 2017, at 8:00 AM, Mark Dilger <hornschnorter(at)gmail(dot)com> wrote:
>
>
>> On Mar 8, 2017, at 5:47 AM, Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
>>
>>>>>>> "Mark" == Mark Dilger <hornschnorter(at)gmail(dot)com> writes:
>>
>> Mark> On my MacBook, `make check-world` gives differences in the
>> Mark> contrib modules:
>>
>> Thanks! Latest cleaned up version of patch is attached.
>
> This fixes both the warning and the contrib tests on linux and mac.
>