From a5fc470c8a3f3a3b0be073db1450318cc1b9f07b Mon Sep 17 00:00:00 2001
From: Lukas Fittl <lukas@fittl.com>
Date: Wed, 28 Oct 2026 08:02:05 -0700
Subject: [PATCH 1/3] Introduce UtilityStmtDesc to bundle ProcessUtility()
 arguments

Utility statements share many things with regular queries, but thus far
there was no utility-statement equivalent of QueryDesc to hold common
fields that matter for execution. Instead this information was passed
down via an ever-growing list of arguments to ProcessUtility /
ProcessUtility_hook.

To enable a future change that will introduce another field, and to
reduce future breakage of the hook API, introduce a dedicated utility
statement descriptor (UtilityStmtDesc), and make it the single argument
passed to ProcessUtility.

Author: Lukas Fittl <lukas@fittl.com>
Reviewed-by:
Discussion:
---
 .../pg_stat_statements/pg_stat_statements.c   | 31 +++-----
 contrib/sepgsql/hooks.c                       | 19 +----
 src/backend/commands/extension.c              | 15 ++--
 src/backend/commands/foreigncmds.c            |  9 ++-
 src/backend/commands/schemacmds.c             | 14 ++--
 src/backend/executor/functions.c              | 19 +++--
 src/backend/executor/spi.c                    | 20 +++--
 src/backend/tcop/pquery.c                     | 20 +++--
 src/backend/tcop/utility.c                    | 73 ++++++++-----------
 src/include/tcop/utility.h                    | 41 ++++++-----
 .../modules/test_oat_hooks/test_oat_hooks.c   | 26 ++-----
 src/tools/pgindent/typedefs.list              |  1 +
 12 files changed, 128 insertions(+), 160 deletions(-)

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 8db4b3d6e24b9..ca98372ea8c4c 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -362,11 +362,7 @@ static void pgss_ExecutorRun(QueryDesc *queryDesc,
 							 uint64 count);
 static void pgss_ExecutorFinish(QueryDesc *queryDesc);
 static void pgss_ExecutorEnd(QueryDesc *queryDesc);
