Date: Wed, 24 Jun 2020 20:32:03 +0200
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Bruce Momjian <bruce@momjian.us>,
	David Rowley <dgrowleyml@gmail.com>, Jeff Davis <pgsql@j-davis.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Melanie Plageman <melanieplageman@gmail.com>,
	pgsql-hackers@postgresql.org
Subject: Re: Default setting for enable_hashagg_disk
Message-ID: <20200624183203.cnqcol27ruastit7@development>
References: <20200610021544.GA14879@telsasoft.com>
 <8bff2e4e8020c3caa16b61a46918d21b573eaf78.camel@j-davis.com>
 <CA+TgmoagMLfruY4y_40oKLmFFdBRVSGraZsTa5ss9wH546EYhQ@mail.gmail.com>
 <ee1a7be071349b93c01ffb9ef4cf3a6830d9e780.camel@j-davis.com>
 <CA+TgmoY4-zSMFf8XbpO6uRsMX4vNPPLXvtajAZpG3f9eoEyjdA@mail.gmail.com>
 <129c8f1739957589584d9525e1d8782eae3f412f.camel@j-davis.com>
 <CAApHDvqFZikXhAGW=UKZKq1_FzHy+XzmUzAJiNj6RWyTHH4UfA@mail.gmail.com>
 <20200624090628.GA19640@momjian.us>
 <20200624123843.GX4107@telsasoft.com>
 <2214502.1593019796@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <2214502.1593019796@sss.pgh.pa.us>
Precedence: bulk

On Wed, Jun 24, 2020 at 01:29:56PM -0400, Tom Lane wrote:
>Justin Pryzby <pryzby@telsasoft.com> writes:
>> On Wed, Jun 24, 2020 at 05:06:28AM -0400, Bruce Momjian wrote:
>>> It would seem merge join has almost the same complexities as the new
>>> hash join code, since it can spill to disk doing sorts for merge joins,
>>> and adjusting work_mem is the only way to control that spill to disk.  I
>>> don't remember anyone complaining about spills to disk during merge
>>> join, so I am unclear why we would need a such control for hash join.
>
>> It loooks like merge join was new in 8.3.  I don't think that's a good analogy,
>> since the old behavior was still available with enable_mergejoin=off.
>
>Uh, what?  A look into our git history shows immediately that
>nodeMergejoin.c has been there since the initial code import in 1996.
>
>I tend to agree with Bruce that it's not very obvious that we need
>another GUC knob here ... especially not one as ugly as this.
>I'm especially against the "neverspill" option, because that makes a
>single GUC that affects both the planner and executor independently.
>
>If we feel we need something to let people have the v12 behavior
>back, let's have
>(1) enable_hashagg on/off --- controls planner, same as it ever was
>(2) enable_hashagg_spill on/off --- controls executor by disabling spill
>

What if a user specifies

   enable_hashagg = on
   enable_hashagg_spill = off

and the estimates say the hashagg would need to spill to disk. Should
that disable the query (in which case the second GUC affects both
executor and planner) or run it (in which case we knowingly ignore
work_mem, which seems wrong).


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services