Date: Thu, 9 Jul 2020 10:03:30 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
	Andres Freund <andres@anarazel.de>,
	Bruce Momjian <bruce@momjian.us>,
	Robert Haas <robertmhaas@gmail.com>,
	David Rowley <dgrowleyml@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>
Subject: Re: Default setting for enable_hashagg_disk
Message-ID: <20200709140330.GO3125@tamriel.snowman.net>
References: <85F9F4F0-918A-457F-BC9E-F2AC2EFC1F86@anarazel.de>
 <20200625225853.GA11137@alvherre.pgsql>
 <20200708140037.GI3125@tamriel.snowman.net>
 <99b047386525b87df1e611335c2e57a6cdef346b.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="hJVFpZSnnomxDa7F"
Content-Disposition: inline
In-Reply-To: <99b047386525b87df1e611335c2e57a6cdef346b.camel@j-davis.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--hJVFpZSnnomxDa7F
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Jeff Davis (pgsql@j-davis.com) wrote:
> On Wed, 2020-07-08 at 10:00 -0400, Stephen Frost wrote:
> > That HashAgg previously didn't care that it was going wayyyyy over
> > work_mem was, if anything, a bug.
>=20
> I think we all agree about that, but some people may be depending on
> that bug.

That's why we don't make these kinds of changes in a minor release and
instead have major releases.

> >   Inventing new GUCs late in the
> > cycle like this under duress seems like a *really* bad idea.
>=20
> Are you OK with escape-hatch GUCs that allow the user to opt for v12
> behavior in the event that they experience a regression?

The enable_* options aren't great, and the one added for this is even
stranger since it's an 'enable' option for a particular capability of a
node rather than just a costing change for a node, but I feel like
people generally understand that they shouldn't be messing with the
enable_* options and that they're not really intended for end users.

> The one for the planner is already there, and it looks like we need one
> for the executor as well (to tell HashAgg to ignore the memory limit
> just like v12).

No, ignoring the limit set was, as agreed above, a bug, and I don't
think it makes sense to add some new user tunable for this.  If folks
want to let HashAgg use more memory then they can set work_mem higher,
just the same as if they want a Sort node to use more memory or a
HashJoin.  Yes, that comes with potential knock-on effects about other
nodes (possibly) using more memory but that's pretty well understood for
all the other cases and I don't think that it makes sense to have a
special case for HashAgg when the only justification is that "well, you
see, it used to have this bug, so...".

Thanks,

Stephen

--hJVFpZSnnomxDa7F
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJfByOyAAoJEO1sijiDR2RV5HEP/0YojyGtNP7nkPOb3yEP3fFN
uM8Yst0BywwOHVpyY3Gxih3XUgijqnc+EdwErGQLTGXLZPZIqf/euzw39m3sDf8p
KEmffoIvEe7Zg7ZJMAXb2i8bybRdyksnpqDtz0a5P4hf0z1F3RdkQttsJlHuuZqp
OPtlhjunBKOjDX9HrhnLzrXBP313trr32N2d4yVBJbq3cXfIos0fdv8oveBlxifh
n2xba8hSr04iT9CIOqGB0Fw/V05REtHxvwBK/19Qt+ZvF0tVLbLFL2C4VNm8XX/n
SdxX5xqp+dlcaqh8N69acqjhyuH8TiALAA076jVHaViQyVOKUIDQE5qFmqX0qvdN
iEtC6U9DxHZMBoJWps61ZPiy+SHACxXjxbwS3OixG9e+1l8JZYo+7c8oxwqLvHtY
ouf8f3r2FQfBL9gH6T9WwzXUclhst8II/TUidFRT2Yr2Q6AohfjNtdvCasR801TX
+MEf8OHb+6F7YzO8nCsY/01JB3PxQUE0GR0fFWVr4KWYK/vcKqsS8C6h/UFmkusB
MgoaYqe7L8bQcwbahcBeaQ8szbXXHHzcATICeEeIS8nBZjohbp4o7FduN9oSJ3hw
On5V2b6DTlFiZqF9NWi4J8HRyZi08A3ybhVl2OTHIz+kr9voWOgWYxDcC9XHjU63
/hYzog+pb2vAlHz6W4Lx
=HFD6
-----END PGP SIGNATURE-----

--hJVFpZSnnomxDa7F--