Message-ID: <75b987c43ea03900f83e3612f43e2ffc841db553.camel@j-davis.com>
Subject: Re: Default setting for enable_hashagg_disk
From: Jeff Davis <pgsql@j-davis.com>
To: Peter Geoghegan <pg@bowt.ie>, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: 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: Mon, 27 Jul 2020 17:10:41 -0700
In-Reply-To: 
 <CAH2-Wznmr5r2GtRh4h7AYP+d6EGyKFmKg39wp71ei2UC5db3zA@mail.gmail.com>
References: 
	<CAH2-Wzngb2TVfJc=eC_oXYsRKix_2Cc+ooHR-vb=qJ9kcAYWDA@mail.gmail.com>
	 <20200727182436.GA20148@alvherre.pgsql>
	 <CAH2-Wznmr5r2GtRh4h7AYP+d6EGyKFmKg39wp71ei2UC5db3zA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 2020-07-27 at 11:30 -0700, Peter Geoghegan wrote:
> The v4-0001-Remove-hashagg_avoid_disk_plan-GUC.patch changes are
> surprisingly complicated. It would be nice if you could take a look
> at
> that aspect (or confirm that it's included in your review).

I noticed that one of the conditionals, "cheapest_total_path != NULL",
was already redundant with the outer conditional before your patch. I
guess that was just a mistake which your patch corrects along the way?

Anyway, the patch looks good to me. We can have a separate discussion
about pessimizing the costing, if necessary.

Regards,
	Jeff Davis