Date: Mon, 5 Sep 2022 06:20:31 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Nikolay Samokhvalov <samokhvalov@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: explain_regress, explain(MACHINE), and default to
 explain(BUFFERS) (was: BUFFERS enabled by default in EXPLAIN (ANALYZE))
Message-ID: <20220905112031.GF31833@telsasoft.com>
References: 
 <CANNMO++=LrJ4upoeydZhbmpd_ZgZjrTLueKSrivn6xmb=yFwQw@mail.gmail.com>
 <20211115190954.GO17618@telsasoft.com>
 <20211202005820.GJ17618@telsasoft.com>
 <20220124173548.GV23027@telsasoft.com>
 <CAKFQuwbnT_B9i4NFWXX0V=xdcsTi0XTHvm6NVcr8XHi39O_A7w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="jfWagoTHmfL/c8Ax"
Content-Disposition: inline
In-Reply-To: 
 <CAKFQuwbnT_B9i4NFWXX0V=xdcsTi0XTHvm6NVcr8XHi39O_A7w@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220905112031.GF31833%40telsasoft.com>
Precedence: bulk


--jfWagoTHmfL/c8Ax
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Jul 26, 2022 at 03:38:53PM -0700, David G. Johnston wrote:
> On Mon, Jan 24, 2022 at 9:54 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

> > > Unfortunately, "COSTS OFF" breaks postgres_fdw remote_estimate.  If specifying
> > > "COSTS ON" in postgres_fdw.c is considered to be a poor fix , then I suppose
> > > this patch series could do as suggested and enable buffers by default only when
> > > ANALYZE is specified.  Then postgres_fdw is not affected, and the
> > > explain_regress GUC is optional: instead, we'd need to specify BUFFERS OFF in
> > > ~100 regression tests which use EXPLAIN ANALYZE.
> 
> I'm not following the transition from the prior sentences about COSTS to
> this one regarding BUFFERS.

In this patch, SET explain_regress=on changes to defaults to
explain(COSTS OFF), so postgres_fdw now explicitly uses COSTS ON.  The
patch also changes default to (BUFFERS OFF), which is what allows
enabling buffers by default.  I think I was just saying that the
enabling buffers by default depends on somehow disabling it in
regression tests (preferably not by adding 100 instances of "BUFFERS
OFF").

> The following change in the machine commit seems out-of-place; are we
> fixing a bug here?
> 
> explain.c:
>    /* Show buffer usage in planning */
> - if (bufusage)
> + if (bufusage && es->buffers)

I think that was an extraneous change, maybe from a conflict while
re-ordering the patches.  Removed it and rebased.  Thanks for looking.

-- 
Justin

--jfWagoTHmfL/c8Ax
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0001-Add-GUC-explain_regress.patch"