public inbox for [email protected]  

help / color / mirror / Atom feed

From: Justin Pryzby <[email protected]>
To: Laurenz Albe <[email protected]>
Cc: 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: Sat, 5 Nov 2022 08:44:25 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CANNMO++=LrJ4upoeydZhbmpd_ZgZjrTLueKSrivn6xmb=yFwQw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAKFQuwbnT_B9i4NFWXX0V=xdcsTi0XTHvm6NVcr8XHi39O_A7w@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Fri, Nov 04, 2022 at 11:46:03AM +0100, Laurenz Albe wrote:

> "explain_regress" reduces the EXPLAIN options you need for regression tests.
> This is somewhat useful, but not a big win.  Also, it will make backpatching
> regression tests slightly harder for the next 5 years.

But it doesn't cause backpatching to be harder; if a patch is meant to
be backpatched, its author can still choose to write the old way, with
(COSTS OFF, ...)

> For me, an important question is: do we really want more EXPLAIN (ANALYZE)
> in the regression tests?  It will probably slow the tests down, and I wonder
> if there is a lot of added value, if we have to remove all the machine-specific
> output anyway.

I don't intend to encourage putting lots of "explain analyze" in the
tests.  Rather, this makes that a reasonable possibility rather than
something that people are discouraged from doing by its difficulty.
Using "explain analyze" still needs to be evaluated (for speed and
otherwise), same as always.

In any case, I suggested to defer review of patches 004 and beyond,
which are optional, and it distracts from the essential discussion about
001.

> I would really like to see 0003 go in, but it currently depends on 0001, which
> I am not so sure about.
> I understand that you did that so that "explain_regress" can turn off BUFFERS
> and there is no extra churn in the regression tests.
> 
> Still, it would be a shame if resistance against "explain_regress" would
> be a show-stopper for 0003.
> 
> If I could get my way, I'd want two separate patches: first, one to turn
> BUFFERS on, and second one for "explain_regress" with its current functionality
> on top of that.

You set the patch to "waiting on author", which indicates that there's
no need for further input or review.  But, I think that's precisely
what's needed - without input from more people, what could I do to
progress the patch ?  I don't think it's reasonable to put 001 first and
change thousands (actually, 1338) of regression results.  If nobody
wants to discuss 001, then this patch series won't progress.

-- 
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], [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