Re: GUC parameter ACLs and physical walsender - Mailing list pgsql-hackers
On Wed, Apr 22, 2026 at 8:27 PM John Naylor <johncnaylorls@gmail.com> wrote:
On Thu, Apr 23, 2026 at 2:19 AM Jeff Davis <pgsql@j-davis.com> wrote:
> 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?
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 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
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);
+++ 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
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
pgsql-hackers by date: