From 23bf901fa5306b245e23ace52e5a6777ea08bed6 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 27 Oct 2026 15:17:17 +0900 Subject: [PATCH] pg_stat_statements: Fix toplevel tracking across nesting levels pg_stat_statements decides whether a query is toplevel from the nesting level at the time each hook runs. A QueryDesc can outlive the level it started at. For example, a cursor's query starts at level 1, inside DECLARE, but if the cursor is closed implicitly at COMMIT, ExecutorEnd runs at level 0. The entry created for the query at parse analysis has toplevel = false, so ExecutorEnd misses it and stores a duplicate with the raw query text and toplevel = true. The same happens with track = top when another extension such as auto_explain requests query instrumentation. ExecutorEnd stores the query even though ExecutorStart did not track it. PGSS is upgraded to save the nesting level in the extension state of the QueryDesc when ExecutorStart() tracks a query. ExecutorEnd() then uses it in place of the current level and skips the query if no state was saved. For now, only the nesting level is tracked in the private extension state attached to QueryDesc by PGSS. auto_explain is not touched either. All these pieces are left as future improvements. No backpatch is done, due to the ABI changes required in QueryDesc. This relies on the work done previously in e5d5d1ad1e3a. Reported-by: Gaurav Singh Author: Sami Imseih Author: Michael Paquier Discussion: https://postgr.es/m/CAEcQ1bZrgwKsauC=03otebkLk77Zc_hYNjhTP3OUdpXs++u+BA@mail.gmail.com Discussion: https://postgr.es/m/CAA5RZ0v281uwSudcFrY8BymSRDM4J6PAHfzqg_9R+i7zWbTb2A@mail.gmail.com --- .../expected/level_tracking.out | 25 +++++++++++ .../pg_stat_statements/pg_stat_statements.c | 43 ++++++++++++++++++- .../pg_stat_statements/sql/level_tracking.sql | 11 +++++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/contrib/pg_stat_statements/expected/level_tracking.out b/contrib/pg_stat_statements/expected/level_tracking.out index 832d65e97ca..ccebf8b2960 100644 --- a/contrib/pg_stat_statements/expected/level_tracking.out +++ b/contrib/pg_stat_statements/expected/level_tracking.out @@ -1151,6 +1151,31 @@ SELECT toplevel, calls, query FROM pg_stat_statements t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t (7 rows) +-- DECLARE CURSOR, all-level tracking with implicit close. +SET pg_stat_statements.track = 'all'; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +BEGIN; +DECLARE FOOCUR CURSOR FOR SELECT * FROM stats_track_tab WHERE x = 1; +FETCH FORWARD 1 FROM foocur; + x +--- +(0 rows) + +COMMIT; +SELECT toplevel, calls, query FROM pg_stat_statements + WHERE query LIKE 'DECLARE FOOCUR%' + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+----------------------------------------------------------------------- + t | 1 | DECLARE FOOCUR CURSOR FOR SELECT * FROM stats_track_tab WHERE x = $1 + f | 1 | DECLARE FOOCUR CURSOR FOR SELECT * FROM stats_track_tab WHERE x = $1; +(2 rows) + -- DECLARE CURSOR, top-level tracking. SET pg_stat_statements.track = 'top'; SELECT pg_stat_statements_reset() IS NOT NULL AS t; diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 4562222c9fc..8db4b3d6e24 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -274,6 +274,19 @@ static const ShmemCallbacks pgss_shmem_callbacks = { /* Current nesting depth of planner/ExecutorRun/ProcessUtility calls */ static int nesting_level = 0; +/* + * State saved in a QueryDesc at ExecutorStart, for use at ExecutorEnd. A + * query may end at a different nesting level than the one it started at, + * for example a cursor declared in a transaction and closed at COMMIT. + */ +typedef struct pgssQueryState +{ + int nesting_level; /* nesting level at ExecutorStart */ +} pgssQueryState; + +/* ID of pg_stat_statements for QueryDesc extension state */ +static int pgss_queryDesc_id = -1; + /* Saved hook values */ static post_parse_analyze_hook_type prev_post_parse_analyze_hook = NULL; static planner_hook_type prev_planner_hook = NULL; @@ -356,6 +369,7 @@ static void pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, DestReceiver *dest, QueryCompletion *qc); static void pgss_store(const char *query, int64 queryId, int query_location, int query_len, + bool toplevel, pgssStoreKind kind, double total_time, uint64 rows, const BufferUsage *bufusage, @@ -474,6 +488,11 @@ _PG_init(void) */ RegisterShmemCallbacks(&pgss_shmem_callbacks); + /* + * Get an ID to store state in QueryDesc. + */ + pgss_queryDesc_id = GetQueryDescExtensionId("pg_stat_statements"); + /* * Install hooks. */ @@ -866,6 +885,7 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query, const JumbleState *jst query->queryId, query->stmt_location, query->stmt_len, + nesting_level == 0, PGSS_INVALID, 0, 0, @@ -948,6 +968,7 @@ pgss_planner(Query *parse, parse->queryId, parse->stmt_location, parse->stmt_len, + nesting_level == 0, PGSS_PLAN, INSTR_TIME_GET_MILLISEC(duration), 0, @@ -992,6 +1013,8 @@ pgss_planner(Query *parse, static void pgss_ExecutorStart(QueryDesc *queryDesc, int eflags) { + pgssQueryState *qstate; + /* * If query has queryId zero, don't track it. This prevents double * counting of optimizable statements that are directly contained in @@ -1001,6 +1024,12 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags) { /* Request all summary instrumentation, i.e. timing, buffers and WAL */ queryDesc->query_instr_options |= INSTRUMENT_ALL; + + /* Remember the nesting level for ExecutorEnd */ + qstate = MemoryContextAlloc(GetMemoryChunkContext(queryDesc), + sizeof(pgssQueryState)); + qstate->nesting_level = nesting_level; + SetQueryDescExtensionState(queryDesc, pgss_queryDesc_id, qstate); } if (prev_ExecutorStart) @@ -1058,14 +1087,22 @@ static void pgss_ExecutorEnd(QueryDesc *queryDesc) { int64 queryId = queryDesc->plannedstmt->queryId; + pgssQueryState *qstate = GetQueryDescExtensionState(queryDesc, + pgss_queryDesc_id); + /* + * Use the nesting level saved at ExecutorStart, if available, since the + * query may end at a different level (e.g. a cursor closed at COMMIT). + */ if (queryId != INT64CONST(0) && queryDesc->query_instr && - pgss_enabled(nesting_level)) + qstate != NULL && + pgss_enabled(qstate->nesting_level)) { pgss_store(queryDesc->sourceText, queryId, queryDesc->plannedstmt->stmt_location, queryDesc->plannedstmt->stmt_len, + qstate->nesting_level == 0, PGSS_EXEC, INSTR_TIME_GET_MILLISEC(queryDesc->query_instr->total), queryDesc->estate->es_total_processed, @@ -1202,6 +1239,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, saved_queryId, saved_stmt_location, saved_stmt_len, + nesting_level == 0, PGSS_EXEC, INSTR_TIME_GET_MILLISEC(duration), rows, @@ -1267,6 +1305,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, static void pgss_store(const char *query, int64 queryId, int query_location, int query_len, + bool toplevel, pgssStoreKind kind, double total_time, uint64 rows, const BufferUsage *bufusage, @@ -1310,7 +1349,7 @@ pgss_store(const char *query, int64 queryId, key.userid = GetUserId(); key.dbid = MyDatabaseId; key.queryid = queryId; - key.toplevel = (nesting_level == 0); + key.toplevel = toplevel; /* Lookup the hash table entry with shared lock. */ LWLockAcquire(&pgss->lock.lock, LW_SHARED); diff --git a/contrib/pg_stat_statements/sql/level_tracking.sql b/contrib/pg_stat_statements/sql/level_tracking.sql index 003efb8184b..cc14a9808dd 100644 --- a/contrib/pg_stat_statements/sql/level_tracking.sql +++ b/contrib/pg_stat_statements/sql/level_tracking.sql @@ -262,6 +262,17 @@ COMMIT; SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; +-- DECLARE CURSOR, all-level tracking with implicit close. +SET pg_stat_statements.track = 'all'; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +BEGIN; +DECLARE FOOCUR CURSOR FOR SELECT * FROM stats_track_tab WHERE x = 1; +FETCH FORWARD 1 FROM foocur; +COMMIT; +SELECT toplevel, calls, query FROM pg_stat_statements + WHERE query LIKE 'DECLARE FOOCUR%' + ORDER BY query COLLATE "C", toplevel; + -- DECLARE CURSOR, top-level tracking. SET pg_stat_statements.track = 'top'; SELECT pg_stat_statements_reset() IS NOT NULL AS t; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index a7a8dbdd23f..cfef572df80 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4124,6 +4124,7 @@ pgsql_thing_t pgssEntry pgssGlobalStats pgssHashKey +pgssQueryState pgssSharedState pgssStoreKind pgssVersion