On Tue, Mar 31, 2026 at 6:10 PM Matheus Alcantara <[email protected]> wrote: > 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?
We could add a comment here that the validity checks have already been done at an earlier stage, but I felt like that was overkill. Possibly not? > extension_options is being added to REGRESS in both Makefile and > meson.build, but the actual test files are not included. Well, that sucks. I accidentally erased that file instead of committing it. Here's a new version with mostly the same tests, plus I updated the TAP test with a related test case as well. > + 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. I actually thought that this might not be true until I tested it and found that it sort of is. If you don't set auto_explain.log_extension_options in postgresql.conf, then you can load the modules in either order and it's fine. But if you do set it, then you need to have the EXPLAIN option provider before auto_explain, or else you get something like this: 2026-04-02 14:03:19.282 EDT [4614] WARNING: unrecognized EXPLAIN option "debug" ...because we read the whole postgresql.conf file before applying any of it, and so if auto_explain's _PG_init() runs first, the GUC value is already visible but the EXPLAIN option doesn't exist yet. That's annoying, but I'm not sure it's worth any more of a documentation reference than what I have already. "Before this parameter is set" could be read to encompass "put it earlier in shared_preload_libraries," and if someone does it wrong, it will become obvious quickly enough. If you (or someone else) doesn't agree, feel free to propose better wording -- I just don't want to expand the description so much that it becomes a distraction. -- Robert Haas EDB: http://www.enterprisedb.com
v2-0003-auto_explain-Add-new-GUC-auto_explain.log_extensi.patch
Description: Binary data
v2-0002-Add-a-guc_check_handler-to-the-EXPLAIN-extension-.patch
Description: Binary data
v2-0001-Expose-helper-functions-scan_quoted_identifier-an.patch
Description: Binary data
