Separate state from query string in pg_stat_activity

This separates the state (running/idle/idleintransaction etc) into
it's own field ("state"), and leaves the query field containing just
query text.

The query text will now mean "current query" when a query is running
and "last query" in other states. Accordingly,the field has been
renamed from current_query to query.

Since backwards compatibility was broken anyway to make that, the procpid
field has also been renamed to pid - along with the same field in
pg_stat_replication for consistency.

Scott Mead and Magnus Hagander, review work from Greg Smith

5 files changed, 134 insertions, 65 deletions

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 50ba20cce2a..e25914b3065 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -520,7 +520,7 @@ CREATE VIEW pg_stat_activity AS
     SELECT
             S.datid AS datid,
             D.datname AS datname,
-            S.procpid,
+            S.pid,
             S.usesysid,
             U.rolname AS usename,
             S.application_name,
@@ -530,15 +530,17 @@ CREATE VIEW pg_stat_activity AS
             S.backend_start,
             S.xact_start,
             S.query_start,
+            S.state_change,
             S.waiting,
-            S.current_query
+            S.state,
+            S.query
     FROM pg_database D, pg_stat_get_activity(NULL) AS S, pg_authid U
     WHERE S.datid = D.oid AND
             S.usesysid = U.oid;
 
 CREATE VIEW pg_stat_replication AS
     SELECT
-            S.procpid,
+            S.pid,
             S.usesysid,
             U.rolname AS usename,
             S.application_name,
@@ -556,7 +558,7 @@ CREATE VIEW pg_stat_replication AS
     FROM pg_stat_get_activity(NULL) AS S, pg_authid U,
             pg_stat_get_wal_senders() AS W
     WHERE S.usesysid = U.oid AND
-            S.procpid = W.procpid;
+            S.pid = W.pid;
 
 CREATE VIEW pg_stat_database AS
     SELECT
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index f858a6df19d..e84e21c62cd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2781,7 +2781,7 @@ autovac_report_activity(autovac_table *tab)
 	/* Set statement_timestamp() to current time for pg_stat_activity */
 	SetCurrentStatementStartTimestamp();
 
-	pgstat_report_activity(activity);
+	pgstat_report_activity(STATE_RUNNING, activity);
 }
 
 /*
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 323d42b5175..3ab8dfe065b 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2410,12 +2410,14 @@ pgstat_bestart(void)
 	beentry->st_procpid = MyProcPid;
 	beentry->st_proc_start_timestamp = proc_start_timestamp;
 	beentry->st_activity_start_timestamp = 0;
+	beentry->st_state_start_timestamp = 0;
 	beentry->st_xact_start_timestamp = 0;
 	beentry->st_databaseid = MyDatabaseId;
 	beentry->st_userid = userid;
 	beentry->st_clientaddr = clientaddr;
 	beentry->st_clienthostname[0] = '\0';
 	beentry->st_waiting = false;
+	beentry->st_state = STATE_UNDEFINED;
 	beentry->st_appname[0] = '\0';
 	beentry->st_activity[0] = '\0';
 	/* Also make sure the last byte in each string area is always 0 */
@@ -2476,39 +2478,70 @@ pgstat_beshutdown_hook(int code, Datum arg)
  *
  *	Called from tcop/postgres.c to report what the backend is actually doing
  *	(usually "<IDLE>" or the start of the query to be executed).
+ *
+ * All updates of the status entry follow the protocol of bumping
+ * st_changecount before and after.  We use a volatile pointer here to
+ * ensure the compiler doesn't try to get cute.
  * ----------
  */
 void
