Horton

Re: Memory-Bounded Hash Aggregation

Melanie Plageman <melanieplageman@gmail.com>

View thread View on postgr.es
From: Melanie Plageman <melanieplageman@gmail.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Jeff Davis <pgsql@j-davis.com>, Taylor Vesely <tvesely@pivotal.io>, Adam Lee <ali@pivotal.io>, Melanie Plageman <mplageman@pivotal.io>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2020-02-18T23:31:22Z
Lists: pgsql-hackers 
On Tue, Feb 18, 2020 at 10:57 AM Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:

> Hi,
>
> I wanted to take a look at this thread and do a review, but it's not
> very clear to me if the recent patches posted here are independent or
> how exactly they fit together. I see
>
> 1) hashagg-20200212-1.patch (2020/02/13 by Jeff)
>
> 2) refactor.patch (2020/02/13 by Jeff)
>
> 3) v1-0001-aggregated-unaggregated-cols-together.patch (2020/02/14 by
>     Melanie)
>
> I suppose this also confuses the cfbot - it's probably only testing (3)
> as it's the last thing posted here, at least I think that's the case.
>
> And it fails:
>
> nodeAgg.c: In function ‘find_aggregated_cols_walker’:
> nodeAgg.c:1208:2: error: ISO C90 forbids mixed declarations and code
> [-Werror=declaration-after-statement]
>    FindColsContext *find_cols_context = (FindColsContext *) context;
>    ^
> nodeAgg.c: In function ‘find_unaggregated_cols_walker’:
> nodeAgg.c:1225:2: error: ISO C90 forbids mixed declarations and code
> [-Werror=declaration-after-statement]
>    FindColsContext *find_cols_context = (FindColsContext *) context;
>    ^
> cc1: all warnings being treated as errors
> <builtin>: recipe for target 'nodeAgg.o' failed
> make[3]: *** [nodeAgg.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
>
>
Oops! Sorry, I would fix the code that those compiler warnings is
complaining about, but that would confuse the cfbot more. So, I'll let
Jeff decide what he wants to do about the patch at all (e.g. include
it in his overall patch or exclude it for now). Anyway it is trivial
to move those declarations up, were he to decide to include it.

-- 
Melanie Plageman

Commits

  1. Fix costing for disk-based hash aggregation.

  2. Fixes for Disk-based Hash Aggregation.

  3. Disk-based Hash Aggregation.

  4. Extend ExecBuildAggTrans() to support a NULL pointer check.

  5. Save calculated transitionSpace in Agg node.

  6. Minor refactor of nodeAgg.c.

  7. Logical Tape Set: lazily allocate read buffer.

  8. Introduce TupleHashTableHash() and LookupTupleHashEntryHash().

  9. Logical Tape Set: use min heap for freelist.

  10. Fix comments in execGrouping.c

  11. Avoid integer overflow while sifting-up a heap in tuplesort.c.

  12. Make the overflow guards in ExecChooseHashTableSize be more protective.

  13. Improve performance of our private version of qsort. Per recent testing,