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@gmail.com> wrote:
> 
> 
>> On Mar 8, 2017, at 5:47 AM, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote:
>> 
>>>>>>> "Mark" == Mark Dilger <hornschnorter@gmail.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.
>