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

At Mon, 13 Feb 2023 05:00:01 +0000, PG Bug reporting form <noreply(at)postgresql(dot)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(at)entry=0x7fb1feb3f81e "MyProcPort != NULL",
> errorType=errorType(at)entry=0x7fb1feb3e700 "FailedAssertion",
> fileName=fileName(at)entry=0x7fb1feb3f75a "option.c",
> lineNumber=lineNumber(at)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