On Tue Mar 31, 2026 at 1:18 PM -03, Robert Haas wrote: > On Mon, Mar 30, 2026 at 5:49 PM Robert Haas <[email protected]> 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 == T_Integer) + arg = (Node *) makeInteger(strtol(opt->value, NULL, 0)); I think that we are safe against overflow because on auto_explain_split_options() it has intval == (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="pgplanadvice"><literal>pg_plan_advice</literal></link> or + <link linkend="pgoverexplain"><literal>pg_overexplain</literal></link>, + 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
