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

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