-pgstat_report_activity(const char *cmd_str)
+pgstat_report_activity(BackendState state, const char *cmd_str)
 {
 	volatile PgBackendStatus *beentry = MyBEEntry;
 	TimestampTz start_timestamp;
+	TimestampTz current_timestamp;
 	int			len;
 
 	TRACE_POSTGRESQL_STATEMENT_STATUS(cmd_str);
 
-	if (!pgstat_track_activities || !beentry)
+	if (!beentry)
 		return;
 
 	/*
 	 * To minimize the time spent modifying the entry, fetch all the needed
 	 * data first.
 	 */
-	start_timestamp = GetCurrentStatementStartTimestamp();
+	current_timestamp = GetCurrentTimestamp();
 
-	len = strlen(cmd_str);
-	len = pg_mbcliplen(cmd_str, len, pgstat_track_activity_query_size - 1);
+	if (!pgstat_track_activities && beentry->st_state != STATE_DISABLED)
+	{
+		/*
+		 * Track activities is disabled, but we have a non-disabled state set.
+		 * That means the status changed - so as our last update, tell the
+		 * collector that we disabled it and will no longer update.
+		 */
+		beentry->st_changecount++;
+		beentry->st_state = STATE_DISABLED;
+		beentry->st_state_start_timestamp = current_timestamp;
+		beentry->st_changecount++;
+		Assert((beentry->st_changecount & 1) == 0);
+		return;
+	}
 
 	/*
-	 * Update my status entry, following the protocol of bumping
-	 * st_changecount before and after.  We use a volatile pointer here to
-	 * ensure the compiler doesn't try to get cute.
+	 * Fetch more data before we start modifying the entry
+	 */
+	start_timestamp = GetCurrentStatementStartTimestamp();
+	if (cmd_str != NULL)
+	{
+		len = strlen(cmd_str);
+		len = pg_mbcliplen(cmd_str, len, pgstat_track_activity_query_size - 1);
+	}
+
+	/*
+	 * Now update the status entry
 	 */
 	beentry->st_changecount++;
 
-	beentry->st_activity_start_timestamp = start_timestamp;
-	memcpy((char *) beentry->st_activity, cmd_str, len);
-	beentry->st_activity[len] = '\0';
+	beentry->st_state = state;
+	beentry->st_state_start_timestamp = current_timestamp;
+
+	if (cmd_str != NULL)
+	{
+		memcpy((char *) beentry->st_activity, cmd_str, len);
+		beentry->st_activity[len] = '\0';
+		beentry->st_activity_start_timestamp = start_timestamp;
+	}
 
 	beentry->st_changecount++;
 	Assert((beentry->st_changecount & 1) == 0);
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 59a287f79bf..49a396918d9 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -809,7 +809,7 @@ exec_simple_query(const char *query_string)
 	 */
 	debug_query_string = query_string;
 
-	pgstat_report_activity(query_string);
+	pgstat_report_activity(STATE_RUNNING, query_string);
 
 	TRACE_POSTGRESQL_QUERY_START(query_string);
 
@@ -1134,7 +1134,7 @@ exec_parse_message(const char *query_string,	/* string to execute */
 	 */
 	debug_query_string = query_string;
 
-	pgstat_report_activity(query_string);
+	pgstat_report_activity(STATE_RUNNING, query_string);
 
 	set_ps_display("PARSE", false);
 
@@ -1429,7 +1429,7 @@ exec_bind_message(StringInfo input_message)
 	 */
 	debug_query_string = psrc->query_string;
 
-	pgstat_report_activity(psrc->query_string);
+	pgstat_report_activity(STATE_RUNNING, psrc->query_string);
 
 	set_ps_display("BIND", false);
 
@@ -1836,7 +1836,7 @@ exec_execute_message(const char *portal_name, long max_rows)
 	 */
 	debug_query_string = sourceText;
 
-	pgstat_report_activity(sourceText);
+	pgstat_report_activity(STATE_RUNNING, sourceText);
 
 	set_ps_display(portal->commandTag, false);
 
@@ -3811,12 +3811,12 @@ PostgresMain(int argc, char *argv[], const char *username)
 			if (IsAbortedTransactionBlockState())
 			{
 				set_ps_display("idle in transaction (aborted)", false);
-				pgstat_report_activity("<IDLE> in transaction (aborted)");
+				pgstat_report_activity(STATE_IDLEINTRANSACTION_ABORTED, NULL);
 			}
 			else if (IsTransactionOrTransactionBlock())
 			{
 				set_ps_display("idle in transaction", false);
-				pgstat_report_activity("<IDLE> in transaction");
+				pgstat_report_activity(STATE_IDLEINTRANSACTION, NULL);
 			}
 			else
 			{
@@ -3824,7 +3824,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 				pgstat_report_stat(false);
 
 				set_ps_display("idle", false);
-				pgstat_report_activity("<IDLE>");
+				pgstat_report_activity(STATE_IDLE, NULL);
 			}
 
 			ReadyForQuery(whereToSendOutput);
@@ -3944,7 +3944,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 				SetCurrentStatementStartTimestamp();
 
 				/* Report query to various monitoring facilities. */
-				pgstat_report_activity("<FASTPATH> function call");
+				pgstat_report_activity(STATE_FASTPATH, NULL);
 				set_ps_display("<FASTPATH>", false);
 
 				/* start an xact for this function invocation */
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index b4986d80bab..ed39f27ef48 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -507,31 +507,34 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 
 		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
 
-		tupdesc = CreateTemplateTupleDesc(12, false);
+		tupdesc = CreateTemplateTupleDesc(14, false);
 		TupleDescInitEntry(tupdesc, (AttrNumber) 1, "datid",
 						   OIDOID, -1, 0);
-		/* This should have been called 'pid';  can't change it. 2011-06-11 */
-		TupleDescInitEntry(tupdesc, (AttrNumber) 2, "procpid",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 2, "pid",
 						   INT4OID, -1, 0);
 		TupleDescInitEntry(tupdesc, (AttrNumber) 3, "usesysid",
 						   OIDOID, -1, 0);
 		TupleDescInitEntry(tupdesc, (AttrNumber) 4, "application_name",
 						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 5, "current_query",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 5, "state",
 						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 6, "waiting",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 6, "query",
+						   TEXTOID, -1, 0);
+		TupleDescInitEntry(tupdesc, (AttrNumber) 7, "waiting",
 						   BOOLOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 7, "act_start",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 8, "act_start",
+						   TIMESTAMPTZOID, -1, 0);
+		TupleDescInitEntry(tupdesc, (AttrNumber) 9, "query_start",
 						   TIMESTAMPTZOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 8, "query_start",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 10, "backend_start",
 						   TIMESTAMPTZOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 9, "backend_start",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 11, "state_change",
 						   TIMESTAMPTZOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 10, "client_addr",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 12, "client_addr",
 						   INETOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 11, "client_hostname",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 13, "client_hostname",
 						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 12, "client_port",
+		TupleDescInitEntry(tupdesc, (AttrNumber) 14, "client_port",
 						   INT4OID, -1, 0);
 
 		funcctx->tuple_desc = BlessTupleDesc(tupdesc);
@@ -584,8 +587,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 	if (funcctx->call_cntr < funcctx->max_calls)
 	{
 		/* for each row */
-		Datum		values[12];
-		bool		nulls[12];
+		Datum		values[14];
+		bool		nulls[14];
 		HeapTuple	tuple;
 		PgBackendStatus *beentry;
 		SockAddr	zero_clientaddr;
@@ -610,8 +613,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			for (i = 0; i < sizeof(nulls) / sizeof(nulls[0]); i++)
 				nulls[i] = true;
 
-			nulls[4] = false;
-			values[4] = CStringGetTextDatum("<backend information not available>");
+			nulls[5] = false;
+			values[5] = CStringGetTextDatum("<backend information not available>");
 
 			tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls);
 			SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
@@ -629,40 +632,69 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 		/* Values only available to same user or superuser */
 		if (superuser() || beentry->st_userid == GetUserId())
 		{
-			if (*(beentry->st_activity) == '\0')
+			switch (beentry->st_state)
+			{
+				case STATE_IDLE:
+					values[4] = CStringGetTextDatum("idle");
+					break;
+				case STATE_RUNNING:
+					values[4] = CStringGetTextDatum("active");
+					break;
+				case STATE_IDLEINTRANSACTION:
+					values[4] = CStringGetTextDatum("idle in transaction");
+					break;
+				case STATE_FASTPATH:
+					values[4] = CStringGetTextDatum("fastpath function call");
+					break;
+				case STATE_IDLEINTRANSACTION_ABORTED:
+					values[4] = CStringGetTextDatum("idle in transaction (aborted)");
+					break;
+				case STATE_DISABLED:
+					values[4] = CStringGetTextDatum("disabled");
+					break;
+				case STATE_UNDEFINED:
+					nulls[4] = true;
+					break;
+			}
+			if (beentry->st_state == STATE_UNDEFINED ||
+				beentry->st_state == STATE_DISABLED)
 			{
-				values[4] = CStringGetTextDatum("<command string not enabled>");
+				values[5] = CStringGetTextDatum("");
 			}
 			else
 			{
-				values[4] = CStringGetTextDatum(beentry->st_activity);
+				values[5] = CStringGetTextDatum(beentry->st_activity);
 			}
-
-			values[5] = BoolGetDatum(beentry->st_waiting);
+			values[6] = BoolGetDatum(beentry->st_waiting);
 
 			if (beentry->st_xact_start_timestamp != 0)
-				values[6] = TimestampTzGetDatum(beentry->st_xact_start_timestamp);
+				values[7] = TimestampTzGetDatum(beentry->st_xact_start_timestamp);
 			else
-				nulls[6] = true;
+				nulls[7] = true;
 
 			if (beentry->st_activity_start_timestamp != 0)
-				values[7] = TimestampTzGetDatum(beentry->st_activity_start_timestamp);
+				values[8] = TimestampTzGetDatum(beentry->st_activity_start_timestamp);
 			else
-				nulls[7] = true;
+				nulls[8] = true;
 
 			if (beentry->st_proc_start_timestamp != 0)
-				values[8] = TimestampTzGetDatum(beentry->st_proc_start_timestamp);
+				values[9] = TimestampTzGetDatum(beentry->st_proc_start_timestamp);
 			else
-				nulls[8] = true;
+				nulls[9] = true;
+
+			if (beentry->st_state_start_timestamp != 0)
+				values[10] = TimestampTzGetDatum(beentry->st_state_start_timestamp);
+			else
+				nulls[10] = true;
 
 			/* A zeroed client addr means we don't know */
 			memset(&zero_clientaddr, 0, sizeof(zero_clientaddr));
 			if (memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
 					   sizeof(zero_clientaddr) == 0))
 			{
-				nulls[9] = true;
-				nulls[10] = true;
 				nulls[11] = true;
+				nulls[12] = true;
+				nulls[13] = true;
 			}
 			else
 			{
@@ -686,19 +718,19 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 					if (ret == 0)
 					{
 						clean_ipv6_addr(beentry->st_clientaddr.addr.ss_family, remote_host);
-						values[9] = DirectFunctionCall1(inet_in,
+						values[11] = DirectFunctionCall1(inet_in,
 											   CStringGetDatum(remote_host));
 						if (beentry->st_clienthostname)
-							values[10] = CStringGetTextDatum(beentry->st_clienthostname);
+							values[12] = CStringGetTextDatum(beentry->st_clienthostname);
 						else
-							nulls[10] = true;
-						values[11] = Int32GetDatum(atoi(remote_port));
+							nulls[12] = true;
+						values[13] = Int32GetDatum(atoi(remote_port));
 					}
 					else
 					{
-						nulls[9] = true;
-						nulls[10] = true;
 						nulls[11] = true;
+						nulls[12] = true;
+						nulls[13] = true;
 					}
 				}
 				else if (beentry->st_clientaddr.addr.ss_family == AF_UNIX)
@@ -709,30 +741,32 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 					 * connections we have no permissions to view, or with
 					 * errors.
 					 */
-					nulls[9] = true;
-					nulls[10] = true;
-					values[11] = DatumGetInt32(-1);
+					nulls[11] = true;
+					nulls[12] = true;
+					values[13] = DatumGetInt32(-1);
 				}
 				else
 				{
 					/* Unknown address type, should never happen */
-					nulls[9] = true;
-					nulls[10] = true;
 					nulls[11] = true;
+					nulls[12] = true;
+					nulls[13] = true;
 				}
 			}
 		}
 		else
 		{
 			/* No permissions to view data about this session */
-			values[4] = CStringGetTextDatum("<insufficient privilege>");
-			nulls[5] = true;
+			values[5] = CStringGetTextDatum("<insufficient privilege>");
+			nulls[4] = true;
 			nulls[6] = true;
 			nulls[7] = true;
 			nulls[8] = true;
 			nulls[9] = true;
 			nulls[10] = true;
 			nulls[11] = true;
+			nulls[12] = true;
+			nulls[13] = true;
 		}
 
 		tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls);