Re: Default setting for enable_hashagg_disk

On Mon, Jul 27, 2020 at 10:30 AM Alvaro Herrera
<alvherre(at)2ndquadrant(dot)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