Thread: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
hi, I have a proposal, resulted from numerous communications with various folks, both very experienced and new Postgres users:
1) EXPLAIN ANALYZE Is sometimes very confusing (because there is ANALYZE). Let's rename it to EXPLAIN EXECUTE?
2) VERBOSE doesn't include BUFFERS, and doesn't include SETTINGS; it might be also confusing sometimes. Let's include them so VERBOSE would be really verbose?
3) small thing about grammar: allow omitting parentheses, so EXPLAIN EXECUTE VERBOSE would work.
if both changes are done, we could use EXPLAIN (EXECUTE, VERBOSE) to be able to collect data in a great way for analysis.
have a really nice week,
Nik
Nikolay Samokhvalov <samokhvalov@gmail.com> writes:
> 1) EXPLAIN ANALYZE Is sometimes very confusing (because there is ANALYZE).
> Let's rename it to EXPLAIN EXECUTE?
This has got far too many years of history to be renamed now.
> 2) VERBOSE doesn't include BUFFERS, and doesn't include SETTINGS; it might
> be also confusing sometimes. Let's include them so VERBOSE would be really
> verbose?
This is not likely to fly for compatibility reasons.
> 3) small thing about grammar: allow omitting parentheses, so EXPLAIN
> EXECUTE VERBOSE would work.
The reason for the parens is that the other way would require reserving
all these options as keywords.
regards, tom lane
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Nikolay Samokhvalov
Date:
On Tue, Nov 5, 2024 at 10:16 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Nikolay Samokhvalov <samokhvalov@gmail.com> writes:
> 1) EXPLAIN ANALYZE Is sometimes very confusing (because there is ANALYZE).
> Let's rename it to EXPLAIN EXECUTE?
This has got far too many years of history to be renamed now.
This is a really, really strange argument. Postgres keeps receiving new audiences at larger and larger scale. And they are confused.
It's better late than never. I didn't believe we would have "quit" working in psql.
> 2) VERBOSE doesn't include BUFFERS, and doesn't include SETTINGS; it might
> be also confusing sometimes. Let's include them so VERBOSE would be really
> verbose?
This is not likely to fly for compatibility reasons.
Can you elaborate?
> 3) small thing about grammar: allow omitting parentheses, so EXPLAIN
> EXECUTE VERBOSE would work.
The reason for the parens is that the other way would require reserving
all these options as keywords.
turns out, EXPLAIN ANALYZE VERBOSE already working (it's just not as verbose as one might expect_:
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.01 rows=1 width=0)
(1 row)
On Tue, Nov 5, 2024 at 1:02 PM Nikolay Samokhvalov <samokhvalov@gmail.com> wrote: > hi, I have a proposal, resulted from numerous communications with various folks, both very experienced and new Postgresusers: > > 1) EXPLAIN ANALYZE Is sometimes very confusing (because there is ANALYZE). Let's rename it to EXPLAIN EXECUTE? The trouble is that EXPLAIN EXECUTE already means something. robert.haas=# explain execute foo; ERROR: prepared statement "foo" does not exist Granted, that would not make it impossible to make EXPLAIN (EXECUTE) a synonym for EXPLAIN (ANALYZE), but IMHO it would be pretty confusing if EXPLAIN EXECUTE and EXPLAIN (EXECUTE) did different things. > 2) VERBOSE doesn't include BUFFERS, and doesn't include SETTINGS; it might be also confusing sometimes. Let's include themso VERBOSE would be really verbose? I agree that the naming here isn't great, but I think making the options non-orthogonal would probably be worse. > 3) small thing about grammar: allow omitting parentheses, so EXPLAIN EXECUTE VERBOSE would work. Perhaps surprisingly, it turns out that this is not a small change. As Tom mentions, this would have a pretty large blast radius. In fact, the reason I wrote the patch to introduce parenthesized options for EXPLAIN was precisely because the unparenthesized option syntax does not scale nicely at all. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Nikolay Samokhvalov
Date:
On Tue, Nov 5, 2024 at 10:19 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Nov 5, 2024 at 1:02 PM Nikolay Samokhvalov
<samokhvalov@gmail.com> wrote:
> hi, I have a proposal, resulted from numerous communications with various folks, both very experienced and new Postgres users:
>
> 1) EXPLAIN ANALYZE Is sometimes very confusing (because there is ANALYZE). Let's rename it to EXPLAIN EXECUTE?
The trouble is that EXPLAIN EXECUTE already means something.
robert.haas=# explain execute foo;
ERROR: prepared statement "foo" does not exist
Granted, that would not make it impossible to make EXPLAIN (EXECUTE) a
synonym for EXPLAIN (ANALYZE), but IMHO it would be pretty confusing
if EXPLAIN EXECUTE and EXPLAIN (EXECUTE) did different things.
> 2) VERBOSE doesn't include BUFFERS, and doesn't include SETTINGS; it might be also confusing sometimes. Let's include them so VERBOSE would be really verbose?
I agree that the naming here isn't great, but I think making the
options non-orthogonal would probably be worse.
> 3) small thing about grammar: allow omitting parentheses, so EXPLAIN EXECUTE VERBOSE would work.
Perhaps surprisingly, it turns out that this is not a small change. As
Tom mentions, this would have a pretty large blast radius. In fact,
the reason I wrote the patch to introduce parenthesized options for
EXPLAIN was precisely because the unparenthesized option syntax does
not scale nicely at all.
I appreciate all yours and Tom's very quick comments here!
Item 3 is already solved, as it turned out.
Let's focus on item 2. Is it really impossible to make VERBOSE really verbose?
Nikolay Samokhvalov <samokhvalov@gmail.com> writes:
> Let's focus on item 2. Is it really impossible to make VERBOSE really
> verbose?
It's obviously not "impossible" -- the code changes would likely be
trivial. The question is whether it's a good idea. These semantics
were (I presume) deliberately chosen when the options were added,
so somebody thought not. You would need to go back and review the
relevant mail thread and then make arguments why that decision
was wrong.
In short: we're not working in a green field here, and all these
decisions have history. You will not get far by just popping
up and saying "I think it should be different". You need to make
a case why the decision was wrong, and why it was so wrong that
we should risk cross-version-compatibility problems by changing.
regards, tom lane
On Tue, Nov 5, 2024 at 1:24 PM Nikolay Samokhvalov <samokhvalov@gmail.com> wrote: > Item 3 is already solved, as it turned out. ANALYZE and VERBOSE are treated specially because those options existed prior to the parenthesized syntax. Scaling that treatment to a large number of options will not work out. > Let's focus on item 2. Is it really impossible to make VERBOSE really verbose? It is, of course, not impossible. But the fact that something is possible does not necessarily mean that it is a good idea. I think it can be quite confusing when the same behavior is controlled in more than one way. If the VERBOSE option turns information about BUFFERS on and off, and the BUFFERS option does the same thing, what happens if I say EXPLAIN (VERBOSE ON, BUFFERS OFF)? Is it different if I say EXPLAIN (BUFFERS OFF, VERBOSE ON)? There's a lot of opportunity for the behavior to be confusing here. Then, too, we can argue about what should be included in VERBOSE. You propose BUFFERS and SETTINGS, but we've also got SERIALIZE (which is not even Boolean-valued), WAL, and MEMORY. One can argue that we ought to include everything when VERBOSE is specified; one can also argue that some of this stuff is too marginal and too high-overhead to justify its inclusion. Both arguments have merit, IMHO. I'm not very happy with the current situation. I agree that EXPLAIN has gotten a bit too complicated. However, I also know that not everyone wants the same things. And I can say from a PostgreSQL support perspective that I do not always want a customer to just "turn on everything", as EXPLAIN output can be extremely long and adding a whole bunch of additional details that make already-long output even longer can easily be actively unhelpful. For me personally, just plain EXPLAIN ANALYZE is usually enough. Sometimes I need VERBOSE to see the target lists at each level, and very occasionally I need BUFFERS to see how much data is being accessed, but at least for me, those are pretty rare cases. So I don't think I really believe the "everybody always wants that" argument. One of the most common things that I have to do with EXPLAIN output is trim the small amounts of relevant material out of the giant pile of things that don't matter to the problem at hand. If you enable an option that adds an extra line of output for every node and there are 100 nodes in the query plan, that is a whole lot of additional clutter. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Nikolay Samokhvalov
Date:
On Tue, Nov 5, 2024 at 10:30 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
we're not working in a green field here, and all these
decisions have history.
I hear you and understand.
Ready to do legwork here.
1. VERBOSE first appeared in 1997 in 6.3 in 3a02ccfa, with different meaning:
> This command [EXPLAIN] outputs details about the supplied query. The default
> output is the computed query cost. \f2verbose\f1 displays the full query
> plan and cost.
2. Support for parenthesis was added in d4382c4a (2009, 8.5), with "test" option COSTS, and this opened gates to extending with many options.
3. BUFFERS was added in d4382c4 (also 2009, 8.5), discussion https://www.postgresql.org/message-id/flat/4AC12A17.5040305%40timbira.com, I didn't see that inclusion it to VERBOSE was discussed.
In my opinion, this option is invaluable: most of the performance optimization is done by reducing IO so seeing these numbers helps make decisions much faster. I always use them. When you optimize and, for example, want to verify an index idea, it's not good to do it on production – it's better to work with clones. There, we can have weaker hardware, different buffer state, etc. So timing numbers might be really off. Timing can be different even on the same server, e.g. after restart, when buffer pool is not warmed up. But BUFFERS never lie – they are not affected by saturated CPU if it happens, lock acquisition waits, etc. Not looking at them is missing an essential part of analysis, I strongly believe.
It looks like in 2009, when the BUFFERS option was created, it was not enough understanding that it is so useful, so it was not discussed to include them by default or at least – as we discuss here – to involve in VERBOSE.
I want to emphasize: BUFFERS is essential in my work and more and more people are convinced that during the optimization process, when you're inside it, in most cases it's beneficial to focus on BUFFERS. Notice that explain.depesz.com, explain.dalibo.com, pgMustard and many tools recognize it and ask users to include BUFFERS to analysis. And see the next item:
4. Making BUFFERS default behavior for EXPLAIN ANALYZE was raised several times, for example https://www.postgresql.org/message-id/flat/CANNMO++=LrJ4upoeydZhbmpd_ZgZjrTLueKSrivn6xmb=yFwQw@mail.gmail.com (2021) – and my understanding that it was received great support and it discussed in detail why it's useful, but then several attempts to implement it were not accomplished because of tech difficulties (as I remember, problem with broken tests and how to fix that).
5. EXPLAIN ALL proposed in https://www.postgresql.org/message-id/flat/080FE841-E38D-42A9-AD6D-48CABED163C9@endpoint.com (2016) – I think it's actually a good idea originally, but didn't survive questions of mutually exclusive options and non-binary options, and then discussion stopped after pivoting in direction of GUC.
6. FInally, the fresh SERIALIZE option was discussed in https://www.postgresql.org/message-id/flat/ca0adb0e-fa4e-c37e-1cd7-91170b18cae1%40gmx.de (2023-2024, 17), and unfortunately again.
I might be missing some discussions – please help me find them; I also expect that there are many people who support me thinking that BUFFERS are very useful and should be default or at least inside VERBOSE. Meanwhile:
- to be able to have all data in hand during analysis, we need to recommend users to collect plans using EXPLAIN (ANALYZE, BUFFERS, VERBOSE, SETTINGS), which looks really long
- independently, I know see pgMustard ended up having a similar recommendation: https://www.pgmustard.com/getting-a-query-plan:
> For better advice, we recommend using at least: explain (analyze, format json, buffers, verbose, settings)
My proposal remains: EXPLAIN ANALYZE VERBOSE -- let's consider this, please.
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Nikolay Samokhvalov
Date:
On Tue, Nov 5, 2024 at 2:54 PM Nikolay Samokhvalov <samokhvalov@gmail.com> wrote:
6. FInally, the fresh SERIALIZE option was discussed in https://www.postgresql.org/message-id/flat/ca0adb0e-fa4e-c37e-1cd7-91170b18cae1%40gmx.de (2023-2024, 17), and unfortunately again.
(didn't finish the phrase here and hit Send)
...again, I don't see that it was discussed to include the SERIALIZE behavior to VERBOSE. I don't use SERIALIZE myself, but during our podcasts, Michael (CCing him) was wondering why it was so.
Summary: I haven't found explicit discussions of including new options to VERBOSE, when that new options were created. I used Google, the .org search, and postgres.ai semantic search over archives involving pgvector/HNSW – I might be missing something, or it was really not discussed when new options were added.
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
"David G. Johnston"
Date:
On Tue, Nov 5, 2024 at 3:55 PM Nikolay Samokhvalov <samokhvalov@gmail.com> wrote:
4. Making BUFFERS default behavior for EXPLAIN ANALYZE was raised several times, for example https://www.postgresql.org/message-id/flat/CANNMO++=LrJ4upoeydZhbmpd_ZgZjrTLueKSrivn6xmb=yFwQw@mail.gmail.com (2021) – and my understanding that it was received great support and it discussed in detail why it's useful, but then several attempts to implement it were not accomplished because of tech difficulties (as I remember, problem with broken tests and how to fix that).
The main premise here is that explain should include buffers by default, and to do so we are willing to inconvenience testers who do not want buffer data in their test plans to have to modify their tests to explicitly exclude buffers. We'll have to eat our own dog food here and go and add "buffers off" throughout our code base to make this happen. I personally feel that we should accept a patch that does so. The benefits to the many outweigh the one-time inconveniencing of the few. Especially if limited to explain analyze.
5. EXPLAIN ALL proposed in https://www.postgresql.org/message-id/flat/080FE841-E38D-42A9-AD6D-48CABED163C9@endpoint.com (2016) – I think it's actually a good idea originally, but didn't survive questions of mutually exclusive options and non-binary options, and then discussion stopped after pivoting in direction of GUC.
If the desire is to make the current keyword VERBOSE behave like the proposed ALL keyword then one must first get a version of ALL accepted, then argue for repurposing VERBOSE instead of adding the new keyword. But at this point I really do not see extending verbose to mean more than "add more comments and context labels". Verbose has never meant to include everything and getting buy-in to change that seems highly unlikely.
In short, neither change is deemed unwanted, and indeed has desire. It's a matter of learning from the previous attempt to increase the odds of getting something committed.
I wouldn't advise expending effort or political capital on the parentheses topic at this point.
David J.
On Tue, Nov 5, 2024 at 1:19 PM Nikolay Samokhvalov <samokhvalov@gmail.com> wrote:
> 2) VERBOSE doesn't include BUFFERS, and doesn't include SETTINGS; it might
> be also confusing sometimes. Let's include them so VERBOSE would be really
> verbose?
This is not likely to fly for compatibility reasons.Can you elaborate?
I am not sure about the compatibility reasons (other than backtesting, or scripts?).
But, personally, as a relatively new person to PG, I was surprised that VERBOSE did not include the buffers.
Could we somehow turn this on? (GUC: VERBOSE_INCLUDES_BUFFERS = yes/no)?
On Wed, 6 Nov 2024 at 13:14, Kirk Wolak <wolakk@gmail.com> wrote: > But, personally, as a relatively new person to PG, I was surprised that VERBOSE did not include the buffers. > Could we somehow turn this on? (GUC: VERBOSE_INCLUDES_BUFFERS = yes/no)? Please read https://postgr.es/m/CA+TgmoYH_p-y=45SAJ58cU6jsMH6ojgqQZiA2aePpvZ0J+uLbA@mail.gmail.com David
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Michael Christofides
Date:
I'm not against analyze = on turning buffers on by default. However, I
think it would be quite painful to fix the tests if it were on without
analyze.
This would be amazing. I'm finding BUFFERS are especially helpful for
giving developers a clearer idea of why their queries are slow (especially
once converted to KB/MB/GB/TB).
> The trouble is that EXPLAIN EXECUTE already means something.
I completely agree with this. So -1 from me on the first suggestion.
> Let's focus on item 2.
+1 from me on item 2. I'd go further and have VERBOSE flip most other
parameters to on (or to their default for non-booleans), unless specified
otherwise. Specifically SUMMARY, BUFFERS, SETTINGS, WAL,
SERIALIZE (TEXT), and MEMORY. Although I do think it would be best if
BUFFERS and SERIALIZE were ON and TEXT by default respectively with
ANALYZE, which may reduce/remove the need for them to be affected by
VERBOSE.
> If the VERBOSE option turns information about BUFFERS on
> and off, and the BUFFERS option does the same thing, what happens if I> say EXPLAIN (VERBOSE ON, BUFFERS OFF)? Is it different if I say
> EXPLAIN (BUFFERS OFF, VERBOSE ON)?
I'd expect this to work like other parameters that have dependencies, for
example both EXPLAIN (ANALYZE, SUMMARY OFF) and EXPLAIN
(SUMMARY OFF, ANALYZE) exclude the SUMMARY, even though it is
on by default with ANALYZE. So users could turn off anything they don't
want, if needed.
> I'm not very happy with the current situation. I agree that EXPLAIN
has gotten a bit too complicated.I agree. In the past 6 versions, 5 new parameters have been added.
SETTINGS in v12, WAL in v13, GENERIC_PLAN in v16, SERIALIZE in
v17, and MEMORY in v17. It feels like we should have some easier way
to get everything. Currently, we need to specify: EXPLAIN (ANALYZE,
VERBOSE, BUFFERS, SETTINGS, WAL, SERIALIZE, MEMORY).
> If you enable an option that adds an extra line of
> output for every node and there are 100 nodes in the query plan, that> is a whole lot of additional clutter.
This is a fair point, but I think it is worth it in the case of BUFFERS. The
other parameter that adds a line per node is WAL, but the others don't
add much clutter.
Many people use tools these days to help read plans (I work on one,
so have some biased opinions of course). Tools help folks calculate
timings and spot bottlenecks , so once you're using a tool to read a plan,
more information is often beneficial for minimal overhead.
> This is not likely to fly for compatibility reasons.
> (2023-2024, 17)
I'd be interested to hear more on this front too. One issue is that folks
with auto_explain.log_verbose = on would get extra output in their logs,
but I strongly suspect I'm missing some more important things.
> the fresh SERIALIZE option was discussed in
> https://www.postgresql.org/message-id/flat/ca0adb0e-fa4e-c37e-1cd7-91170b18cae1%40gmx.de> (2023-2024, 17)
I noticed in this thread Tom was against SERIALIZE being on by default
with ANALYZE, "because it would silently render EXPLAIN outputs from
different versions quite non-comparable." I'm not sure I agree with the
silently part, as the output from 17+ would include the serialization details,
but again perhaps I'm missing something important.
> Ready to do legwork here.
—
Michael Christofides
Founder, pgMustard
Michael Christofides
Founder, pgMustard
On 05.11.24 19:19, Robert Haas wrote: >> 1) EXPLAIN ANALYZE Is sometimes very confusing (because there is ANALYZE). Let's rename it to EXPLAIN EXECUTE? > The trouble is that EXPLAIN EXECUTE already means something. > > robert.haas=# explain execute foo; > ERROR: prepared statement "foo" does not exist > > Granted, that would not make it impossible to make EXPLAIN (EXECUTE) a > synonym for EXPLAIN (ANALYZE), but IMHO it would be pretty confusing > if EXPLAIN EXECUTE and EXPLAIN (EXECUTE) did different things. At some point in the past, the idea of renaming EXPLAIN ANALYZE to PROFILE was thrown around. I still kind of like that idea. You'd have to keep the existing syntax around, of course.
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Guillaume Lelarge
Date:
Hi,
Le mer. 6 nov. 2024 à 17:57, Michael Christofides <michael@pgmustard.com> a écrit :
[...]I agree. In the past 6 versions, 5 new parameters have been added.SETTINGS in v12, WAL in v13, GENERIC_PLAN in v16, SERIALIZE inv17, and MEMORY in v17. It feels like we should have some easier wayto get everything. Currently, we need to specify: EXPLAIN (ANALYZE,VERBOSE, BUFFERS, SETTINGS, WAL, SERIALIZE, MEMORY).
Agreed. Having an "EXPLAIN (ALL)" would be a great addition. I could tell a customer to do an "EXPLAIN (ALL)", rather than first asking the PostgreSQL release installed on the server and after that, giving the correct options for EXPLAIN.
--
Guillaume.
On Mon, Nov 11, 2024 at 3:59 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Agreed. Having an "EXPLAIN (ALL)" would be a great addition. I could tell a customer to do an "EXPLAIN (ALL)", rather
thanfirst asking the PostgreSQL release installed on the server and after that, giving the correct options for EXPLAIN.
I realize that you're probably going to hate my guts -- or hate them
even more than you do already -- but I doubt that a proposal to add
EXPLAIN (ALL) will go anywhere. The definitional problem is that it is
not clear what to do with non-Boolean valued options, such as
SERIALIZE. People who think that we were wrong not to make SERIALIZE
TEXT the default in v17 will argue that EXPLAIN (ALL) should turn it
on; after all, the backward-compatibility argument carries no water in
that case. But people who do not like the behavior of SERIALIZE TEXT
will not be happy about that. They might directly make that argument,
or they might instead make the argument that ALL should do nothing
about a non-Boolean valued option. But that position is really quite
difficult to justify. Let's suppose that the current BUFFERS option,
which is Boolean, got replaced with BUFFERS { detailed | on | off }.
Well, then, by the principle that ALL only affects Boolean-valued
options, it's no longer included in EXPLAIN (ALL). Nobody will be
happy with that. Practically speaking, I think it will be very
difficult to get agreement on what EXPLAIN (ALL) should do, and I
think it is unlikely that anything will get committed no matter how
much time we spend arguing about it.
But I think we would get most of the same benefit from just doing what
David Rowley proposed and turning on EXPLAIN (BUFFERS) by default. I'd
suggest that we decide that, without ANALYZE, the option would not do
anything; that is already how TIMING works. So this would be a very
small patch and would probably get a lot of support from a lot of
people. It also wouldn't require users to change their habits or learn
any new syntax -- they could just keep typing EXPLAIN ANALYZE or
EXPLAIN ANALYZE VERBOSE and all would be well.
And the same principle could be applied to other EXPLAIN options if
there is sufficient consensus. We could default to WAL ON, SERIALIZE
TEXT, and MEMORY ON, if we wanted to do that. However, the more we try
to change at once, the less likely it is that anything will happen at
all. For example, I personally believe that EXPLAIN (MEMORY) should be
ripped out of the server as both badly-named and mostly useless, so
I'm not going to vote in favor of turning it on by default; and I
wouldn't vote for enabling WAL by default because I have no experience
with it to suggest that it's routinely valuable and thus worth the
overhead. I would vote for SERIALIZE TEXT because I've seen that cause
gross distortion of EXPLAIN ANALYZE results on many occasions. But the
point is that other people will vote differently, so tying all the
proposals together just increases the chances of agreeing on nothing
at all.
So to recap: everyone is free to propose whatever they like, and I am
not in charge here, but if you want to get something committed, the
proposal which I think has the highest chance of success is: propose
to make BUFFERS ON the default (but a noop without ANALYZE, similar to
how TIMING already works).
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Guillaume Lelarge
Date:
Le mar. 12 nov. 2024 à 16:21, Robert Haas <robertmhaas@gmail.com> a écrit :
On Mon, Nov 11, 2024 at 3:59 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Agreed. Having an "EXPLAIN (ALL)" would be a great addition. I could tell a customer to do an "EXPLAIN (ALL)", rather than first asking the PostgreSQL release installed on the server and after that, giving the correct options for EXPLAIN.
I realize that you're probably going to hate my guts -- or hate them
even more than you do already -- but I doubt that a proposal to add
EXPLAIN (ALL) will go anywhere.
I don't hate your guts :) and...
The definitional problem is that it is
not clear what to do with non-Boolean valued options, such as
SERIALIZE. People who think that we were wrong not to make SERIALIZE
TEXT the default in v17 will argue that EXPLAIN (ALL) should turn it
on; after all, the backward-compatibility argument carries no water in
that case. But people who do not like the behavior of SERIALIZE TEXT
will not be happy about that. They might directly make that argument,
or they might instead make the argument that ALL should do nothing
about a non-Boolean valued option. But that position is really quite
difficult to justify. Let's suppose that the current BUFFERS option,
which is Boolean, got replaced with BUFFERS { detailed | on | off }.
Well, then, by the principle that ALL only affects Boolean-valued
options, it's no longer included in EXPLAIN (ALL). Nobody will be
happy with that. Practically speaking, I think it will be very
difficult to get agreement on what EXPLAIN (ALL) should do, and I
think it is unlikely that anything will get committed no matter how
much time we spend arguing about it.
... I kinda agree with you. It would have been nice to have an "EXPLAIN (ALL)" but I completely understand the issue.
But I think we would get most of the same benefit from just doing what
David Rowley proposed and turning on EXPLAIN (BUFFERS) by default. I'd
suggest that we decide that, without ANALYZE, the option would not do
anything; that is already how TIMING works. So this would be a very
small patch and would probably get a lot of support from a lot of
people. It also wouldn't require users to change their habits or learn
any new syntax -- they could just keep typing EXPLAIN ANALYZE or
EXPLAIN ANALYZE VERBOSE and all would be well.
That would be a nice enhancement.
And the same principle could be applied to other EXPLAIN options if
there is sufficient consensus. We could default to WAL ON, SERIALIZE
TEXT, and MEMORY ON, if we wanted to do that. However, the more we try
to change at once, the less likely it is that anything will happen at
all. For example, I personally believe that EXPLAIN (MEMORY) should be
ripped out of the server as both badly-named and mostly useless, so
I'm not going to vote in favor of turning it on by default; and I
wouldn't vote for enabling WAL by default because I have no experience
with it to suggest that it's routinely valuable and thus worth the
overhead. I would vote for SERIALIZE TEXT because I've seen that cause
gross distortion of EXPLAIN ANALYZE results on many occasions. But the
point is that other people will vote differently, so tying all the
proposals together just increases the chances of agreeing on nothing
at all.
Agreed.
So to recap: everyone is free to propose whatever they like, and I am
not in charge here, but if you want to get something committed, the
proposal which I think has the highest chance of success is: propose
to make BUFFERS ON the default (but a noop without ANALYZE, similar to
how TIMING already works).
Sounds like a plan.
Thanks.
--
Guillaume.
On Tue, Nov 12, 2024 at 4:02 PM Guillaume Lelarge <guillaume@lelarge.info> wrote: > Sure looks easy enough to do (though it still lacks doc and tests changes). See patch attached. Yep, that's very small. I'm a bit wondering if it's too small, though. standard_ExplainOneQuery() seems to do some stuff with es->buffers even before it does planning, so if the idea is that this will be a noop without ANALYZE, maybe this doesn't implement that. Also, you should probably update the default value for auto_explain.log_buffers. In general, I would recommend "git grep 'es->buffers'" and look carefully at each place where it's mentioned and decide if anything needs to be changed. And then change the stuff that needs it, and include in your email an explanation of why the other things don't need to be changed, unless it's obvious. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Greg Sabino Mullane
Date:
I'm with Robert on this one. I'm a grudging +1 to buffers defaulting on, and a strong -1 to all the other proposals. Guillaume, the patch looks pretty good. I would like to see some of the example output have more than just "shared hit" and "read" though: let's throw some "dirtied" and "written" in there as well.
It defaults to <literal>TRUE</literal> when <literal>ANALYZE</literal> is also enabled. Otherwise, it defaults to <literal>FALSE</literal>.
Is that second sentence really needed? Because "BUFFERS ON" will never be needed anymore (save as a no-op to allow the same explain queries to run cross-version), and BUFFERS OFF outside of analyze is meaningless.
Cheers,
Greg
On Wed, 20 Nov 2024 at 13:13, Greg Sabino Mullane <htamfids@gmail.com> wrote: >> It defaults to <literal>TRUE</literal> when <literal>ANALYZE</literal> is also enabled. Otherwise, it defaults to <literal>FALSE</literal>. > > Is that second sentence really needed? Because "BUFFERS ON" will never be needed anymore (save as a no-op to allow thesame explain queries to run cross-version), and BUFFERS OFF outside of analyze is meaningless. "BUFFERS ON" will be needed if the user wants to see the buffer usage in the planner when ANALYZE isn't specified. You'll see the planner access buffers when the caches are not fully populated and for get_actual_variable_range() work. Maybe the wording could just be based on the wording for the SUMMARY option, i.e. "Summary information is included by default when ANALYZE is used but otherwise is not included by default". David
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Greg Sabino Mullane
Date:
Maybe the wording could just be based on the wording for the SUMMARY
option, i.e. "Summary information is included by default when ANALYZE
is used but otherwise is not included by default".
Yes, I like that. Avoids the whole TRUE/FALSE altogether.
Thanks,
Greg
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Greg Sabino Mullane
Date:
On Wed, Nov 20, 2024 at 1:26 AM Guillaume Lelarge <guillaume@lelarge.info> wrote:
OK, but I'm not sure which example I should pick to add dirtied and written shared buffers. It looks kinda artificial. Should I choose one randomly?
It will be artificial, but I think that's ok: anyone running it on their own will be getting different numbers anyway. I was looking at the "14.1.2 EXPLAIN ANALYZE" section in perform.sgml. Here's some actual numbers I got with some playing around with concurrent updates:
Recheck Cond: (unique1 < 10)
Heap Blocks: exact=5
Buffers: shared hit=2 read=5 written=4
...
Planning:
Buffers: shared hit=289 dirtied=9
Cheers,
Greg
On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote: > OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers. I think this might be a good time for anyone out there who is against turning on BUFFERS when ANALYZE is on to speak up. Votes for changing this so far seem to be: Me, Michael Christofides, Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from 2020) [1]. Votes against: None David [1] https://www.postgresql.org/message-id/20200601010025.GS13741%40fetter.org
On Thu, Nov 21, 2024 at 10:22:54AM +1300, David Rowley wrote: > On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers. > > I think this might be a good time for anyone out there who is against > turning on BUFFERS when ANALYZE is on to speak up. > > Votes for changing this so far seem to be: Me, Michael Christofides, > Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from > 2020) [1]. Please add me to the above list. What we have now is too confusing. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com When a patient asks the doctor, "Am I going to die?", he means "Am I going to die soon?"
On 20/11/2024 22:22, David Rowley wrote: > On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers. > I think this might be a good time for anyone out there who is against > turning on BUFFERS when ANALYZE is on to speak up. > > Votes for changing this so far seem to be: Me, Michael Christofides, > Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from > 2020) [1]. Add me to the pro list, please. https://www.postgresql.org/message-id/b3197ba8-225f-f53c-326d-5b1756c77c3e@postgresfriends.org -- Vik Fearing
On Wed, 2024-11-20 at 22:54 +0100, Vik Fearing wrote: > On 20/11/2024 22:22, David Rowley wrote: > > On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > > OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers. > > I think this might be a good time for anyone out there who is against > > turning on BUFFERS when ANALYZE is on to speak up. > > > > Votes for changing this so far seem to be: Me, Michael Christofides, > > Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from > > 2020) [1]. > > Add me to the pro list, please. > > https://www.postgresql.org/message-id/b3197ba8-225f-f53c-326d-5b1756c77c3e@postgresfriends.org Me, too! https://postgr.es/m/790e175d16cca11244907d3366a6fa376c33e882.camel@cybertec.at Yours, Laurenz Albe
On Wed, Nov 20, 2024 at 4:23 PM David Rowley <dgrowleyml@gmail.com> wrote: > On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote: > > OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers. > > I think this might be a good time for anyone out there who is against > turning on BUFFERS when ANALYZE is on to speak up. > > Votes for changing this so far seem to be: Me, Michael Christofides, > Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from > 2020) [1]. > > Votes against: None Just to be clear, my vote is more like +0 than +1. I recognize that changing this is popular and I'm not opposed to it, but I'm also not unhappy with things as they are. -- Robert Haas EDB: http://www.enterprisedb.com
On Tue, 26 Nov 2024 at 09:44, Robert Haas <robertmhaas@gmail.com> wrote: > > On Wed, Nov 20, 2024 at 4:23 PM David Rowley <dgrowleyml@gmail.com> wrote: > > I think this might be a good time for anyone out there who is against > > turning on BUFFERS when ANALYZE is on to speak up. > > > > Votes for changing this so far seem to be: Me, Michael Christofides, > > Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from > > 2020) [1]. > > > > Votes against: None > > Just to be clear, my vote is more like +0 than +1. I recognize that > changing this is popular and I'm not opposed to it, but I'm also not > unhappy with things as they are. OK, thanks. I likely took your neutrality here in a more positive light after reading the 4-year-old message at [1]. That probably wasn't the right thing to do. There are still no votes against it, so unless some come in, I plan to start looking at the patches proposed to turn buffers on with analyze with my committer hat on. David [1] https://www.postgresql.org/message-id/CA%2BTgmoYH_p-y%3D45SAJ58cU6jsMH6ojgqQZiA2aePpvZ0J%2BuLbA%40mail.gmail.com
David Rowley <dgrowleyml@gmail.com> writes:
> There are still no votes against it, so unless some come in, I plan to
> start looking at the patches proposed to turn buffers on with analyze
> with my committer hat on.
I'm kind of -0.5, but I'd not bothered to vote because it was
pretty clear what the result was going to be.
regards, tom lane
On Tue, 26 Nov 2024 at 12:12, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm kind of -0.5, but I'd not bothered to vote because it was > pretty clear what the result was going to be. Just so I can properly understand, how much of the -0.5 is "don't like the idea" vs "rather make changes to well-known behaviour"? David
David Rowley <dgrowleyml@gmail.com> writes:
> On Tue, 26 Nov 2024 at 12:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm kind of -0.5, but I'd not bothered to vote because it was
>> pretty clear what the result was going to be.
> Just so I can properly understand, how much of the -0.5 is "don't like
> the idea" vs "rather make changes to well-known behaviour"?
Well, I'm with Robert in that I've not found the buffer counts to be
all that useful most of the time. I grant that they're sometimes
useful, but that's not enough to justify them being on by default:
EXPLAIN output is cluttered enough already. And I'm also not
thrilled with changing longstanding behavior, but that's a
relatively minor point in this case.
regards, tom lane
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Daniel Gustafsson
Date:
> On 20 Nov 2024, at 22:22, David Rowley <dgrowleyml@gmail.com> wrote: > > On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers. > > I think this might be a good time for anyone out there who is against > turning on BUFFERS when ANALYZE is on to speak up. > > Votes for changing this so far seem to be: Me, Michael Christofides, > Guillaume Lelarge, Robert, Greg Sabino Mullane, and David Fetter (from > 2020) [1]. I would also like to see this on by default, while it does make it more verbose the extra data is valuable in debugging. +1 from me. -- Daniel Gustafsson
On Mon, Nov 25, 2024 at 5:52 PM David Rowley <dgrowleyml@gmail.com> wrote: > OK, thanks. I likely took your neutrality here in a more positive > light after reading the 4-year-old message at [1]. That probably > wasn't the right thing to do. Yeah, I think my view has wavered over time. Probably when I wrote that message I had recently seen some cases where the additional detail would have been helpful; but as of this writing the cases I've seen most recently are those where the output is mind-numbingly long already. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Michael Christofides
Date:
> v4 patch attached
Thank you Guillaume, and nice work! I tried to see if there was anywhere else in the documentation that would need updating, but it looks like you covered everywhere already.
> I'm with Robert in that I've not found the buffer counts to be all that useful most of the time.
I find the buffer counts especially helpful for educating newer folks on why things are slow, even when they are not necessary for spotting the issue (for more advanced users). One of my hopes is that by educating and empowering newer users on how I/O relates to performance issues, fewer cases will get escalated to more experienced folks.
> the cases I've seen most recently are those where the output is mind-numbingly long already.
Are you mostly seeing query plans that have stumped other people already (eg second or third line support), so perhaps seeing more complex plans than the average user?
Both Depesz[1] and Tensor[2] have archives of publicly submitted plans, which I found helpful for checking how slow plans look for users of those tools. I have a similar archive, and while we do not publish them (and there are plenty of huge plans) it also suggests that the majority of slow plans people are reviewing have fewer than 20 nodes.
I realise it’s optimistic to think that the time experienced hackers would lose having to sift through longer plans would be gained back by having to do so less often, but I thought it was worth raising as part of the aim.
I also looked into the Slow Query Questions page on the wiki that we ask people to review before posting to pgsql-performance, and noticed that has suggested requesting buffers for the past 12 years[3].
—
Michael
Michael
On Mon, Dec 2, 2024 at 6:42 AM Michael Christofides <michael@pgmustard.com> wrote: > Are you mostly seeing query plans that have stumped other people already (eg second or third line support), so perhapsseeing more complex plans than the average user? Yes. :-) -- Robert Haas EDB: http://www.enterprisedb.com
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Guillaume Lelarge
Date:
Hello,
Le mar. 10 déc. 2024 à 03:57, David Rowley <dgrowleyml@gmail.com> a écrit :
On Thu, 21 Nov 2024 at 08:30, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> OK, I'm fine with this. v4 patch attached with one plan showing read, written, and dirtied buffers.
Today I spent more time polishing this patch. There were a few cases
in the docs that displayed EXPLAIN ANALYZE output that you'd not
adjusted to include the buffers output or adjusted to do BUFFERS OFF.
I think I've got all these now. Tom went to some effort to fix some
outdated EXPLAIN outputs for v17 in 5caa05749, so I think we owe it to
him not to let these go out of date so soon after that change.
You're right and I completely forgot to check the whole documentation. I just looked at perform.sgml which was the obvious file for explain plans. Anyway, sorry about this, and thanks a lot for your work on this patch.
I also was thinking again about what Robert mentioned about
auto_explain.log_buffers should now also be on by default. I'm less
certain than him about this change. It seems like a separate
consideration that we could apply many of the same arguments for the
main change to. In any case, I extracted that change from the 0001
patch and put it in a 0002 patch as it doesn't seem like something
that should be a sidenote in the commit message. I felt doing that
increases the chances that it would be overlooked in the release
notes.
I was very close to pushing 0001 today, but will hold off until
tomorrow to see if anyone has final comments.
No more comments. I'm fine with both patches.
For 0002, I'd really like to see a bit more justification for it. For
the record, I'm not against 0002, it's just that my personal arguments
for wanting 0001 don't apply to 0002.
I guess consistency is the key word here. But I agree that 0001 is the one that's really important to me.
Thanks again for your work on this.
Guillaume.
On Wed, 11 Dec 2024 at 01:22, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Le mar. 10 déc. 2024 à 03:57, David Rowley <dgrowleyml@gmail.com> a écrit : >> I was very close to pushing 0001 today, but will hold off until >> tomorrow to see if anyone has final comments. >> > No more comments. I'm fine with both patches. Thanks for looking. I've pushed the main patch. I'm not planning on pushing the auto_explain.log_buffers default change unless there's a bit more discussion about it. David
On Wed, 11 Dec 2024 at 22:38, David Rowley <dgrowleyml@gmail.com> wrote: > I've pushed the main patch. I'm not planning on pushing the > auto_explain.log_buffers default change unless there's a bit more > discussion about it. There were a few places that were missing a BUFFERS OFF as indicated by the buildfarm. I've fixed a few of these, but there's at least one more in pg_stat_statements's level_tracking.sql. I'm currently running a -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE enabled make check-world. It's going to take a while, so I'll hold off doing yet another commit to fix these until I see if the level_tracking.sql one is the last issue to fix. David
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Guillaume Lelarge
Date:
Le mer. 11 déc. 2024 à 12:16, David Rowley <dgrowleyml@gmail.com> a écrit :
On Wed, 11 Dec 2024 at 22:38, David Rowley <dgrowleyml@gmail.com> wrote:
> I've pushed the main patch. I'm not planning on pushing the
> auto_explain.log_buffers default change unless there's a bit more
> discussion about it.
There were a few places that were missing a BUFFERS OFF as indicated
by the buildfarm. I've fixed a few of these, but there's at least one
more in pg_stat_statements's level_tracking.sql. I'm currently running
a -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE enabled make
check-world. It's going to take a while, so I'll hold off doing yet
another commit to fix these until I see if the level_tracking.sql one
is the last issue to fix.
Sorry for all those late fixes. I'll refrain from saying "Sure looks easy enough to do" from now on :) Guess this is a great example of what Robert said in http://rhaas.blogspot.com/2024/05/hacking-on-postgresql-is-really-hard.html .
Anyway, thanks again for your work on this.
Guillaume.
do we also need to update doc/src/sgml/rules.sgml? https://www.postgresql.org/docs/current/rules-materializedviews.html
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Jelte Fennema-Nio
Date:
On Wed, 11 Dec 2024 at 10:38, David Rowley <dgrowleyml@gmail.com> wrote: > I've pushed the main patch. I'm not planning on pushing the > auto_explain.log_buffers default change unless there's a bit more > discussion about it. FreeBSD CFBot seems broken on a pg_stat_statements test because of this change: https://api.cirrus-ci.com/v1/artifact/task/4845908522696704/testrun/build/testrun/pg_stat_statements/regress/regression.diffs
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Guillaume Lelarge
Date:
Le mer. 11 déc. 2024 à 21:41, Jelte Fennema-Nio <postgres@jeltef.nl> a écrit :
On Wed, 11 Dec 2024 at 10:38, David Rowley <dgrowleyml@gmail.com> wrote:
> I've pushed the main patch. I'm not planning on pushing the
> auto_explain.log_buffers default change unless there's a bit more
> discussion about it.
FreeBSD CFBot seems broken on a pg_stat_statements test because of
this change: https://api.cirrus-ci.com/v1/artifact/task/4845908522696704/testrun/build/testrun/pg_stat_statements/regress/regression.diffs
Thanks for the info. Peter sent an email on the pgsql-committers list to say the same thing. I wrote a patch for this, and sent it to the same list. See https://www.postgresql.org/message-id/CAECtzeVXyRruRODFXZ67goMOYtieLbLmcUY6P5L_LrEq8b%3DgxQ%40mail.gmail.com for more details.
--
Guillaume.
On Thu, 12 Dec 2024 at 00:16, David Rowley <dgrowleyml@gmail.com> wrote: > There were a few places that were missing a BUFFERS OFF as indicated > by the buildfarm. I've fixed a few of these, but there's at least one > more in pg_stat_statements's level_tracking.sql. I'm currently running > a -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE enabled make > check-world. It's going to take a while, so I'll hold off doing yet > another commit to fix these until I see if the level_tracking.sql one > is the last issue to fix. That run didn't show any other failures, so I pushed the patch. David
Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
From
Michael Christofides
Date:
> I've pushed the main patch.
Thanks again all,
Woohoo! And thank you. I've already seen quite a lot of positivity around the commit on Twitter[1][2][3].
> I'm not planning on pushing the auto_explain.log_buffers default change unless there's a bit more discussion about it.
Much like Guillaume, I'd also be in favour of 0002, but it's nowhere near as important to me. I think most people consider the parameters far more when setting up auto_explain, so I believe far fewer omit buffers by mistake. Also, most cloud providers don't ship with auto_explain on, and the only one I know of that does[4], ships with log_buffers on too. On the plus side, it would be nice to be consistent. But on the downside, it might add a little extra overhead for folks who run auto_explain with log_analyze on, and who opted not to set log_buffers and upgrade without setting it to off explicitly. I am still in favour of the 0002 patch being applied, to avoid confusion and maximise the chance people that don't know about buffers still get them in their plans.
> do we also need to update doc/src/sgml/rules.sgml?
> do we also need to update doc/src/sgml/rules.sgml?
Good catch. Testing those file_fdw queries locally, I don't see buffers reported by the Foreign Scan, but I do initially see some Planning buffers (on first run). The two plans from the queries on the materialized view do show buffers now though, of course. Since the file_fdw Foreign Scan is not reporting buffers, I'm wondering if in this one case simply changing "With EXPLAIN ANALYZE" to "With EXPLAIN (ANALYZE, BUFFERS OFF)" might be the least confusing solution?
Thanks again all,
Michael