On Fri, Mar 21, 2025 at 2:37 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > I took a look through v8, and have just a couple of trivial nits:
Thank you! > +static void overexplain_debug_handler(ExplainState *, DefElem *, > + ParseState *); > +static void overexplain_range_table_handler(ExplainState *, DefElem *, > + ParseState *); > > I cordially hate this parameter-name-less style of function > declaration, and consider it Stroustrup's single worst idea in C++. > It rests on the assumption that parameter names convey zero > information, which is wrongheaded for any function more complex than, > say, addition. Also, even if you like this style, clang-tidy probably > won't (cf for example 035ce1feb). I don't hate the style but I didn't use it intentionally. Fixed now, I hope. > pgoverexplain.sgml seems to have been formatted to fit in about an > 85-column window. Please don't do that --- you might as well have > made it 99 columns wide or any other random number, it still looks > like heck in 80 columns. Oops. Fixed. Also, I tried to disclaim stability and comprehensibility a little better and fixed a dumb mistake in the page title. > Other than that, there's room to debate exactly what to show. > But as long as we're agreed that we won't hold this module to > high cross-version compatibility standards, that doesn't seem > like a problem. I'm okay with this as a starting point. Great news, thanks again! Here's v9, which also adds 'SET debug_parallel_query = off' to the pg_overexplain tests, per CI, because the test results are not (and cannot realistically be made) stable under under that option. -- Robert Haas EDB: http://www.enterprisedb.com
v9-0001-pg_overexplain-Additional-EXPLAIN-options-for-deb.patch
Description: Binary data
