On Fri, Aug 30, 2024 at 09:37:03AM +0200, Anthonin Bonnefoy wrote: > Thanks for the review. I think the parser state is mostly used for the > error callbacks and parser_errposition but I'm not 100% sure. Either > way, you're right and it probably shouldn't be in the patch. I've > modified the patch to restrict the changes to only add the necessary > query jumble and post parse hook calls.
So this adds four calls post_parse_analyze_hook, leading to more data added to pgss for non-toplevel queries: one in createas.c for the CTAS internal query, one in portalcmds.c for the inner query of DECLARE, and two for utilities in EXPLAIN. This is a rather old problem, trying to bring more consistency across the board, and it comes down to this bit with EXPLAIN, that can be seen on HEAD: SET pg_stat_statements.track = 'all'; explain (costs off) select 1; =# select calls, query, toplevel from pg_stat_statements where query ~'explain'; calls | query | toplevel -------+--------------------------------+---------- 1 | explain (costs off) select $1; | f 1 | explain (costs off) select $1 | t (2 rows) FWIW, I've always found this case with EXPLAIN with two entries confusing, so what's the advantage in trying to apply this rule for the rest? We know that EXPLAIN, DECLARE and CTAS run a query attached to their DDL, hence isn't it sufficient to register a single entry for the top-level query, then nothing for the internal one. The documentation tells about inner queries when pg_stat_statements.track = all, like the ones in PL functions, DO blocks, because we have a clear view of the query string, creating a unique one-one mapping between a query string and its ID. This patch maps the same query string to more than one query ID, spreading that. So it seems that there are arguments for not doing what this patch proposes, but also make sure that EXPLAIN logs a single entry, not two currently when using pg_stat_statements.track = all. Side note. It looks like the patch is forgetting about CREATE VIEW and CREATE MATERIALIZED VIEW, creating only a top-level entry when running these utilities. -- Michael
signature.asc
Description: PGP signature