From 0af947aad4224b40e0108cc2d05029e21705f3a9 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Mon, 11 May 2026 13:36:26 +0800 Subject: [PATCH v1] Fix pg_stat_statements display of normalized FETCH counts pg_stat_statements can group FETCH variants that differ only by the presence of a count, such as "FETCH c" and "FETCH 2 c". If the entry was first created by a FETCH without a count, the representative query text remained the unnormalized spelling even after a later counted FETCH was seen. Allow FETCH entries created during post-parse analysis to replace their representative query text once a later statement provides constant locations. Track whether the stored query text is already normalized so the replacement is done only when needed, without overloading sticky entry state. Author: Chao Li --- .../pg_stat_statements/expected/cursors.out | 34 +++++- .../pg_stat_statements/pg_stat_statements.c | 104 ++++++++++++++++-- contrib/pg_stat_statements/sql/cursors.sql | 9 ++ 3 files changed, 134 insertions(+), 13 deletions(-) diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out index 6afb48ace92..d8e5c9bbc6a 100644 --- a/contrib/pg_stat_statements/expected/cursors.out +++ b/contrib/pg_stat_statements/expected/cursors.out @@ -190,18 +190,46 @@ SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; 1 | BEGIN 1 | COMMIT 1 | DECLARE pgss_cursor CURSOR FOR SELECT FROM generate_series($1, $2) + 4 | FETCH $1 pgss_cursor 3 | FETCH ABSOLUTE $1 pgss_cursor 1 | FETCH ALL pgss_cursor + 4 | FETCH BACKWARD $1 pgss_cursor 1 | FETCH BACKWARD ALL pgss_cursor - 4 | FETCH BACKWARD pgss_cursor 1 | FETCH FIRST pgss_cursor + 4 | FETCH FORWARD $1 pgss_cursor 1 | FETCH FORWARD ALL pgss_cursor - 4 | FETCH FORWARD pgss_cursor 1 | FETCH LAST pgss_cursor 1 | FETCH NEXT pgss_cursor 1 | FETCH PRIOR pgss_cursor 3 | FETCH RELATIVE $1 pgss_cursor - 4 | FETCH pgss_cursor 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t (16 rows) +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +-- Order of FETCH statements should not matter. +BEGIN; +DECLARE pgss_cursor CURSOR FOR SELECT FROM generate_series(1, 10); +FETCH 1 pgss_cursor; +-- +(1 row) + +FETCH pgss_cursor; +-- +(1 row) + +COMMIT; +SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; + calls | query +-------+-------------------------------------------------------------------- + 1 | BEGIN + 1 | COMMIT + 1 | DECLARE pgss_cursor CURSOR FOR SELECT FROM generate_series($1, $2) + 2 | FETCH $1 pgss_cursor + 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(5 rows) + diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 95a5411a39d..aa336ff02bc 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -85,7 +85,7 @@ PG_MODULE_MAGIC_EXT( #define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat" /* Magic number identifying the stats file format */ -static const uint32 PGSS_FILE_HEADER = 0x20250731; +static const uint32 PGSS_FILE_HEADER = 0x20260511; /* PostgreSQL major version number, changes in which invalidate all entries */ static const uint32 PGSS_PG_MAJOR_VERSION = PG_VERSION_NUM / 100; @@ -237,6 +237,7 @@ typedef struct pgssEntry Size query_offset; /* query text offset in external file */ int query_len; /* # of valid bytes in query string, or -1 */ int encoding; /* query text encoding */ + bool query_normalized; /* query text is a normalized string */ TimestampTz stats_since; /* timestamp of entry allocation */ TimestampTz minmax_stats_since; /* timestamp of last min/max values reset */ slock_t mutex; /* protects the counters only */ @@ -364,12 +365,14 @@ static void pgss_store(const char *query, int64 queryId, const JumbleState *jstate, int parallel_workers_to_launch, int parallel_workers_launched, + bool replace_query_text, PlannedStmtOrigin planOrigin); static void pg_stat_statements_internal(FunctionCallInfo fcinfo, pgssVersion api_version, bool showtext); static pgssEntry *entry_alloc(pgssHashKey *key, Size query_offset, int query_len, - int encoding, bool sticky); + int encoding, bool sticky, + bool query_normalized); static void entry_dealloc(void); static bool qtext_store(const char *query, int query_len, Size *query_offset, int *gc_count); @@ -658,7 +661,8 @@ pgss_shmem_init(void *arg) /* make the hashtable entry (discards old entries if too many) */ entry = entry_alloc(&temp.key, query_offset, temp.query_len, temp.encoding, - false); + false, + temp.query_normalized); /* copy in the actual stats */ entry->counters = temp.counters; @@ -876,6 +880,7 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query, const JumbleState *jst jstate, 0, 0, + query->utilityStmt && IsA(query->utilityStmt, FetchStmt), PLAN_STMT_UNKNOWN); } @@ -958,6 +963,7 @@ pgss_planner(Query *parse, NULL, 0, 0, + false, result->planOrigin); } else @@ -1076,6 +1082,7 @@ pgss_ExecutorEnd(QueryDesc *queryDesc) NULL, queryDesc->estate->es_parallel_workers_to_launch, queryDesc->estate->es_parallel_workers_launched, + false, queryDesc->plannedstmt->planOrigin); } @@ -1210,6 +1217,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, NULL, 0, 0, + false, pstmt->planOrigin); } else @@ -1274,11 +1282,13 @@ pgss_store(const char *query, int64 queryId, const JumbleState *jstate, int parallel_workers_to_launch, int parallel_workers_launched, + bool replace_query_text, PlannedStmtOrigin planOrigin) { pgssHashKey key; pgssEntry *entry; char *norm_query = NULL; + int norm_query_len = -1; int encoding = GetDatabaseEncoding(); Assert(query != NULL); @@ -1316,11 +1326,80 @@ pgss_store(const char *query, int64 queryId, entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_FIND, NULL); + /* + * If this entry was first created from an unnormalized spelling, but this + * call has constant locations, replace the representative query text with + * the normalized form. This can happen for utility statements such as + * FETCH, where "FETCH c" and "FETCH 2 c" have the same jumble but only + * the latter has a source token that can be replaced by "$1". + */ + if (entry && jstate && jstate->clocations_count > 0 && + replace_query_text && !entry->query_normalized) + { + Size query_offset; + int gc_count; + bool stored; + bool do_gc; + + /* Release the lock before normalizing the query */ + LWLockRelease(&pgss->lock.lock); + + norm_query_len = query_len; + norm_query = generate_normalized_query(jstate, query, + query_location, + &norm_query_len); + + /* Reacquire the lock and look up the entry again */ + LWLockAcquire(&pgss->lock.lock, LW_SHARED); + + entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_FIND, NULL); + if (entry && !entry->query_normalized) + { + stored = qtext_store(norm_query, norm_query_len, + &query_offset, &gc_count); + do_gc = need_gc_qtexts(); + + LWLockRelease(&pgss->lock.lock); + LWLockAcquire(&pgss->lock.lock, LW_EXCLUSIVE); + + /* + * A garbage collection may have removed the text we stored while + * the lock was released to promote it. Store it again if so. + */ + if (!stored || pgss->gc_count != gc_count) + stored = qtext_store(norm_query, norm_query_len, + &query_offset, NULL); + + entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_FIND, NULL); + if (entry && stored && !entry->query_normalized) + { + entry->query_offset = query_offset; + entry->query_len = norm_query_len; + entry->encoding = encoding; + entry->query_normalized = true; + } + + /* If needed, perform garbage collection while exclusive lock held */ + if (do_gc) + gc_qtexts(); + + /* + * Restore the lock mode expected by the common path below. The + * entry might have disappeared while switching lock modes, so + * look it up again. + */ + LWLockRelease(&pgss->lock.lock); + LWLockAcquire(&pgss->lock.lock, LW_SHARED); + entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_FIND, NULL); + } + } + /* Create new entry, if not present */ if (!entry) { Size query_offset; int gc_count; + int stored_query_len; bool stored; bool do_gc; @@ -1331,17 +1410,20 @@ pgss_store(const char *query, int64 queryId, * in the interval where we don't hold the lock below. That case is * handled by entry_alloc.) */ - if (jstate) + if (jstate && norm_query == NULL) { LWLockRelease(&pgss->lock.lock); + norm_query_len = query_len; norm_query = generate_normalized_query(jstate, query, query_location, - &query_len); + &norm_query_len); LWLockAcquire(&pgss->lock.lock, LW_SHARED); } + stored_query_len = norm_query ? norm_query_len : query_len; + /* Append new query text to file with only shared lock held */ - stored = qtext_store(norm_query ? norm_query : query, query_len, + stored = qtext_store(norm_query ? norm_query : query, stored_query_len, &query_offset, &gc_count); /* @@ -1363,7 +1445,7 @@ pgss_store(const char *query, int64 queryId, * exclusive lock isn't a performance problem. */ if (!stored || pgss->gc_count != gc_count) - stored = qtext_store(norm_query ? norm_query : query, query_len, + stored = qtext_store(norm_query ? norm_query : query, stored_query_len, &query_offset, NULL); /* If we failed to write to the text file, give up */ @@ -1371,8 +1453,9 @@ pgss_store(const char *query, int64 queryId, goto done; /* OK to create a new hashtable entry */ - entry = entry_alloc(&key, query_offset, query_len, encoding, - jstate != NULL); + entry = entry_alloc(&key, query_offset, stored_query_len, encoding, + jstate != NULL, + norm_query != NULL); /* If needed, perform garbage collection while exclusive lock held */ if (do_gc) @@ -2078,7 +2161,7 @@ pg_stat_statements_info(PG_FUNCTION_ARGS) */ static pgssEntry * entry_alloc(pgssHashKey *key, Size query_offset, int query_len, int encoding, - bool sticky) + bool sticky, bool query_normalized) { pgssEntry *entry; bool found; @@ -2105,6 +2188,7 @@ entry_alloc(pgssHashKey *key, Size query_offset, int query_len, int encoding, entry->query_offset = query_offset; entry->query_len = query_len; entry->encoding = encoding; + entry->query_normalized = query_normalized; entry->stats_since = GetCurrentTimestamp(); entry->minmax_stats_since = entry->stats_since; } diff --git a/contrib/pg_stat_statements/sql/cursors.sql b/contrib/pg_stat_statements/sql/cursors.sql index 78bb4228433..57f28220335 100644 --- a/contrib/pg_stat_statements/sql/cursors.sql +++ b/contrib/pg_stat_statements/sql/cursors.sql @@ -71,3 +71,12 @@ FETCH BACKWARD -1 pgss_cursor; FETCH BACKWARD ALL pgss_cursor; COMMIT; SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + +-- Order of FETCH statements should not matter. +BEGIN; +DECLARE pgss_cursor CURSOR FOR SELECT FROM generate_series(1, 10); +FETCH 1 pgss_cursor; +FETCH pgss_cursor; +COMMIT; +SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; -- 2.50.1 (Apple Git-155)