Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Tue, 31 Mar 2026 19:10:08 -0300
Message-Id: <DHHB7U2PSFYG.1A0WVPTX5LNOD@gmail.com>
Cc: <pgsql-hackers@lists.postgresql.org>, "Lukas Fittl" <lukas@fittl.com>,
 "Tom Lane" <tgl@sss.pgh.pa.us>
Subject: Re: Add custom EXPLAIN options support to auto_explain
From: "Matheus Alcantara" <matheusssilv97@gmail.com>
To: "Robert Haas" <robertmhaas@gmail.com>
References: <DHCYFNC0WIF4.3T1M8AUK0DG7T@gmail.com>
 <CA+Tgmob-0W8306mvrJX5Urtqt1AAasu8pi4yLrZ1XfwZU-Uj1w@mail.gmail.com>
 <CA+TgmoYUdeCdGfk8H6Ni2obXVixLvYaDkRGtxKLEmaCVNffsVA@mail.gmail.com>
In-Reply-To: 
 <CA+TgmoYUdeCdGfk8H6Ni2obXVixLvYaDkRGtxKLEmaCVNffsVA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/DHHB7U2PSFYG.1A0WVPTX5LNOD%40gmail.com>
Precedence: bulk

On Tue Mar 31, 2026 at 1:18 PM -03, Robert Haas wrote:
> On Mon, Mar 30, 2026 at 5:49=E2=80=AFPM Robert Haas <robertmhaas@gmail.co=
m> wrote:
>> I'm currently poking at some ideas for fixing this... more soon.
>
> Here are some patches. I got started poking at this in earnest
> because, on the pg_plan_advice thread, Lukas was saying that instead
> of adopting pg_collect_advice, we should just add an option to send
> advice strings for each executed query to the server log. I went to
> implement that and then felt like it should really be part of
> auto_explain rather than its own thing, which took me down a bit of a
> rathole. But I eventually found my way back out of it, so here's a
> patch set implementing auto_explain.log_extension_options.
>
> ...
>
> So I ended up with this:
>
> ...
>
> Thoughts?

Hi, thanks for the patches. I think that the architecture is much better
now.

For 0001 I don't have any comment, it looks good to me. The 0002 also
looks good, just a typo on "thent" on commit message. Some comments on
0003:

+		else if (opt->type =3D=3D T_Integer)
+			arg =3D (Node *) makeInteger(strtol(opt->value, NULL, 0));

I think that we are safe against overflow because on
auto_explain_split_options() it has intval =3D=3D (int) intval, but I'm
wondering if it's worth documenting this?

-----

extension_options is being added to REGRESS in both Makefile and
meson.build, but the actual test files are not included.

-----

+      an associated value. The module that provides the
+      <literal>EXPLAIN</literal> option, such as
+      <link linkend=3D"pgplanadvice"><literal>pg_plan_advice</literal></li=
nk> or
+      <link linkend=3D"pgoverexplain"><literal>pg_overexplain</literal></l=
ink>,
+      should be loaded before this parameter is set.

Wondering if this is clear enough about the shared_preload_libraries
order (auto_explain should be loaded after extensions that include
explain options) or if we should mention this explicitly.

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