From e687ec472c5478716b31189e25e942ff3abf11bf Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Thu, 4 Jul 2024 12:09:39 +0900
Subject: [PATCH v1] Prevent incorrect entries in pg_stat_activity.

In pg_stat_activity, when the backend's state is "idle",
no transaction should be running, and xact_start
(indicating the transaction start time) should be NULL.

Previously, during the processing of "Describe" messages,
removing temp relations at exit, and handling client read interrupts,
the backend did not change its state from "idle" but still set xact_start,
causing incorrect pg_stat_activity entries. Additionally,
during temp relation removal or interrupt processing,
xact_start could incorrectly reflect the timestamp of
the last query executed.

To fix this, the backend now sets its state to "active"
when processing a "Describe" message.

For interrupts and temp relation removal, since these
involve brief transactions, it is unnecessary to treat them
as normal transactions. Instead of changing the state to
"active" and setting xact_start to the current timestamp,
this commit ensures the backend resets xact_start to NULL
in these cases.
---
 src/backend/access/transam/xact.c |  9 +++++++++
 src/backend/catalog/namespace.c   | 10 ++++++++++
 src/backend/tcop/postgres.c       | 15 +++++++++++++++
 src/include/access/xact.h         |  1 +
 4 files changed, 35 insertions(+)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d119ab909d..58921bc469 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -917,6 +917,15 @@ SetCurrentStatementStartTimestamp(void)
 		Assert(stmtStartTimestamp != 0);
 }
 
+/*
+ * ResetCurrentStatementStartTimestamp
+ */
+void
+ResetCurrentStatementStartTimestamp(void)
+{
+	stmtStartTimestamp = 0;
+}
+
 /*
  *	GetCurrentTransactionNestLevel
  *
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index a2510cf80c..78d530263d 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -4610,6 +4610,16 @@ RemoveTempRelationsCallback(int code, Datum arg)
 {
 	if (OidIsValid(myTempNamespace))	/* should always be true */
 	{
+		/*
+		 * Reset statement_timestamp() to 0 so that subsequent
+		 * StartTransactionCommand() will set pg_stat_activity.xact_start to
+		 * NULL. Otherwise, xact_start could be set to statement_timestamp()
+		 * that may indicate the timestamp of the last query executed, while
+		 * the backend's state remains "idle," leading to incorrect "idle"
+		 * entries with non-NULL xact_start in pg_stat_activity.
+		 */
+		ResetCurrentStatementStartTimestamp();
+
 		/* Need to ensure we have a usable transaction. */
 		AbortOutOfAnyTransaction();
 		StartTransactionCommand();
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e39c6804a7..04046f41d8 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -515,6 +515,17 @@ ProcessClientReadInterrupt(bool blocked)
 		/* Check for general interrupts that arrived before/while reading */
 		CHECK_FOR_INTERRUPTS();
 
+		/*
+		 * Reset statement_timestamp() to 0 so that ProcessCatchupInterrupt()
+		 * and ProcessNotifyInterrupt() will set pg_stat_activity.xact_start
+		 * to NULL when they start new transaction. Otherwise, xact_start
+		 * could be set to statement_timestamp() that may indicate the
+		 * timestamp of the last query executed, while the backend's state
+		 * remains "idle," leading to incorrect "idle" entries with non-NULL
+		 * xact_start in pg_stat_activity.
+		 */
+		ResetCurrentStatementStartTimestamp();
+
 		/* Process sinval catchup interrupts, if any */
 		if (catchupInterruptPending)
 			ProcessCatchupInterrupt();
@@ -2598,6 +2609,8 @@ exec_describe_statement_message(const char *stmt_name)
 {
 	CachedPlanSource *psrc;
 
+	pgstat_report_activity(STATE_RUNNING, NULL);
+
 	/*
 	 * Start up a transaction command. (Note that this will normally change
 	 * current memory context.) Nothing happens if we are already in one.
@@ -2692,6 +2705,8 @@ exec_describe_portal_message(const char *portal_name)
 {
 	Portal		portal;
 
+	pgstat_report_activity(STATE_RUNNING, NULL);
+
 	/*
 	 * Start up a transaction command. (Note that this will normally change
 	 * current memory context.) Nothing happens if we are already in one.
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 6d4439f052..84745575be 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -450,6 +450,7 @@ extern TimestampTz GetCurrentTransactionStartTimestamp(void);
 extern TimestampTz GetCurrentStatementStartTimestamp(void);
 extern TimestampTz GetCurrentTransactionStopTimestamp(void);
 extern void SetCurrentStatementStartTimestamp(void);
+extern void ResetCurrentStatementStartTimestamp(void);
 extern int	GetCurrentTransactionNestLevel(void);
 extern bool TransactionIdIsCurrentTransactionId(TransactionId xid);
 extern void CommandCounterIncrement(void);
-- 
2.45.1