public inbox for [email protected]
help / color / mirror / Atom feedFrom: 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: Fri, 28 Oct 2022 17:59:04 -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]>
On Tue, Oct 25, 2022 at 03:49:14PM +0200, Laurenz Albe wrote:
> On Thu, 2022-10-20 at 21:09 -0500, Justin Pryzby wrote:
> > Rebased.
>
> I had a look at the patch set.
Thanks for looking
> @@ -2288,6 +2288,7 @@ regression_main(int argc, char *argv[],
> fputs("log_lock_waits = on\n", pg_conf);
> fputs("log_temp_files = 128kB\n", pg_conf);
> fputs("max_prepared_transactions = 2\n", pg_conf);
> + // fputs("explain_regress = yes\n", pg_conf);
>
> for (sl = temp_configs; sl != NULL; sl = sl->next)
> {
>
> This code comment is a leftover and should go.
No - I was wondering whether the GUC should be specified by the
environment or by the file; I think it's better by file. Then it also
needs to be in Cluster.pm.
> 0004, 0005, 0006, 0007: EXPLAIN (MACHINE)
>
> I think it is confusing that these are included in this patch set.
> EXPLAIN (MACHINE OFF) is similar to "explain_regress = on", only it goes even further:
> no query ID, no Heap Fetches, no Sort details, ...
> Why not add this functionality to the GUC?
Good question, and one I've asked myself.
explain_regress affects pre-existing uses of explain in the regression
tests, aiming to simplify current (or maybe only future) uses. And
is obviously used to allow enabling BUFFERS by default.
explain(MACHINE off) is mostly about allowing "explain ANALYZE" to be
used in regression tests. Which, right now, is rare. Maybe I shouldn't
include those patches until the earlier patches progress (or, maybe we
should just defer their discussion).
> 0005 suppresses "rows removed by filter", but how is that machine dependent?
It can vary with the number of parallel workers (see 13e8b2ee8), which
may not be "machine dependent" but the point of that patch is to allow
predictable output of EXPLAIN ANALYZE. Maybe it needs a better name (or
maybe it should be folded into the first patch).
I'm not actually sure if this patch should try to address parallel
workers at all, or (if so) if what it's doing now is adequate.
> > BTW, I think it may be that the GUC should be marked PGDLLIMPORT ?
>
> I think it is project policy to apply this mark wherever it is needed. Do you think
> that third-party extensions might need to use this in C code?
Since v15 (8ec569479), the policy is:
| On Windows, export all the server's global variables using PGDLLIMPORT markers (Robert Haas)
I'm convinced now that's what's intended here.
> This is not enough. The patch would have to update all the examples
> that use EXPLAIN ANALYZE.
Fair enough. I've left a comment to handle this later if the patch
gains any traction and 001 is progressed.
--
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