public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matheus Alcantara <[email protected]>
To: Lukas Fittl <[email protected]>
To: Robert Haas <[email protected]>
Cc: [email protected]
Cc: Tom Lane <[email protected]>
Subject: Re: Add custom EXPLAIN options support to auto_explain
Date: Mon, 06 Apr 2026 16:27:37 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAP53PkwsnhmoLXJaUMKWc76S21GzYOX06qKyZdVZhGGW0rCWKw@mail.gmail.com>
References: <[email protected]>
<CA+Tgmob-0W8306mvrJX5Urtqt1AAasu8pi4yLrZ1XfwZU-Uj1w@mail.gmail.com>
<CA+TgmoYUdeCdGfk8H6Ni2obXVixLvYaDkRGtxKLEmaCVNffsVA@mail.gmail.com>
<[email protected]>
<CA+TgmoacOujCREMtQwURTSokB+ks-eLgaQ5vBMdWG959XzFcrA@mail.gmail.com>
<CAP53PkwsnhmoLXJaUMKWc76S21GzYOX06qKyZdVZhGGW0rCWKw@mail.gmail.com>
On 03/04/26 23:09, Lukas Fittl wrote:
>> +static int
>> +auto_explain_split_options(char *rawstring, auto_explain_option *options,
>> + int maxoptions, char **errmsg)
>> +{
>
> I think this function probably has the most complexity due to its
> hand-rolled parsing, so it might be good if someone else takes another
> close look at it.
>
> FWIW, I couldn't find anything wrong from a first read.
>
> Otherwise 0003 looks good to me.
>
I did another review of 0003 (0001 and 0002 are now committed), focusing
on auto_explain_split_options(). I didn't find any issues. The code
coverage looks good - auto_explain_split_options() has nearly 100%
coverage and the new code paths are well exercised by the tests.
Regarding the shared_preload_libraries ordering concern: I don't think
there's much we can do from the GUC perspective, I mean, we're
essentially building an extension dependency chain when
auto_explain.log_extension_options references options from other
extensions. If I configure the GUC with 'plan_advice', I need to ensure
pg_plan_advice is loaded before auto_explain. The documentation makes
this clear.
My concern is about that some cloud providers expose
shared_preload_libraries as a dropdown without user control over
ordering. I can be totally wrong, but it seems to me that in this case,
the provider would need to handle dependencies appropriately or have a
way to let the user define the ordering. Or, a possible improvement
would be a post-configuration validation hook that runs after all
shared_preload_libraries are loaded, allowing deferred validation of
cross-extension dependencies like these EXPLAIN options (I'm wondering
that we can have more extension dependencies in the future, e.g
plan_advice and pg_stat_statements [1])
That said, I think we should proceed with 0003 as-is and revisit this
when real-world usage reveals such problems in practice.
[1] https://www.postgresql.org/message-id/CA%2BTgmobtbB74PTUtCQwUNR8D9ZZwKH5ZptoU30ONm34TR954Nw%40mail.g...
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
view thread (16+ 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: Add custom EXPLAIN options support to auto_explain
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