-static void pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
-								bool readOnlyTree,
-								ProcessUtilityContext context, ParamListInfo params,
-								QueryEnvironment *queryEnv,
-								DestReceiver *dest, QueryCompletion *qc);
+static void pgss_ProcessUtility(UtilityStmtDesc *desc);
 static void pgss_store(const char *query, int64 queryId,
 					   int query_location, int query_len,
 					   bool toplevel,
@@ -1125,12 +1121,11 @@ pgss_ExecutorEnd(QueryDesc *queryDesc)
  * ProcessUtility hook
  */
 static void
-pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
-					bool readOnlyTree,
-					ProcessUtilityContext context,
-					ParamListInfo params, QueryEnvironment *queryEnv,
-					DestReceiver *dest, QueryCompletion *qc)
+pgss_ProcessUtility(UtilityStmtDesc *desc)
 {
+	PlannedStmt *pstmt = desc->pstmt;
+	const char *queryString = desc->queryString;
+	QueryCompletion *qc = desc->qc;
 	Node	   *parsetree = pstmt->utilityStmt;
 	int64		saved_queryId = pstmt->queryId;
 	int			saved_stmt_location = pstmt->stmt_location;
@@ -1188,13 +1183,9 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
 		PG_TRY();
 		{
 			if (prev_ProcessUtility)
-				prev_ProcessUtility(pstmt, queryString, readOnlyTree,
-									context, params, queryEnv,
-									dest, qc);
+				prev_ProcessUtility(desc);
 			else
-				standard_ProcessUtility(pstmt, queryString, readOnlyTree,
-										context, params, queryEnv,
-										dest, qc);
+				standard_ProcessUtility(desc);
 		}
 		PG_FINALLY();
 		{
@@ -1274,13 +1265,9 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
 		PG_TRY();
 		{
 			if (prev_ProcessUtility)
-				prev_ProcessUtility(pstmt, queryString, readOnlyTree,
-									context, params, queryEnv,
-									dest, qc);
+				prev_ProcessUtility(desc);
 			else
-				standard_ProcessUtility(pstmt, queryString, readOnlyTree,
-										context, params, queryEnv,
-										dest, qc);
+				standard_ProcessUtility(desc);
 		}
 		PG_FINALLY();
 		{
diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c
index 9bbc9e069c077..80924d1f15991 100644
--- a/contrib/sepgsql/hooks.c
+++ b/contrib/sepgsql/hooks.c
@@ -313,16 +313,9 @@ sepgsql_exec_check_perms(List *rangeTbls, List *rteperminfos, bool abort)
  * break whole of the things if nefarious user would use.
  */
 static void
-sepgsql_utility_command(PlannedStmt *pstmt,
-						const char *queryString,
-						bool readOnlyTree,
-						ProcessUtilityContext context,
-						ParamListInfo params,
-						QueryEnvironment *queryEnv,
-						DestReceiver *dest,
-						QueryCompletion *qc)
+sepgsql_utility_command(UtilityStmtDesc *desc)
 {
-	Node	   *parsetree = pstmt->utilityStmt;
+	Node	   *parsetree = desc->pstmt->utilityStmt;
 	sepgsql_context_info_t saved_context_info = sepgsql_context_info;
 	ListCell   *cell;
 
@@ -381,13 +374,9 @@ sepgsql_utility_command(PlannedStmt *pstmt,
 		}
 
 		if (next_ProcessUtility_hook)
-			(*next_ProcessUtility_hook) (pstmt, queryString, readOnlyTree,
-										 context, params, queryEnv,
-										 dest, qc);
+			(*next_ProcessUtility_hook) (desc);
 		else
-			standard_ProcessUtility(pstmt, queryString, readOnlyTree,
-									context, params, queryEnv,
-									dest, qc);
+			standard_ProcessUtility(desc);
 	}
 	PG_FINALLY();
 	{
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 5855668c5cade..9a8caa9089d3e 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1187,19 +1187,18 @@ execute_sql_string(const char *sql, const char *filename)
 			}
 			else
 			{
+				UtilityStmtDesc utilityDesc = {0};
+
 				if (IsA(stmt->utilityStmt, TransactionStmt))
 					ereport(ERROR,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 							 errmsg("transaction control statements are not allowed within an extension script")));
 
-				ProcessUtility(stmt,
-							   sql,
-							   false,
-							   PROCESS_UTILITY_QUERY,
-							   NULL,
-							   NULL,
-							   dest,
-							   NULL);
+				utilityDesc.pstmt = stmt;
+				utilityDesc.queryString = sql;
+				utilityDesc.context = PROCESS_UTILITY_QUERY;
+				utilityDesc.dest = dest;
+				ProcessUtility(&utilityDesc);
 			}
 
 			PopActiveSnapshot();
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index c4852be2eb29c..92a9fbc2316e0 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1656,6 +1656,7 @@ ImportForeignSchema(ImportForeignSchemaStmt *stmt)
 			RawStmt    *rs = lfirst_node(RawStmt, lc2);
 			CreateForeignTableStmt *cstmt = (CreateForeignTableStmt *) rs->stmt;
 			PlannedStmt *pstmt;
+			UtilityStmtDesc utilityDesc = {0};
 
 			/*
 			 * Because we only allow CreateForeignTableStmt, we can skip parse
@@ -1686,9 +1687,11 @@ ImportForeignSchema(ImportForeignSchemaStmt *stmt)
 			pstmt->planOrigin = PLAN_STMT_INTERNAL;
 
 			/* Execute statement */
-			ProcessUtility(pstmt, cmd, false,
-						   PROCESS_UTILITY_SUBCOMMAND, NULL, NULL,
-						   None_Receiver, NULL);
+			utilityDesc.pstmt = pstmt;
+			utilityDesc.queryString = cmd;
+			utilityDesc.context = PROCESS_UTILITY_SUBCOMMAND;
+			utilityDesc.dest = None_Receiver;
+			ProcessUtility(&utilityDesc);
 
 			/* Be sure to advance the command counter between subcommands */
 			CommandCounterIncrement();
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index b016983f9049a..7e56501b173e3 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -208,6 +208,7 @@ CreateSchemaCommand(ParseState *pstate, CreateSchemaStmt *stmt,
 	{
 		Node	   *node = (Node *) lfirst(parsetree_item);
 		PlannedStmt *wrapper;
+		UtilityStmtDesc utilityDesc = {0};
 
 		/* need to make a wrapper PlannedStmt */
 		wrapper = makeNode(PlannedStmt);
@@ -219,14 +220,11 @@ CreateSchemaCommand(ParseState *pstate, CreateSchemaStmt *stmt,
 		wrapper->planOrigin = PLAN_STMT_INTERNAL;
 
 		/* do this step */
-		ProcessUtility(wrapper,
-					   pstate->p_sourcetext,
-					   false,
-					   PROCESS_UTILITY_SUBCOMMAND,
-					   NULL,
-					   NULL,
-					   None_Receiver,
-					   NULL);
+		utilityDesc.pstmt = wrapper;
+		utilityDesc.queryString = pstate->p_sourcetext;
+		utilityDesc.context = PROCESS_UTILITY_SUBCOMMAND;
+		utilityDesc.dest = None_Receiver;
+		ProcessUtility(&utilityDesc);
 
 		/* make sure later steps can see the object created here */
 		CommandCounterIncrement();
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 8fec79a2f7672..9a0867750d4ce 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -1408,14 +1408,17 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache)
 
 	if (es->qd->operation == CMD_UTILITY)
 	{
-		ProcessUtility(es->qd->plannedstmt,
-					   fcache->func->src,
-					   true,	/* protect function cache's parsetree */
-					   PROCESS_UTILITY_QUERY,
-					   es->qd->params,
-					   es->qd->queryEnv,
-					   es->qd->dest,
-					   NULL);
+		UtilityStmtDesc utilityDesc = {0};
+
+		utilityDesc.pstmt = es->qd->plannedstmt;
+		utilityDesc.queryString = fcache->func->src;
+		utilityDesc.readOnlyTree = true;	/* protect function cache's
+											 * parsetree */
+		utilityDesc.context = PROCESS_UTILITY_QUERY;
+		utilityDesc.params = es->qd->params;
+		utilityDesc.queryEnv = es->qd->queryEnv;
+		utilityDesc.dest = es->qd->dest;
+		ProcessUtility(&utilityDesc);
 		result = true;			/* never stops early */
 	}
 	else
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 196b998dd7f7b..067f0de85d6c4 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2702,6 +2702,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
 			{
 				ProcessUtilityContext context;
 				QueryCompletion qc;
+				UtilityStmtDesc utilityDesc = {0};
 
 				/*
 				 * If we're not allowing nonatomic operations, tell
@@ -2713,14 +2714,17 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
 					context = PROCESS_UTILITY_QUERY;
 
 				InitializeQueryCompletion(&qc);
-				ProcessUtility(stmt,
-							   plansource->query_string,
-							   true,	/* protect plancache's node tree */
-							   context,
-							   options->params,
-							   _SPI_current->queryEnv,
-							   dest,
-							   &qc);
+
+				utilityDesc.pstmt = stmt;
+				utilityDesc.queryString = plansource->query_string;
+				utilityDesc.readOnlyTree = true;	/* protect plancache's
+													 * node tree */
+				utilityDesc.context = context;
+				utilityDesc.params = options->params;
+				utilityDesc.queryEnv = _SPI_current->queryEnv;
+				utilityDesc.dest = dest;
+				utilityDesc.qc = &qc;
+				ProcessUtility(&utilityDesc);
 
 				/* Update "processed" if stmt returned tuples */
 				if (_SPI_current->tuptable)
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 98205aafb051d..6e956abedd8bd 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1226,6 +1226,8 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
 				 bool isTopLevel, bool setHoldSnapshot,
 				 DestReceiver *dest, QueryCompletion *qc)
 {
+	UtilityStmtDesc utilityDesc = {0};
+
 	/*
 	 * Set snapshot if utility stmt needs one.
 	 */
@@ -1253,14 +1255,16 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
 	else
 		portal->portalSnapshot = NULL;
 
-	ProcessUtility(pstmt,
-				   portal->sourceText,
-				   (portal->cplan != NULL), /* protect tree if in plancache */
-				   isTopLevel ? PROCESS_UTILITY_TOPLEVEL : PROCESS_UTILITY_QUERY,
-				   portal->portalParams,
-				   portal->queryEnv,
-				   dest,
-				   qc);
+	utilityDesc.pstmt = pstmt;
+	utilityDesc.queryString = portal->sourceText;
+	utilityDesc.readOnlyTree = (portal->cplan != NULL); /* protect tree if in
+														 * plancache */
+	utilityDesc.context = isTopLevel ? PROCESS_UTILITY_TOPLEVEL : PROCESS_UTILITY_QUERY;
+	utilityDesc.params = portal->portalParams;
+	utilityDesc.queryEnv = portal->queryEnv;
+	utilityDesc.dest = dest;
+	utilityDesc.qc = qc;
+	ProcessUtility(&utilityDesc);
 
 	/* Some utility statements may change context on us */
 	MemoryContextSwitchTo(portal->portalContext);
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 1512d2df196e6..0b4009ff7ccac 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -498,19 +498,12 @@ CheckRestrictedOperation(const char *cmdname)
  * and stmt_len that were given for the whole statement.
  */
 void
-ProcessUtility(PlannedStmt *pstmt,
-			   const char *queryString,
-			   bool readOnlyTree,
-			   ProcessUtilityContext context,
-			   ParamListInfo params,
-			   QueryEnvironment *queryEnv,
-			   DestReceiver *dest,
-			   QueryCompletion *qc)
+ProcessUtility(UtilityStmtDesc *desc)
 {
-	Assert(IsA(pstmt, PlannedStmt));
-	Assert(pstmt->commandType == CMD_UTILITY);
-	Assert(queryString != NULL);	/* required as of 8.4 */
-	Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN);
+	Assert(IsA(desc->pstmt, PlannedStmt));
+	Assert(desc->pstmt->commandType == CMD_UTILITY);
+	Assert(desc->queryString != NULL);	/* required as of 8.4 */
+	Assert(desc->qc == NULL || desc->qc->commandTag == CMDTAG_UNKNOWN);
 
 	/*
 	 * We provide a function hook variable that lets loadable plugins get
@@ -518,13 +511,9 @@ ProcessUtility(PlannedStmt *pstmt,
 	 * call standard_ProcessUtility().
 	 */
 	if (ProcessUtility_hook)
-		(*ProcessUtility_hook) (pstmt, queryString, readOnlyTree,
-								context, params, queryEnv,
-								dest, qc);
+		(*ProcessUtility_hook) (desc);
 	else
-		standard_ProcessUtility(pstmt, queryString, readOnlyTree,
-								context, params, queryEnv,
-								dest, qc);
+		standard_ProcessUtility(desc);
 }
 
 /*
@@ -542,15 +531,16 @@ ProcessUtility(PlannedStmt *pstmt,
  * event-trigger.sgml is kept up to date.
  */
 void
-standard_ProcessUtility(PlannedStmt *pstmt,
-						const char *queryString,
-						bool readOnlyTree,
-						ProcessUtilityContext context,
-						ParamListInfo params,
-						QueryEnvironment *queryEnv,
-						DestReceiver *dest,
-						QueryCompletion *qc)
+standard_ProcessUtility(UtilityStmtDesc *desc)
 {
+	PlannedStmt *pstmt = desc->pstmt;
+	const char *queryString = desc->queryString;
+	bool		readOnlyTree = desc->readOnlyTree;
+	ProcessUtilityContext context = desc->context;
+	ParamListInfo params = desc->params;
+	QueryEnvironment *queryEnv = desc->queryEnv;
+	DestReceiver *dest = desc->dest;
+	QueryCompletion *qc = desc->qc;
 	Node	   *parsetree;
 	bool		isTopLevel = (context == PROCESS_UTILITY_TOPLEVEL);
 	bool		isAtomicContext = (!(context == PROCESS_UTILITY_TOPLEVEL || context == PROCESS_UTILITY_QUERY_NONATOMIC) || IsTransactionBlock());
@@ -1236,6 +1226,7 @@ ProcessUtilitySlow(ParseState *pstate,
 							 * event trigger context.
 							 */
 							PlannedStmt *wrapper;
+							UtilityStmtDesc subDesc = {0};
 
 							wrapper = makeNode(PlannedStmt);
 							wrapper->commandType = CMD_UTILITY;
@@ -1245,14 +1236,12 @@ ProcessUtilitySlow(ParseState *pstate,
 							wrapper->stmt_len = pstmt->stmt_len;
 							wrapper->planOrigin = PLAN_STMT_INTERNAL;
 
-							ProcessUtility(wrapper,
-										   queryString,
-										   false,
-										   PROCESS_UTILITY_SUBCOMMAND,
-										   params,
-										   NULL,
-										   None_Receiver,
-										   NULL);
+							subDesc.pstmt = wrapper;
+							subDesc.queryString = queryString;
+							subDesc.context = PROCESS_UTILITY_SUBCOMMAND;
+							subDesc.params = params;
+							subDesc.dest = None_Receiver;
+							ProcessUtility(&subDesc);
 						}
 
 						/* Need CCI between commands */
@@ -1976,6 +1965,7 @@ void
 ProcessUtilityForAlterTable(Node *stmt, AlterTableUtilityContext *context)
 {
 	PlannedStmt *wrapper;
+	UtilityStmtDesc subDesc = {0};
 
 	/*
 	 * For event triggers, we must "close" the current complex-command set,
@@ -1993,14 +1983,13 @@ ProcessUtilityForAlterTable(Node *stmt, AlterTableUtilityContext *context)
 	wrapper->stmt_len = context->pstmt->stmt_len;
 	wrapper->planOrigin = PLAN_STMT_INTERNAL;
 
-	ProcessUtility(wrapper,
-				   context->queryString,
-				   false,
-				   PROCESS_UTILITY_SUBCOMMAND,
-				   context->params,
-				   context->queryEnv,
-				   None_Receiver,
-				   NULL);
+	subDesc.pstmt = wrapper;
+	subDesc.queryString = context->queryString;
+	subDesc.context = PROCESS_UTILITY_SUBCOMMAND;
+	subDesc.params = context->params;
+	subDesc.queryEnv = context->queryEnv;
+	subDesc.dest = None_Receiver;
+	ProcessUtility(&subDesc);
 
 	EventTriggerAlterTableStart(context->pstmt->utilityStmt);
 	EventTriggerAlterTableRelid(context->relid);
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index abbdb401bf6d2..ae61ff90412a1 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -67,26 +67,33 @@ typedef struct AlterTableUtilityContext
 	 COMMAND_OK_IN_PARALLEL_MODE)
 #define COMMAND_IS_NOT_READ_ONLY	0
 
+/*
+ * UtilityStmtDesc bundles everything needed to execute one utility statement,
+ * the way QueryDesc does for optimizable statements.  Collecting these into a
+ * single struct keeps ProcessUtility() and its hook from having to grow a new
+ * parameter -- and break every ProcessUtility_hook -- each time a new piece of
+ * per-statement state is needed.
+ */
+typedef struct UtilityStmtDesc
+{
+	PlannedStmt *pstmt;			/* the utility statement to execute */
+	const char *queryString;	/* source text of the command */
+	bool		readOnlyTree;	/* if true, pstmt's node tree must not be
+								 * modified */
+	ProcessUtilityContext context;	/* identifies source of the command */
+	ParamListInfo params;		/* parameters to use during execution */
+	QueryEnvironment *queryEnv; /* environment for parse analysis, etc. */
+	DestReceiver *dest;			/* where to send results, if any */
+	QueryCompletion *qc;		/* where to store command completion status,
+								 * or NULL */
+} UtilityStmtDesc;
+
 /* Hook for plugins to get control in ProcessUtility() */
-typedef void (*ProcessUtility_hook_type) (PlannedStmt *pstmt,
-										  const char *queryString,
-										  bool readOnlyTree,
-										  ProcessUtilityContext context,
-										  ParamListInfo params,
-										  QueryEnvironment *queryEnv,
-										  DestReceiver *dest, QueryCompletion *qc);
+typedef void (*ProcessUtility_hook_type) (UtilityStmtDesc *desc);
 extern PGDLLIMPORT ProcessUtility_hook_type ProcessUtility_hook;
 
-extern void ProcessUtility(PlannedStmt *pstmt, const char *queryString,
-						   bool readOnlyTree,
-						   ProcessUtilityContext context, ParamListInfo params,
-						   QueryEnvironment *queryEnv,
-						   DestReceiver *dest, QueryCompletion *qc);
-extern void standard_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
-									bool readOnlyTree,
-									ProcessUtilityContext context, ParamListInfo params,
-									QueryEnvironment *queryEnv,
-									DestReceiver *dest, QueryCompletion *qc);
+extern void ProcessUtility(UtilityStmtDesc *desc);
+extern void standard_ProcessUtility(UtilityStmtDesc *desc);
 
 extern void ProcessUtilityForAlterTable(Node *stmt,
 										AlterTableUtilityContext *context);
diff --git a/src/test/modules/test_oat_hooks/test_oat_hooks.c b/src/test/modules/test_oat_hooks/test_oat_hooks.c
index 57fd4b5cc9054..e6eb689c75350 100644
--- a/src/test/modules/test_oat_hooks/test_oat_hooks.c
+++ b/src/test/modules/test_oat_hooks/test_oat_hooks.c
@@ -55,12 +55,7 @@ static void REGRESS_object_access_hook_str(ObjectAccessType access,
 static void REGRESS_object_access_hook(ObjectAccessType access, Oid classId,
 									   Oid objectId, int subId, void *arg);
 static bool REGRESS_exec_check_perms(List *rangeTabls, List *rteperminfos, bool do_abort);
-static void REGRESS_utility_command(PlannedStmt *pstmt,
-									const char *queryString, bool readOnlyTree,
-									ProcessUtilityContext context,
-									ParamListInfo params,
-									QueryEnvironment *queryEnv,
-									DestReceiver *dest, QueryCompletion *qc);
+static void REGRESS_utility_command(UtilityStmtDesc *desc);
 
 /* Helper functions */
 static char *accesstype_to_string(ObjectAccessType access, int subId);
@@ -376,16 +371,9 @@ REGRESS_exec_check_perms(List *rangeTabls, List *rteperminfos, bool do_abort)
 }
 
 static void
-REGRESS_utility_command(PlannedStmt *pstmt,
-						const char *queryString,
-						bool readOnlyTree,
-						ProcessUtilityContext context,
-						ParamListInfo params,
-						QueryEnvironment *queryEnv,
-						DestReceiver *dest,
-						QueryCompletion *qc)
+REGRESS_utility_command(UtilityStmtDesc *desc)
 {
-	Node	   *parsetree = pstmt->utilityStmt;
+	Node	   *parsetree = desc->pstmt->utilityStmt;
 	const char *action = GetCommandTagName(CreateCommandTag(parsetree));
 
 	audit_attempt("process utility",
@@ -400,13 +388,9 @@ REGRESS_utility_command(PlannedStmt *pstmt,
 
 	/* Forward to next hook in the chain */
 	if (next_ProcessUtility_hook)
-		(*next_ProcessUtility_hook) (pstmt, queryString, readOnlyTree,
-									 context, params, queryEnv,
-									 dest, qc);
+		(*next_ProcessUtility_hook) (desc);
 	else
-		standard_ProcessUtility(pstmt, queryString, readOnlyTree,
-								context, params, queryEnv,
-								dest, qc);
+		standard_ProcessUtility(desc);
 
 	/* We're done */
 	audit_success("process utility",
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index cfef572df80f0..d93fba3ca7150 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3330,6 +3330,7 @@ UserAuth
 UserContext
 UserMapping
 UserOpts
+UtilityStmtDesc
 VMCorruptionType
 VacAttrStats
 VacAttrStatsP

From de7bb0594130563a83ef1d15ca1f41a45a3f10ff Mon Sep 17 00:00:00 2001
From: Lukas Fittl <lukas@fittl.com>
Date: Wed, 28 Oct 2026 09:25:07 -0700
Subject: [PATCH 2/3] Isolate parallel leader global usage from worker
 buffer/WAL usage

Historically, parallel workers updated the global instrumentation
(pgWalUsage/pgBufferUsage) in two places: the worker's own copy and through
reporting via shared memory, and explicit accumulation, the leader's copy.

This double reporting in the globals of multiple processes was acceptable
when the globals were only used for statement-level reporting (and the
worker globals effectively ignored), but that stopped being true after
commit 01469241b2f started using pgWalUsage for reporting to the cumulative
statistics system. This caused a bug with parallel workers double
reporting their WAL in pg_stat_wal, once via the leader, and once via the
worker, leading to incorrect data for utility statements like parallel
vacuum, and queries that update hint bits, e.g. in a parallel seq scan.

To fix, never accumulate worker activity into the leader's globals.
Instead, introduce a consistent statement-level instrumentation we can
accumulate into. For queries this exists with QueryDesc->query_instr,
but for utility statements this needs to be invented.

Introduce a new instrumentation ("instr") field on UtilityStmtDesc, and
make it accessible to utility statement logic by introducing a new global
ActiveUtilityStmtDesc that points to the currently active utility
statement. Anyone interested in instrumentation must request it through
instr_options, matching the existing logic for queries. Adjust
pg_stat_statements to capture its statistics for utility statements from
this instrumentation, instead of reading the globals.

Author: Lukas Fittl <lukas@fittl.com>
Reviewed-by:
Discussion:
---
 .../pg_stat_statements/pg_stat_statements.c   | 62 ++++++++-----------
 src/backend/access/brin/brin.c                |  7 ++-
 src/backend/access/gin/gininsert.c            |  7 ++-
 src/backend/access/nbtree/nbtsort.c           |  7 ++-
 src/backend/commands/vacuumparallel.c         |  7 ++-
 src/backend/executor/execMain.c               |  3 +
 src/backend/executor/execParallel.c           | 14 +++--
 src/backend/executor/instrument.c             | 12 ++--
 src/backend/tcop/utility.c                    | 47 ++++++++++++--
 src/include/executor/instrument.h             |  3 +-
 src/include/nodes/execnodes.h                 |  1 +
 src/include/tcop/utility.h                    | 15 +++++
 12 files changed, 129 insertions(+), 56 deletions(-)

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index ca98372ea8c4c..52754142809da 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1167,17 +1167,14 @@ pgss_ProcessUtility(UtilityStmtDesc *desc)
 		!IsA(parsetree, ExecuteStmt) &&
 		!IsA(parsetree, PrepareStmt))
 	{
-		instr_time	start;
-		instr_time	duration;
 		uint64		rows;
-		BufferUsage bufusage_start,
-					bufusage;
-		WalUsage	walusage_start,
-					walusage;
 
-		bufusage_start = pgBufferUsage;
-		walusage_start = pgWalUsage;
-		INSTR_TIME_SET_CURRENT(start);
+		/*
+		 * Request statement-level instrumentation (timing, buffers and WAL)
+		 * so that we can read the command's resource usage from desc->instr
+		 * below, the same way we read query_instr for optimizable statements.
+		 */
+		desc->instr_options |= INSTRUMENT_ALL;
 
 		nesting_level++;
 		PG_TRY();
@@ -1204,9 +1201,6 @@ pgss_ProcessUtility(UtilityStmtDesc *desc)
 		 */
 		pstmt = NULL;
 
-		INSTR_TIME_SET_CURRENT(duration);
-		INSTR_TIME_SUBTRACT(duration, start);
-
 		/*
 		 * Track the total number of rows retrieved or affected by the utility
 		 * statements of COPY, FETCH, CREATE TABLE AS, CREATE MATERIALIZED
@@ -1218,29 +1212,27 @@ pgss_ProcessUtility(UtilityStmtDesc *desc)
 					   qc->commandTag == CMDTAG_REFRESH_MATERIALIZED_VIEW)) ?
 			qc->nprocessed : 0;
 
-		/* calc differences of buffer counters. */
-		memset(&bufusage, 0, sizeof(BufferUsage));
-		BufferUsageAccumDiff(&bufusage, &pgBufferUsage, &bufusage_start);
-
-		/* calc differences of WAL counters. */
-		memset(&walusage, 0, sizeof(WalUsage));
-		WalUsageAccumDiff(&walusage, &pgWalUsage, &walusage_start);
-
-		pgss_store(queryString,
-				   saved_queryId,
-				   saved_stmt_location,
-				   saved_stmt_len,
-				   nesting_level == 0,
-				   PGSS_EXEC,
-				   INSTR_TIME_GET_MILLISEC(duration),
-				   rows,
-				   &bufusage,
-				   &walusage,
-				   NULL,
-				   NULL,
-				   0,
-				   0,
-				   saved_planOrigin);
+		/*
+		 * standard_ProcessUtility allocates desc->instr when instrumentation
+		 * was requested; it stays NULL only if another hook took over and
+		 * skipped it, in which case we have nothing to record.
+		 */
+		if (desc->instr)
+			pgss_store(queryString,
+					   saved_queryId,
+					   saved_stmt_location,
+					   saved_stmt_len,
+					   nesting_level == 0,
+					   PGSS_EXEC,
+					   INSTR_TIME_GET_MILLISEC(desc->instr->total),
+					   rows,
+					   &desc->instr->bufusage,
+					   &desc->instr->walusage,
+					   NULL,
+					   NULL,
+					   0,
+					   0,
+					   saved_planOrigin);
 	}
 	else
 	{
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 5059da8dce7c5..fadd9c887b5dc 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -37,6 +37,7 @@
 #include "storage/freespace.h"
 #include "storage/proc.h"
 #include "tcop/tcopprot.h"
+#include "tcop/utility.h"
 #include "utils/acl.h"
 #include "utils/datum.h"
 #include "utils/fmgrprotos.h"
@@ -2572,8 +2573,10 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state)
 	 * Next, accumulate WAL usage.  (This must wait for the workers to finish,
 	 * or we might get incomplete data.)
 	 */
-	for (i = 0; i < brinleader->pcxt->nworkers_launched; i++)
-		InstrAccumParallelQuery(&brinleader->bufferusage[i], &brinleader->walusage[i]);
+	if (ActiveUtilityStmtDesc && ActiveUtilityStmtDesc->instr)
+		for (i = 0; i < brinleader->pcxt->nworkers_launched; i++)
+			InstrAccumParallelQuery(ActiveUtilityStmtDesc->instr,
+									&brinleader->bufferusage[i], &brinleader->walusage[i]);
 
 	/* Free last reference to MVCC snapshot, if one was used */
 	if (IsMVCCSnapshot(brinleader->snapshot))
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 564b1509f1bf2..46323f36d736d 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -32,6 +32,7 @@
 #include "storage/proc.h"
 #include "storage/predicate.h"
 #include "tcop/tcopprot.h"
+#include "tcop/utility.h"
 #include "utils/datum.h"
 #include "utils/memutils.h"
 #include "utils/builtins.h"
@@ -1124,8 +1125,10 @@ _gin_end_parallel(GinLeader *ginleader, GinBuildState *state)
 	 * Next, accumulate WAL usage.  (This must wait for the workers to finish,
 	 * or we might get incomplete data.)
 	 */
-	for (i = 0; i < ginleader->pcxt->nworkers_launched; i++)
-		InstrAccumParallelQuery(&ginleader->bufferusage[i], &ginleader->walusage[i]);
+	if (ActiveUtilityStmtDesc && ActiveUtilityStmtDesc->instr)
+		for (i = 0; i < ginleader->pcxt->nworkers_launched; i++)
+			InstrAccumParallelQuery(ActiveUtilityStmtDesc->instr,
+									&ginleader->bufferusage[i], &ginleader->walusage[i]);
 
 	/* Free last reference to MVCC snapshot, if one was used */
 	if (IsMVCCSnapshot(ginleader->snapshot))
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 756dfa3dcf47e..26a68e0e75104 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -55,6 +55,7 @@
 #include "storage/condition_variable.h"
 #include "storage/proc.h"
 #include "tcop/tcopprot.h"
+#include "tcop/utility.h"
 #include "utils/rel.h"
 #include "utils/sortsupport.h"
 #include "utils/tuplesort.h"
@@ -1619,8 +1620,10 @@ _bt_end_parallel(BTLeader *btleader)
 	 * Next, accumulate WAL usage.  (This must wait for the workers to finish,
 	 * or we might get incomplete data.)
 	 */
-	for (i = 0; i < btleader->pcxt->nworkers_launched; i++)
-		InstrAccumParallelQuery(&btleader->bufferusage[i], &btleader->walusage[i]);
+	if (ActiveUtilityStmtDesc && ActiveUtilityStmtDesc->instr)
+		for (i = 0; i < btleader->pcxt->nworkers_launched; i++)
+			InstrAccumParallelQuery(ActiveUtilityStmtDesc->instr,
+									&btleader->bufferusage[i], &btleader->walusage[i]);
 
 	/* Free last reference to MVCC snapshot, if one was used */
 	if (IsMVCCSnapshot(btleader->snapshot))
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index c29f62c5a9910..6fb6f726c233a 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -47,6 +47,7 @@
 #include "storage/bufmgr.h"
 #include "storage/proc.h"
 #include "tcop/tcopprot.h"
+#include "tcop/utility.h"
 #include "utils/injection_point.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
@@ -989,8 +990,10 @@ parallel_vacuum_process_all_indexes(ParallelVacuumState *pvs, int num_index_scan
 		/* Wait for all vacuum workers to finish */
 		WaitForParallelWorkersToFinish(pvs->pcxt);
 
-		for (int i = 0; i < pvs->pcxt->nworkers_launched; i++)
-			InstrAccumParallelQuery(&pvs->buffer_usage[i], &pvs->wal_usage[i]);
+		if (ActiveUtilityStmtDesc && ActiveUtilityStmtDesc->instr)
+			for (int i = 0; i < pvs->pcxt->nworkers_launched; i++)
+				InstrAccumParallelQuery(ActiveUtilityStmtDesc->instr,
+										&pvs->buffer_usage[i], &pvs->wal_usage[i]);
 	}
 
 	/*
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d7a8f36ca1de1..43c6b10f2b0f6 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -259,6 +259,9 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
 	if (queryDesc->query_instr_options)
 		queryDesc->query_instr = InstrAlloc(queryDesc->query_instr_options);
 
+	/* Let parallel leader code reach the query-level instrumentation */
+	estate->es_query_instr = queryDesc->query_instr;
+
 	/*
 	 * Set up an AFTER-trigger statement context, unless told not to, or
 	 * unless it's EXPLAIN-only mode (when ExecutorFinish won't be called).
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 316dd3f67c6e3..9427481008caa 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1266,11 +1266,17 @@ ExecParallelFinish(ParallelExecutorInfo *pei)
 	WaitForParallelWorkersToFinish(pei->pcxt);
 
 	/*
-	 * Next, accumulate buffer/WAL usage.  (This must wait for the workers to
-	 * finish, or we might get incomplete data.)
+	 * Next, accumulate the workers' buffer/WAL usage into the query-level
+	 * instrumentation, if any was requested.  It is intentionally not folded
+	 * into the global pgBufferUsage/pgWalUsage: each worker reports its own
+	 * usage to the cluster-wide stats, so doing so here would double-count
+	 * it. (This must wait for the workers to finish, or we might get
+	 * incomplete data.)
 	 */
-	for (i = 0; i < nworkers; i++)
-		InstrAccumParallelQuery(&pei->buffer_usage[i], &pei->wal_usage[i]);
+	if (pei->planstate->state->es_query_instr)
+		for (i = 0; i < nworkers; i++)
+			InstrAccumParallelQuery(pei->planstate->state->es_query_instr,
+									&pei->buffer_usage[i], &pei->wal_usage[i]);
 
 	pei->finished = true;
 }
diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c
index ffbcd57213396..bc54d7de0ba9d 100644
--- a/src/backend/executor/instrument.c
+++ b/src/backend/executor/instrument.c
@@ -292,12 +292,16 @@ InstrEndParallelQuery(BufferUsage *bufusage, WalUsage *walusage)
 	WalUsageAccumDiff(walusage, &pgWalUsage, &save_pgWalUsage);
 }
 
-/* accumulate work done by workers in leader's stats */
+/*
+ * Accumulate a parallel worker's usage into the given query/command-level
+ * instrumentation.  Not added to the global pgBufferUsage/pgWalUsage, which
+ * feed per-process cluster stats each worker already reports for itself.
+ */
 void
-InstrAccumParallelQuery(BufferUsage *bufusage, WalUsage *walusage)
+InstrAccumParallelQuery(Instrumentation *dst, BufferUsage *bufusage, WalUsage *walusage)
 {
-	BufferUsageAdd(&pgBufferUsage, bufusage);
-	WalUsageAdd(&pgWalUsage, walusage);
+	BufferUsageAdd(&dst->bufusage, bufusage);
+	WalUsageAdd(&dst->walusage, walusage);
 }
 
 /* dst += add */
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 0b4009ff7ccac..c30059990f5f7 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -57,6 +57,7 @@
 #include "commands/vacuum.h"
 #include "commands/view.h"
 #include "commands/wait.h"
+#include "executor/instrument.h"
 #include "miscadmin.h"
 #include "parser/parse_utilcmd.h"
 #include "postmaster/bgwriter.h"
@@ -70,6 +71,14 @@
 /* Hook for plugins to get control in ProcessUtility() */
 ProcessUtility_hook_type ProcessUtility_hook = NULL;
 
+/*
+ * The utility statement currently executing, or NULL.  Set around each
+ * ProcessUtility() call so that code with no direct handle on the descriptor
+ * (e.g. a parallel CREATE INDEX or VACUUM leader importing worker usage into
+ * the command's instrumentation) can reach it.
+ */
+UtilityStmtDesc *ActiveUtilityStmtDesc = NULL;
+
 /* local function declarations */
 static int	ClassifyUtilityCommandAsReadOnly(Node *parsetree);
 static void ProcessUtilitySlow(ParseState *pstate,
@@ -500,20 +509,36 @@ CheckRestrictedOperation(const char *cmdname)
 void
 ProcessUtility(UtilityStmtDesc *desc)
 {
+	UtilityStmtDesc *save_ActiveUtilityStmtDesc;
+
 	Assert(IsA(desc->pstmt, PlannedStmt));
 	Assert(desc->pstmt->commandType == CMD_UTILITY);
 	Assert(desc->queryString != NULL);	/* required as of 8.4 */
 	Assert(desc->qc == NULL || desc->qc->commandTag == CMDTAG_UNKNOWN);
 
 	/*
+	 * Publish the descriptor as the active utility statement for the duration
+	 * of the call (saving/restoring any outer one, so nesting and errors are
+	 * handled), then run it.
+	 *
 	 * We provide a function hook variable that lets loadable plugins get
 	 * control when ProcessUtility is called.  Such a plugin would normally
 	 * call standard_ProcessUtility().
 	 */
-	if (ProcessUtility_hook)
-		(*ProcessUtility_hook) (desc);
-	else
-		standard_ProcessUtility(desc);
+	save_ActiveUtilityStmtDesc = ActiveUtilityStmtDesc;
+	ActiveUtilityStmtDesc = desc;
+	PG_TRY();
+	{
+		if (ProcessUtility_hook)
+			(*ProcessUtility_hook) (desc);
+		else
+			standard_ProcessUtility(desc);
+	}
+	PG_FINALLY();
+	{
+		ActiveUtilityStmtDesc = save_ActiveUtilityStmtDesc;
+	}
+	PG_END_TRY();
 }
 
 /*
@@ -550,6 +575,17 @@ standard_ProcessUtility(UtilityStmtDesc *desc)
 	/* This can recurse, so check for excessive recursion */
 	check_stack_depth();
 
+	/*
+	 * Set up statement-level instrumentation if a consumer requested it via
+	 * desc->instr_options (see UtilityStmtDesc).  We bracket the whole
+	 * command below so the totals cover any recursively-processed
+	 * sub-commands too.
+	 */
+	if (desc->instr_options)
+		desc->instr = InstrAlloc(desc->instr_options);
+	if (desc->instr)
+		InstrStart(desc->instr);
+
 	/*
 	 * If the given node tree is read-only, make a copy to ensure that parse
 	 * transformations don't damage the original tree.  This could be
@@ -1070,6 +1106,9 @@ standard_ProcessUtility(UtilityStmtDesc *desc)
 	 * #15631).
 	 */
 	CommandCounterIncrement();
+
+	if (desc->instr)
+		InstrStop(desc->instr);
 }
 
 /*
diff --git a/src/include/executor/instrument.h b/src/include/executor/instrument.h
index f093a52aae013..e7c9f9ad4419c 100644
--- a/src/include/executor/instrument.h
+++ b/src/include/executor/instrument.h
@@ -149,7 +149,8 @@ extern void InstrStopTrigger(TriggerInstrumentation *tginstr, int64 firings);
 
 extern void InstrStartParallelQuery(void);
 extern void InstrEndParallelQuery(BufferUsage *bufusage, WalUsage *walusage);
-extern void InstrAccumParallelQuery(BufferUsage *bufusage, WalUsage *walusage);
+extern void InstrAccumParallelQuery(Instrumentation *dst,
+									BufferUsage *bufusage, WalUsage *walusage);
 extern void BufferUsageAccumDiff(BufferUsage *dst,
 								 const BufferUsage *add, const BufferUsage *sub);
 extern void WalUsageAccumDiff(WalUsage *dst, const WalUsage *add,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index fe8681ba642e7..767035c1c9d25 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -732,6 +732,7 @@ typedef struct EState
 
 	int			es_top_eflags;	/* eflags passed to ExecutorStart */
 	int			es_instrument;	/* OR of InstrumentOption flags */
+	struct Instrumentation *es_query_instr; /* query-level instrumentation */
 	bool		es_finished;	/* true when ExecutorFinish is done */
 
 	List	   *es_exprcontexts;	/* List of ExprContexts within EState */
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index ae61ff90412a1..199ccaedcc573 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -86,12 +86,27 @@ typedef struct UtilityStmtDesc
 	DestReceiver *dest;			/* where to send results, if any */
 	QueryCompletion *qc;		/* where to store command completion status,
 								 * or NULL */
+
+	/*
+	 * Statement-level instrumentation, the utility-command counterpart of
+	 * QueryDesc's query_instr.  A consumer (e.g. pg_stat_statements) requests
+	 * it by OR-ing InstrumentOption flags into instr_options before the
+	 * command runs; standard_ProcessUtility then allocates instr and brackets
+	 * the command with it, and the consumer reads the totals afterwards. Left
+	 * at 0/NULL when nobody requested instrumentation.
+	 */
+	int			instr_options;	/* OR of InstrumentOption flags for instr */
+	struct Instrumentation *instr;	/* statement-level instrumentation, or
+									 * NULL */
 } UtilityStmtDesc;
 
 /* Hook for plugins to get control in ProcessUtility() */
 typedef void (*ProcessUtility_hook_type) (UtilityStmtDesc *desc);
 extern PGDLLIMPORT ProcessUtility_hook_type ProcessUtility_hook;
 
+/* The utility statement currently executing, or NULL (see utility.c) */
+extern PGDLLIMPORT UtilityStmtDesc *ActiveUtilityStmtDesc;
+
 extern void ProcessUtility(UtilityStmtDesc *desc);
 extern void standard_ProcessUtility(UtilityStmtDesc *desc);
 

From a1acf2fb37967d2b4a9a485af08532c91071341c Mon Sep 17 00:00:00 2001
From: Lukas Fittl <lukas@fittl.com>
Date: Wed, 28 Oct 2026 15:27:56 -0700
Subject: [PATCH 3/3] pgstat: Derive pg_stat_database I/O timings from
 instrumentation

Previously pg_stat_database's blk_read_time and blk_write_time counters
were accumulated separately via the backend-local pgStatBlockReadTime
and pgStatBlockWriteTime, despite the same information already being
tracked in pgBufferUsage.

Remove the separate counters and instead diff pgBufferUsage on the
pre-existing flush interval to capture the I/O timings. This reporting
happens separately on the leader and each worker in a parallel query,
benefiting from the recent fix that ensured that pgBufferUsage on the
leader does not include worker activity.

In passing, update the ANALYZE command to also derive its I/O timing log
output from pgBufferUsage (via Instrumentation helpers), avoiding a
dependency on the now removed pgStatBlock(Read|Write)Time globals.

Author: Lukas Fittl <lukas@fittl.com>
Reviewed-by:
Discussion:
---
 src/backend/commands/analyze.c               | 18 ++----
 src/backend/utils/activity/pgstat_database.c | 58 +++++++++++++++++---
 src/backend/utils/activity/pgstat_io.c       | 15 ++---
 src/include/pgstat.h                         |  8 ---
 4 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 518d7526255b5..197f4e65566fb 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -357,8 +357,6 @@ do_analyze_rel(Relation onerel, const VacuumParams *params,
 	WalUsage	startwalusage = pgWalUsage;
 	BufferUsage startbufferusage = pgBufferUsage;
 	BufferUsage bufferusage;
-	PgStat_Counter startreadtime = 0;
-	PgStat_Counter startwritetime = 0;
 
 	verbose = (params->options & VACOPT_VERBOSE) != 0;
 	instrument = (verbose || (AmAutoVacuumWorkerProcess() &&
@@ -396,18 +394,10 @@ do_analyze_rel(Relation onerel, const VacuumParams *params,
 
 	/*
 	 * When verbose or autovacuum logging is used, initialize a resource usage
-	 * snapshot and optionally track I/O timing.
+	 * snapshot.
 	 */
 	if (instrument)
-	{
-		if (track_io_timing)
-		{
-			startreadtime = pgStatBlockReadTime;
-			startwritetime = pgStatBlockWriteTime;
-		}
-
 		pg_rusage_init(&ru0);
-	}
 
 	/* Used for instrumentation and stats report */
 	starttime = GetCurrentTimestamp();
@@ -852,8 +842,10 @@ do_analyze_rel(Relation onerel, const VacuumParams *params,
 			}
 			if (track_io_timing)
 			{
-				double		read_ms = (double) (pgStatBlockReadTime - startreadtime) / 1000;
-				double		write_ms = (double) (pgStatBlockWriteTime - startwritetime) / 1000;
+				double		read_ms = INSTR_TIME_GET_MILLISEC(bufferusage.shared_blk_read_time) +
+					INSTR_TIME_GET_MILLISEC(bufferusage.local_blk_read_time);
+				double		write_ms = INSTR_TIME_GET_MILLISEC(bufferusage.shared_blk_write_time) +
+					INSTR_TIME_GET_MILLISEC(bufferusage.local_blk_write_time);
 
 				appendStringInfo(&buf, _("I/O timings: read: %.3f ms, write: %.3f ms\n"),
 								 read_ms, write_ms);
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index 7f3bc0165931c..cfb6308731332 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -17,16 +17,25 @@
 
 #include "postgres.h"
 
+#include "executor/instrument.h"
 #include "storage/standby.h"
 #include "utils/pgstat_internal.h"
 #include "utils/timestamp.h"
 
 
 static bool pgstat_should_report_connstat(void);
+static PgStat_Counter pgstat_block_io_time(const instr_time *shared_time,
+										   const instr_time *local_time);
 
 
-PgStat_Counter pgStatBlockReadTime = 0;
-PgStat_Counter pgStatBlockWriteTime = 0;
+/*
+ * Block I/O timings (in microseconds) of this process' own activity already
+ * reported to pg_stat_database, so that pgstat_update_dbstats() can compute how
+ * much they have increased since the previous report.
+ */
+static PgStat_Counter prevBlockReadTime = 0;
+static PgStat_Counter prevBlockWriteTime = 0;
+
 PgStat_Counter pgStatActiveTime = 0;
 PgStat_Counter pgStatTransactionIdleTime = 0;
 SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
@@ -344,13 +353,35 @@ pgstat_update_dbstats(TimestampTz ts)
 	dbentry = pgstat_prep_database_pending(MyDatabaseId);
 
 	/*
-	 * Accumulate xact commit/rollback and I/O timings to stats entry of the
-	 * current database.
+	 * Accumulate xact commit/rollback to stats entry of the current database.
 	 */
 	dbentry->xact_commit += pgStatXactCommit;
 	dbentry->xact_rollback += pgStatXactRollback;
-	dbentry->blk_read_time += pgStatBlockReadTime;
-	dbentry->blk_write_time += pgStatBlockWriteTime;
+
+	/*
+	 * Accumulate block I/O timings from the buffer usage instrumentation
+	 * (pgBufferUsage), which counts shared and local block read/write times
+	 * (not temporary files).  pgBufferUsage reflects only this process' own
+	 * activity: a parallel leader keeps the usage imported from its workers
+	 * in the query-level instrumentation rather than in pgBufferUsage, and
+	 * each worker reports its own activity, so no adjustment is needed here.
+	 * The counters only ever increase, so report the growth since the last
+	 * report.
+	 */
+	{
+		PgStat_Counter read_us,
+					write_us;
+
+		read_us = pgstat_block_io_time(&pgBufferUsage.shared_blk_read_time,
+									   &pgBufferUsage.local_blk_read_time);
+		write_us = pgstat_block_io_time(&pgBufferUsage.shared_blk_write_time,
+										&pgBufferUsage.local_blk_write_time);
+
+		dbentry->blk_read_time += read_us - prevBlockReadTime;
+		dbentry->blk_write_time += write_us - prevBlockWriteTime;
+		prevBlockReadTime = read_us;
+		prevBlockWriteTime = write_us;
+	}
 
 	if (pgstat_should_report_connstat())
 	{
@@ -370,8 +401,6 @@ pgstat_update_dbstats(TimestampTz ts)
 
 	pgStatXactCommit = 0;
 	pgStatXactRollback = 0;
-	pgStatBlockReadTime = 0;
-	pgStatBlockWriteTime = 0;
 	pgStatActiveTime = 0;
 	pgStatTransactionIdleTime = 0;
 }
@@ -389,6 +418,19 @@ pgstat_should_report_connstat(void)
 	return MyBackendType == B_BACKEND;
 }
 
+/*
+ * Total of the given shared and local block I/O times in microseconds, which
+ * is what pg_stat_database counts as block read/write time.
+ */
+static PgStat_Counter
+pgstat_block_io_time(const instr_time *shared_time, const instr_time *local_time)
+{
+	instr_time	total = *shared_time;
+
+	INSTR_TIME_ADD(total, *local_time);
+	return INSTR_TIME_GET_MICROSEC(total);
+}
+
 /*
  * Find or create a local PgStat_StatDBEntry entry for dboid.
  */
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 3c8fcb20abac6..af91a4a99c822 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -102,13 +102,12 @@ pgstat_prepare_io_time(bool track_io_guc)
 /*
  * Like pgstat_count_io_op() except it also accumulates time.
  *
- * The calls related to pgstat_count_buffer_*() are for pgstat_database.  As
- * pg_stat_database only counts block read and write times, these are done for
- * IOOP_READ, IOOP_WRITE and IOOP_EXTEND.
- *
- * pgBufferUsage is used for EXPLAIN.  pgBufferUsage has write and read stats
- * for shared, local and temporary blocks.  pg_stat_io does not track the
- * activity of temporary blocks, so these are ignored here.
+ * The block read/write times are recorded in pgBufferUsage, which feeds
+ * EXPLAIN and pg_stat_statements, and also pg_stat_database, which derives its
+ * blk_read_time/blk_write_time from pgBufferUsage at flush time (see
+ * pgstat_update_dbstats()).  They are tracked for shared and local blocks on
+ * IOOP_READ, IOOP_WRITE and IOOP_EXTEND.  pgBufferUsage additionally tracks
+ * temporary block I/O, but pg_stat_io does not, so that is ignored here.
  */
 void
 pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
@@ -125,7 +124,6 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 		{
 			if (io_op == IOOP_WRITE || io_op == IOOP_EXTEND)
 			{
-				pgstat_count_buffer_write_time(INSTR_TIME_GET_MICROSEC(io_time));
 				if (io_object == IOOBJECT_RELATION)
 					INSTR_TIME_ADD(pgBufferUsage.shared_blk_write_time, io_time);
 				else if (io_object == IOOBJECT_TEMP_RELATION)
@@ -133,7 +131,6 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 			}
 			else if (io_op == IOOP_READ)
 			{
-				pgstat_count_buffer_read_time(INSTR_TIME_GET_MICROSEC(io_time));
 				if (io_object == IOOBJECT_RELATION)
 					INSTR_TIME_ADD(pgBufferUsage.shared_blk_read_time, io_time);
 				else if (io_object == IOOBJECT_TEMP_RELATION)
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 187d82c96fefd..5efd61565fe42 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -739,10 +739,6 @@ extern void pgstat_report_connect(Oid dboid);
 extern void pgstat_update_parallel_workers_stats(PgStat_Counter workers_to_launch,
 												 PgStat_Counter workers_launched);
 
-#define pgstat_count_buffer_read_time(n)							\
-	(pgStatBlockReadTime += (n))
-#define pgstat_count_buffer_write_time(n)							\
-	(pgStatBlockWriteTime += (n))
 #define pgstat_count_conn_active_time(n)							\
 	(pgStatActiveTime += (n))
 #define pgstat_count_conn_txn_idle_time(n)							\
@@ -981,10 +977,6 @@ extern PGDLLIMPORT PgStat_CheckpointerStats PendingCheckpointerStats;
  * Variables in pgstat_database.c
  */
 
-/* Updated by pgstat_count_buffer_*_time macros */
-extern PGDLLIMPORT PgStat_Counter pgStatBlockReadTime;
-extern PGDLLIMPORT PgStat_Counter pgStatBlockWriteTime;
-
 /*
  * Updated by pgstat_count_conn_*_time macros, called by
  * pgstat_report_activity().