Subject: GUC parameter ACLs and physical walsender

From: Jeff Davis <[email-masked]>
Date: Wed, 22 Apr 2026 12:18:48 -0700

Moving discussion from:

https://www.postgresql.org/message-id/[email-masked]

because this is a separate issue. If you specify a SUSET GUC setting
when connecting as non-superuser for physical replication:

  PGOPTIONS="-c wal_compression=on" \
  pg_receivewal -D archive -U repl

you get:

  FATAL:  cannot read pg_class without having selected a database

but only if you connect immediately after the server starts. If you do
something else first, like an ordinary connection and "SELECT 1", and
then start the replication connection, you get (after commit
dbf217c1c7):

  FATAL:  permission denied to set parameter "wal_compression"

as expected. The problem goes back to a0ffa885e47.

It seems to be because pg_parameter_acl is not nailed in cache. I
attached a quick patch to do so (which turns it into the "expected
permission denied" error). But I'm not sure if that's the right fix, or
if it would be a complete fix. I also don't think that would be
backportable, but perhaps?

Regards,
	Jeff Davis

From 5cf20081de5172ec9a1d768b0584d40bfbf9cb12 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email-masked]>
Date: Wed, 22 Apr 2026 11:50:12 -0700
Subject: [PATCH v1] Nail pg_parameter_acl in relcache.

Previously, a parameter specified in the startup packet for a physical
replication connection could encounter an error trying to perform an
ACL check for the setting.
---
 src/backend/utils/cache/catcache.c     |  2 ++
 src/backend/utils/cache/relcache.c     | 19 ++++++++++++++-----
 src/include/catalog/pg_parameter_acl.h |  2 +-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index a8e7bf649d2..6fb35dedf95 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1335,6 +1335,8 @@ IndexScanOK(CatCache *cache)
 		case AUTHOID:
 		case AUTHMEMMEMROLE:
 		case DATABASEOID:
+		case PARAMETERACLNAME:
+		case PARAMETERACLOID:
 
 			/*
 			 * Protect authentication lookups occurring before relcache has
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index e19f0d3e51c..10d481518eb 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -56,6 +56,7 @@
 #include "catalog/pg_proc.h"
 #include "catalog/pg_publication.h"
 #include "catalog/pg_rewrite.h"
+#include "catalog/pg_parameter_acl.h"
 #include "catalog/pg_shseclabel.h"
 #include "catalog/pg_statistic_ext.h"
 #include "catalog/pg_subscription.h"
@@ -120,6 +121,7 @@ static const FormData_pg_attribute Desc_pg_auth_members[Natts_pg_auth_members] =
 static const FormData_pg_attribute Desc_pg_index[Natts_pg_index] = {Schema_pg_index};
 static const FormData_pg_attribute Desc_pg_shseclabel[Natts_pg_shseclabel] = {Schema_pg_shseclabel};
 static const FormData_pg_attribute Desc_pg_subscription[Natts_pg_subscription] = {Schema_pg_subscription};
+static const FormData_pg_attribute Desc_pg_parameter_acl[Natts_pg_parameter_acl] = {Schema_pg_parameter_acl};
 
 /*
  *		Hash tables that index the relation cache
@@ -4074,8 +4076,10 @@ RelationCacheInitializePhase2(void)
 				  Natts_pg_shseclabel, Desc_pg_shseclabel);
 		formrdesc("pg_subscription", SubscriptionRelation_Rowtype_Id, true,
 				  Natts_pg_subscription, Desc_pg_subscription);
+		formrdesc("pg_parameter_acl", ParameterAclRelation_Rowtype_Id, true,
+				  Natts_pg_parameter_acl, Desc_pg_parameter_acl);
 
-#define NUM_CRITICAL_SHARED_RELS	5	/* fix if you change list above */
+#define NUM_CRITICAL_SHARED_RELS	6	/* fix if you change list above */
 	}
 
 	MemoryContextSwitchTo(oldcxt);
@@ -4196,9 +4200,10 @@ RelationCacheInitializePhase3(void)
 	 * non-shared catalogs at all.  Autovacuum calls InitPostgres with a
 	 * database OID, so it instead depends on DatabaseOidIndexId.  We also
 	 * need to nail up some indexes on pg_authid and pg_auth_members for use
-	 * during client authentication.  SharedSecLabelObjectIndexId isn't
-	 * critical for the core system, but authentication hooks might be
-	 * interested in it.
+	 * during client authentication.  We need indexes on pg_parameter_acl for
+	 * ACL checks on settings specified in the startup packet for a physical
+	 * replication connection.  SharedSecLabelObjectIndexId isn't critical for
+	 * the core system, but authentication hooks might be interested in it.
 	 */
 	if (!criticalSharedRelcachesBuilt)
 	{
@@ -4214,8 +4219,12 @@ RelationCacheInitializePhase3(void)
 							AuthMemRelationId);
 		load_critical_index(SharedSecLabelObjectIndexId,
 							SharedSecLabelRelationId);
+		load_critical_index(ParameterAclParnameIndexId,
+							ParameterAclRelationId);
+		load_critical_index(ParameterAclOidIndexId,
+							ParameterAclRelationId);
 
-#define NUM_CRITICAL_SHARED_INDEXES 6	/* fix if you change list above */
+#define NUM_CRITICAL_SHARED_INDEXES 8	/* fix if you change list above */
 
 		criticalSharedRelcachesBuilt = true;
 	}
