Re: [PATCH] Improve ability to display optimizer analysis using OPTIMIZER_DEBUG

From: Vladimir Churyukin <vladimir(at)churyukin(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Ankit Kumar Pandey <itsankitkp(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pghackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Improve ability to display optimizer analysis using OPTIMIZER_DEBUG
Date: 2023-01-04 04:39:19
Message-ID: CAFSGpE35ryyjGprjQ2WMVrvfJotwSdpfv6RKN5bLVhPDYNueDA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 3, 2023 at 7:41 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Wed, 4 Jan 2023 at 16:15, Vladimir Churyukin <vladimir(at)churyukin(dot)com>
> wrote:
> > As an end user that spends a lot of time optimizing pretty complicated
> queries, I'd say that something like this could be useful.
>
> I think we really need to at least see that it *is* useful, not that
> it *could be* useful. For example, as an end user, you might not find
> it great that the output is sent to stdout rather than to the window
> that you execute the query in.
>

That's true, as an end user I would expect to see the output as a query
output, not in stdout.

> From what I can see here, the motivation to make this a useful feature
> is backwards from what is normal. I think if you're keen to see a
> feature that allows you better visibility into rejected paths then you
> need to prove this is it rather than speculating that it might be
> useful.
>
>
You can't see people using the feature unless you make it useful. If it's
not useful right now (because it's implemented as a compile-time flag with
stdout prints for example),
it doesn't mean it's not useful when it becomes more convenient. Probably
the best way to find out is to create a *convenient* extension and see if
people start using it.

> There was a bit of work being done in [1] with the end goal of having
> the ability for add_path to call a hook function before it outright
> rejects a path. Maybe that would be a better place to put this and
> then write some contrib module that provides some extended output in
> EXPLAIN. That might require some additional fields so that we could
> carry forward additional information that we'd like to show in
> EXPLAIN. I imagine it's not ok just to start writing result lines in
> the planner. The EXPLAIN format must be considered too and explain.c
> seems like the place that should be done. add_path might need to
> become a bit more verbose about the reason it rejected a certain path
> for this to be useful.
>

I agree, extended EXPLAIN output would be a much better solution than
writing into stdout. Can be implemented as an extra EXPLAIN flag, something
like EXPLAIN (TRACE).
One of the issues here is the result will rather be pretty long (and may
consist of multiple parts, so something like returning multiple
refcursors might be necessary, so a client can fetch multiple result sets.
Otherwise it won't be human-readable. Although it's not necessary the
purpose, if the purpose is to make it machine-readable and create tools to
interpret the results, json format and a single resultset would be ok.
The result can be represented as a list of trace events that shows profiler
logic (the traces can be generated by the hook you mentioned and or by some
other additional hooks).
Is that what you were talking about?
Another thing, since people react to this TODO item on
https://wiki.postgresql.org/wiki/Todo, maybe it's better to modify
or remove it, so they don't spend time working on something that is pretty
much a dead end currently?

-Vladimir

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2023-01-04 04:56:20 Re: [PATCH] CF app: add "Returned: Needs more interest"
Previous Message Peter Geoghegan 2023-01-04 04:29:53 Re: pgsql: Delay commit status checks until freezing executes.