Horton

Re: Default setting for enable_hashagg_disk

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Jeff Davis <pgsql@j-davis.com>, David Rowley <dgrowleyml@gmail.com>, Stephen Frost <sfrost@snowman.net>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Bruce Momjian <bruce@momjian.us>, Robert Haas <robertmhaas@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Melanie Plageman <melanieplageman@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-07-27T18:00:39Z
Lists: pgsql-hackers, pgsql-docs 
On Mon, Jul 27, 2020 at 10:30 AM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> On 2020-Jul-23, Peter Geoghegan wrote:
> I notice you put the prototype for get_hash_mem in nodeHash.h.  This
> would be fine if not for the fact that optimizer needs to call the
> function too, which means now optimizer have to include executor headers
> -- not a great thing.  I'd move the prototype elsewhere to avoid this,
> and I think miscadmin.h is a decent place for the prototype, next to
> work_mem and m_w_m.

The location of get_hash_mem() is awkward, but there is no obvious alternative.

Are you proposing that I just put the prototype in miscadmin.h, while
leaving the implementation where it is (in nodeHash.c)? Maybe that
sounds like an odd question, but bear in mind that the natural place
to put the implementation of a function declared in miscadmin.h is
either utils/init/postinit.c or utils/init/miscinit.c -- moving the
implementation of get_hash_mem() to either of those two files seems
worse to me.

That said, there is an existing oddball case in miscadmin.h, right at
the end -- the two functions whose implementation is in
access/transam/xlog.c. So I can see an argument for adding another
oddball case (i.e. moving the prototype to the end of miscadmin.h
without changing anything else).

> Other than that admittedly trivial complaint, I found nothing to
> complain about in this patch.

Great. Thanks for the review.

My intention is to commit hash_mem_multiplier on Wednesday. We need to
move on from this, and get the release out the door.

-- 
Peter Geoghegan

Commits

  1. Add hash_mem_multiplier GUC.

  2. HashAgg: use better cardinality estimate for recursive spilling.

  3. Remove hashagg_avoid_disk_plan GUC.

  4. Doc fixup for hashagg_avoid_disk_plan GUC.

  5. Rework HashAgg GUCs.

  6. Disk-based Hash Aggregation.

  7. Implement partition-wise grouping/aggregation.

  8. Defer creation of partially-grouped relation until it's needed.