Re: Hash support for grouping sets

From: Mark Dilger <hornschnorter(at)gmail(dot)com>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Hash support for grouping sets
Date: 2017-03-08 17:54:22
Message-ID: E8933372-D267-4A6A-BBAC-14582F671812@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On Mar 8, 2017, at 9:40 AM, Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
>
>>>>>> "Mark" == Mark Dilger <hornschnorter(at)gmail(dot)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):

Much better. Thanks!

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-03-08 18:00:06 Re: Parallel bitmap heap scan
Previous Message Robert Haas 2017-03-08 17:54:21 Re: Need a builtin way to run all tests faster manner