diff --git a/src/include/catalog/pg_parameter_acl.h b/src/include/catalog/pg_parameter_acl.h
index a26b05a9bf2..902e2666069 100644
--- a/src/include/catalog/pg_parameter_acl.h
+++ b/src/include/catalog/pg_parameter_acl.h
@@ -29,7 +29,7 @@
  */
 BEGIN_CATALOG_STRUCT
 
-CATALOG(pg_parameter_acl,6243,ParameterAclRelationId) BKI_SHARED_RELATION
+CATALOG(pg_parameter_acl,6243,ParameterAclRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(2173,ParameterAclRelation_Rowtype_Id) BKI_SCHEMA_MACRO
 {
 	Oid			oid;			/* oid */
 
-- 
2.43.0

---

From: John Naylor <[email-masked]>
Date: Thu, 23 Apr 2026 10:27:15 +0700

> backportable, but perhaps?

I think in existing installations AddNewRelationType() would have
picked some oid already, so fixing rowtype_oid at initdb time would
only work in the master branch.

-- 
John Naylor
Amazon Web Services

---

From: Mark Dilger <[email-masked]>
Date: Thu, 23 Apr 2026 10:57:28 -0700

> only work in the master branch.

John is right that the hardcoded BKI_ROWTYPE_OID(2173) makes this
non-backportable as-is.

On existing installations (PG 15-18), AddNewRelationType() assigned rowtype
OID
10097 to pg_parameter_acl during initdb. Jeff's patch has formrdesc set
rd_att->tdtypeid = 2173, but that field is never corrected — Phase3
overwrites
rd_rel (including reltype) via memcpy, but rd_att->tdtypeid stays at the
formrdesc value. The comment in formrdesc says as much: "this data had
better
be right because it will never be replaced."

On assert-enabled builds, every backend crashes on the first connection:

TRAP: failed Assert("relation->rd_att->tdtypeid == relp->reltype"),
  File: "relcache.c", Line: 4294

On non-assert builds, the mismatch has no observable effect. The wrong
typeId
gets stamped into pg_parameter_acl tuple headers by heap_form_tuple, but
nothing reads it back -- all consumers of HeapTupleHeaderGetTypeId are in
the
executor, PL/pgSQL, and record-type processing, not catalog operations.

For backports, formrdesc would need to use something other than the
hardcoded
OID -- either look up the real rowtype OID, or pass InvalidOid and teach the
Assert to tolerate it for late-nailed relations.

To demonstrate, using an existing cluster whose initdb was done without the
nailing patch (rowtype 10097), then started with patched binaries:  after
applying
Jeff's v1-0001-Nail-pg_parameter_acl-in-relcache.patch to master, which has
dbf217c1c7 already applied:

--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1965,6 +1965,12 @@ formrdesc(const char *relationName, Oid
relationReltype,
    relation->rd_att->tdtypeid = relationReltype;
    relation->rd_att->tdtypmod = -1;    /* just to be sure */

+   elog(LOG, "formrdesc \"%s\": relid=%u reltype=%u tdtypeid=%u",
+        relationName,
+        TupleDescAttr(relation->rd_att, 0)->attrelid,
+        relationReltype,
+        relation->rd_att->tdtypeid);
+
    /*
     * initialize tuple desc info
     */
@@ -4283,6 +4289,14 @@ RelationCacheInitializePhase3(void)
             * data correctly to start with, because it may already have
been
             * copied into one or more catcache entries.)
             */
+           elog(LOG, "RelationCacheInitializePhase3 \"%s\": relid=%u "
+                "formrdesc tdtypeid=%u, pg_class reltype=%u%s",
+                RelationGetRelationName(relation),
+                RelationGetRelid(relation),
+                relation->rd_att->tdtypeid,
+                relp->reltype,
+                (relation->rd_att->tdtypeid != relp->reltype)
+                ? " MISMATCH" : "");
            Assert(relation->rd_att->tdtypeid == relp->reltype);
            Assert(relation->rd_att->tdtypmod == -1);

On an existing cluster with patched binaries it shows:

  formrdesc "pg_parameter_acl": relid=0 reltype=2173 tdtypeid=2173
  RelationCacheInitializePhase3 "pg_parameter_acl": relid=6243
    formrdesc tdtypeid=2173, pg_class reltype=10097 MISMATCH

-- 

*Mark Dilger*

---

From: Jeff Davis <[email-masked]>
Date: Thu, 23 Apr 2026 14:06:18 -0700

> John is right that the hardcoded BKI_ROWTYPE_OID(2173) makes this

Right, but that leaves the questions:

(a) Is this the right fix for master?
(b) Is there anything we can do in the back branches, or we just leave
it as fix going forward only?

Regards,
	Jeff Davis

---

From: Mark Dilger <[email-masked]>
Date: Thu, 23 Apr 2026 16:41:13 -0700

> (a) Is this the right fix for master?

Yes.  This approach has no problem in master that I can see.

> it as fix going forward only?

I don't see a solution.  We could try to replace the error message with
something better, but even that seems hard to phrase.  Replacing
"cannot read pg_class without having selected a database" with, say,
"permission denied" would also be confusing for a role which does have
the privilege but just can't verify it.

-- 

*Mark Dilger*

---

From: Jeff Davis <[email-masked]>
Date: Fri, 24 Apr 2026 12:30:09 -0700

On Thu, 2026-04-23 at 16:41 -0700, Mark Dilger wrote:
> I don't see a solution.

OK. I'll wait a couple days to see if others have input on these two
points, and then I can commit this to master only. Ideally before
19beta1, because it would bump the catversion.

That would leave a bug in the back branches, but not a very serious one
as far as I can tell.

Regards,
	Jeff Davis