Content-Type: text/plain; charset=UTF-8
Date: Mon, 06 Apr 2026 16:27:37 -0300
Message-Id: <DHMBIO0G7WL4.1EMZ7AG9ZS148@gmail.com>
Subject: Re: Add custom EXPLAIN options support to auto_explain
From: "Matheus Alcantara" <matheusssilv97@gmail.com>
To: "Lukas Fittl" <lukas@fittl.com>, "Robert Haas" <robertmhaas@gmail.com>
Cc: <pgsql-hackers@lists.postgresql.org>, "Tom Lane" <tgl@sss.pgh.pa.us>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
References: <DHCYFNC0WIF4.3T1M8AUK0DG7T@gmail.com>
 <CA+Tgmob-0W8306mvrJX5Urtqt1AAasu8pi4yLrZ1XfwZU-Uj1w@mail.gmail.com>
 <CA+TgmoYUdeCdGfk8H6Ni2obXVixLvYaDkRGtxKLEmaCVNffsVA@mail.gmail.com>
 <DHHB7U2PSFYG.1A0WVPTX5LNOD@gmail.com>
 <CA+TgmoacOujCREMtQwURTSokB+ks-eLgaQ5vBMdWG959XzFcrA@mail.gmail.com>
 <CAP53PkwsnhmoLXJaUMKWc76S21GzYOX06qKyZdVZhGGW0rCWKw@mail.gmail.com>
Content-Language: en-US
Fcc: imap://matheusssilv97%40gmail.com@imap.gmail.com/[Gmail]/Sent Mail
In-Reply-To: 
 <CAP53PkwsnhmoLXJaUMKWc76S21GzYOX06qKyZdVZhGGW0rCWKw@mail.gmail.com>
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/DHMBIO0G7WL4.1EMZ7AG9ZS148%40gmail.com>
Precedence: bulk

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.gmail.com

--
Matheus Alcantara
EDB: https://www.enterprisedb.com