From 0a2c631a3f2fc4013b5a2960c6a46e0b4b224568 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Mon, 12 Oct 2026 23:09:43 +0000 Subject: [PATCH v2 1/1] 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. Add start_nesting_level to QueryDesc, initialized to -1. pg_stat_statements sets it in ExecutorStart when it tracks a query. ExecutorEnd then uses it in place of the current level and skips the query if it is unset. Other extensions may set it too, as long as they count nesting levels the same way pg_stat_statements does. No backpatch due to lack of widespread reports. Reported-by: Gaurav Singh Author: Sami Imseih 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 | 18 +++++++++++-- .../pg_stat_statements/sql/level_tracking.sql | 11 ++++++++ src/backend/tcop/pquery.c | 1 + src/include/executor/execdesc.h | 14 +++++++++++ 5 files changed, 67 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..87d11f79717 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -356,6 +356,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, @@ -866,6 +867,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 +950,7 @@ pgss_planner(Query *parse, parse->queryId, parse->stmt_location, parse->stmt_len, + nesting_level == 0, PGSS_PLAN, INSTR_TIME_GET_MILLISEC(duration), 0, @@ -1001,6 +1004,9 @@ 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; see execdesc.h */ + queryDesc->start_nesting_level = nesting_level; } if (prev_ExecutorStart) @@ -1059,13 +1065,19 @@ pgss_ExecutorEnd(QueryDesc *queryDesc) { int64 queryId = queryDesc->plannedstmt->queryId; + /* + * Use the nesting level saved at ExecutorStart, 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)) + queryDesc->start_nesting_level >= 0 && + pgss_enabled(queryDesc->start_nesting_level)) { pgss_store(queryDesc->sourceText, queryId, queryDesc->plannedstmt->stmt_location, queryDesc->plannedstmt->stmt_len, + queryDesc->start_nesting_level == 0, PGSS_EXEC, INSTR_TIME_GET_MILLISEC(queryDesc->query_instr->total), queryDesc->estate->es_total_processed, @@ -1202,6 +1214,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 +1280,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 +1324,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/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index fa2ef20f178..8d72c870ec1 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -87,6 +87,7 @@ CreateQueryDesc(PlannedStmt *plannedstmt, qd->queryEnv = queryEnv; qd->instrument_options = instrument_options; /* instrumentation wanted? */ qd->query_instr_options = 0; + qd->start_nesting_level = -1; /* null these fields until set by ExecutorStart */ qd->tupDesc = NULL; diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h index 37c2576e4bc..f07f35d9dc0 100644 --- a/src/include/executor/execdesc.h +++ b/src/include/executor/execdesc.h @@ -45,6 +45,20 @@ typedef struct QueryDesc int query_instr_options; /* OR of InstrumentOption flags for * query_instr */ + /* + * Nesting level at which this query started, or -1 if not set. An + * extension may set this from its ExecutorStart hook and read it back at + * ExecutorEnd, which can run at a different level. For example, a + * cursor's query starts inside DECLARE but may end at COMMIT. + * + * The level counts as pg_stat_statements does: 0 for a statement sent by + * the client, including EXECUTE of a prepared statement; one deeper for + * each query run by another statement or by a function. All extensions + * that set this field must use that definition; one that counts + * differently must leave it alone. + */ + int start_nesting_level; + /* These fields are set by ExecutorStart */ TupleDesc tupDesc; /* descriptor for result tuples */ EState *estate; /* executor's query-wide state */ -- 2.54.0 (Apple Git-157)