public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
To: David G. Johnston <[email protected]>
Cc: Nikolay Samokhvalov <[email protected]>
Cc: [email protected]
Subject: Re: explain_regress, explain(MACHINE), and default to explain(BUFFERS) (was: BUFFERS enabled by default in EXPLAIN (ANALYZE))
Date: Mon, 5 Sep 2022 06:20:31 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAKFQuwbnT_B9i4NFWXX0V=xdcsTi0XTHvm6NVcr8XHi39O_A7w@mail.gmail.com>
References: <CANNMO++=LrJ4upoeydZhbmpd_ZgZjrTLueKSrivn6xmb=yFwQw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAKFQuwbnT_B9i4NFWXX0V=xdcsTi0XTHvm6NVcr8XHi39O_A7w@mail.gmail.com>
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 <[email protected]> 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
view thread (14+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: explain_regress, explain(MACHINE), and default to explain(BUFFERS) (was: BUFFERS enabled by default in EXPLAIN (ANALYZE))
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox