Re: BUG #17789: process_pgfdw_appname() fails for autovacuum workers

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-02-14T05:05:44Z
Lists: pgsql-bugs
At Mon, 13 Feb 2023 05:00:01 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in 
> CREATE FOREIGN TABLE ft1 (b int) INHERITS (t1)
> 	  SERVER LOOPBACK OPTIONS (table_name 'anytable');

Yeah, the autovacuum processes t1 and then its child foreign table
ft1. MyProcPort is NULL on autovacuum processes. This doesn't happen
for partitioned tables.

> #5  0x0000564b2ecc55c8 in ExceptionalCondition
> (conditionName=conditionName@entry=0x7fb1feb3f81e "MyProcPort != NULL", 
>     errorType=errorType@entry=0x7fb1feb3e700 "FailedAssertion",
> fileName=fileName@entry=0x7fb1feb3f75a "option.c", 
>     lineNumber=lineNumber@entry=464) at assert.c:69
> #6  0x00007fb1feb31b30 in process_pgfdw_appname (appname=<optimized out>) at
> option.c:464

Commit 6e0cb3dec1 overlooked the case of autovacuum analyzing foreign
tables as an inheritance child. We thought NULL MyProcPort was
impossible in the discussion for the patch. Using %d and %u in
postgres_fdw.application_name we hit the bug.

The following change fixes the bug.

- Is it okay to use '-' as %u (user name) and %d (databsae name) in
  the NULL MyProcPort case?

- Do we need tests for the case? They need to wait for autovacuum to run.



diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 984e4d168a..b6239a46a3 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -485,8 +485,6 @@ process_pgfdw_appname(const char *appname)
 	const char *p;
 	StringInfoData buf;
 
-	Assert(MyProcPort != NULL);
-
 	initStringInfo(&buf);
 
 	for (p = appname; *p != '\0'; p++)
@@ -522,13 +520,21 @@ process_pgfdw_appname(const char *appname)
 				appendStringInfoString(&buf, cluster_name);
 				break;
 			case 'd':
-				appendStringInfoString(&buf, MyProcPort->database_name);
+				/* MyProcPort can be NULL on autovacuum proces */
+				if (MyProcPort)
+					appendStringInfoString(&buf, MyProcPort->database_name);
+				else
+					appendStringInfoChar(&buf, '-');
 				break;
 			case 'p':
 				appendStringInfo(&buf, "%d", MyProcPid);
 				break;
 			case 'u':
-				appendStringInfoString(&buf, MyProcPort->user_name);
+				if (MyProcPort)
+					appendStringInfoString(&buf, MyProcPort->user_name);
+				else
+					appendStringInfoChar(&buf, '-');
+					
 				break;
 			default:
 				/* format error - ignore it */

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Commits

  1. Fix handling of escape sequences in postgres_fdw.application_name

  2. postgres_fdw: Allow postgres_fdw.application_name to include escape sequences.