Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Mon, 2026-06-22 at 08:07 +0200, Hüseyin Demir wrote:
> > v4 applies the filter at all four sites where pg_dump queries pg_init_privs:
> > - getAggregates() — WHERE clause comparison
> > - getFuncs() — WHERE clause comparison
> > - getAdditionalACLs() — SELECT expression (object-level initprivs)
> > - PREPQUERY_GETCOLUMNACLS — SELECT expression (column-level initprivs,
> > objsubid != 0)
> > 
> > Secondly to avoid duplicating the multi-line subquery at every call
> > site, I introduced a SAFE_INITPRIVS(col) macro.

Great, that's just what I had in mind.

> One question from my side: can't we use function instead of macro ?
> Would it be more accurate for future readers ?

That would work too, but if you do it in C rather than with the
preprocessor, you have to deal with string manipulation, which
will makes the patch more complicated.  I think it is better the
way it is now.

I'll mark the patch "ready for committer".

Since there have been very few reports of this problem, the question
remains if we need this patch at all, or of it should be backpatched.
My opinion is that it should; every upgrade or restore failure is
one too many.

Yours,
Laurenz Albe

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Laurenz Albe  writes:
> What if there is a dangling role OID 65432 in pg_init_privs *and*
> a valid role with the same name (but a different OID)?  Then the patch
> would tacitly restore the dangling reference to the latter role.

That is a good point, but I would put the blame on aclitemout: in such
a case it's entirely impossible for pg_dump to distinguish whether an
apparently all-numeric role name in an ACL item is the valid role or a
dangling OID.

I was tempted yesterday to propose a simpler solution in which
we back-patch the putid() fix I showed earlier, and just change
dumputils.c to drop ACLs that have unquoted all-numeric grantees.
(If the grantor part is a dangling OID, we could omit GRANTED BY, as
we did recently for role grants.)  Now the problem with this is that
if you have a case like my "007" example, you're going to lose some
grants if you dump with an updated pg_dump from a not-updated server.
That cure is very likely worse than the disease.

So what I'm thinking today is we apply the putid() fix only in HEAD,
and make dumputils.c ignore unquoted all-numeric roles only if
server version >= 19.  This means we don't have a fix for the actually
known problems with old server versions, which is kind of sad, but
given Laurenz's point I don't think a reliable fix is possible with
an unpatched server.

An alternative answer is to back-patch the putid() fix and teach
dumputils.c to consider the server minor version when deciding whether
to reject unquoted all-numeric roles.  We don't typically make pg_dump
pay attention to minor versions, but it would provide a pathway for
users to deal with this problem: if you've got dangling grants then
update the old server before dumping.

			regards, tom lane

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Greg Sabino Mullane  writes:
>> 5. Verify orphan records remain in pg_init_privs:

> Thanks for providing a failing use case. I ran this on a 18.3 server and
> found no orphaned rows - but I used the pg_stat_statements extension
> instead of pg_wait_sampling. Could you try your experiment using
> pg_stat_statements? And could you also show us the contents of the errant
> rows in pg_init_privs for the failing case?

The orphaned-rows problem shouldn't exist in v17 and later (see
534287403, 35dd40d34, and related commits).  The OP is apparently
complaining about an upgrade from v14, where such rows could exist.

I don't especially care for the proposed fix of making pg_upgrade
refuse to run.  Manually correcting such situations would be tedious
and error-prone.  Plus, it's inconsistent with what we did about
related issues with role GRANTs (see 29d75b25b and 74b4438a7).
I wonder if it'd be sane for pg_dump to just skip dangling role
references in pg_init_privs.

			regards, tom lane

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

=?UTF-8?Q?H=C3=BCseyin_Demir?=  writes:
>> An alternative answer is to back-patch the putid() fix and teach
>> dumputils.c to consider the server minor version when deciding whether
>> to reject unquoted all-numeric roles.  We don't typically make pg_dump
>> pay attention to minor versions, but it would provide a pathway for
>> users to deal with this problem: if you've got dangling grants then
>> update the old server before dumping.

> It will require an almost mandatory minor-version update prior to the
> upgrade, which adds extra friction and a more complex upgrade story
> for our users since both sides need to be updated.

Only if you actually have dangling ACL references, which I think
is the case for a vanishingly small set of users --- otherwise
we'd have heard more complaints and been motivated to fix this
long ago.  Let's not optimize for the broken case.

			regards, tom lane

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Sat, 2026-06-20 at 23:53 +0200, I wrote:
> I think that we need something like this fix, because a failing
> upgrade is a bug.  For the same reason I think that the fix
> should be backpatched.
> 
> I looked at your patch and found that the query you added doesn't
> cover the important case where the grantor is a non-existing role
> 
> I suggest a query like this one: [...]

Further testing shows that changing this query isn't enough.
There are three more places where pg_dump queries pg_init_privs
(in getAggregates, getFuncs and dumpTable).

So we'd have to use a similarly ugly query in all these places,
which doesn't seem particularly attractive and introduces
considerable code duplication.

One approach I can think of is to have a macro SAFE_INITPRIVS
that contains the ugly subquery and is used in all these places.

The other idea is to do some post-processing of the aclitems
found, but they are in string form and would need to get parsed
again, which doesn't look attractive either.

Yours,
Laurenz Albe

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Laurenz Albe  writes:
> Since there have been very few reports of this problem, the question
> remains if we need this patch at all, or of it should be backpatched.
> My opinion is that it should; every upgrade or restore failure is
> one too many.

I have a more pressing concern: has any performance testing been
done on this?  It looks like it'd be absolutely catastrophic for
pg_dump performance on databases with lots of objects.

The implementation direction I'd been vaguely imagining was for
pg_dump's buildACLCommands() to drop any AclItems that contain
dangling role references (ie, numeric OIDs where a role name
should be).  If the given role name contains any non-digit
characters then it's certainly not dangling, so most of the time
this'd be a very cheap check.  However, if somebody does

	CREATE USER "007";
	GRANT ALL ON TABLE mi6_operations TO "007";

we mustn't get fooled by that.  The backend is doing us no favors by
not making numeric OIDs visibly different from all-digit role names
in AclItems.  In HEAD I'd advocate fixing that on the server side
(as attached), but we can't assume that a back-branch server has such
a fix.  What we could do with an old server is issue a query (once per
pg_dump run) to collect all the valid all-digit role names, which
should surely be a short list in most databases, and then filter
against that within buildACLCommands().

			regards, tom lane

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 01caa12eca7..84803351f18 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -235,6 +235,10 @@ putid(char *p, const char *s)
 			break;
 		}
 	}
+	/* If name is all-digits, quote it to distinguish from lookup failure */
+	if (safe && strspn(s, "0123456789") == strlen(s))
+		safe = false;
+
 	if (!safe)
 		*p++ = '"';
 	for (src = s; *src; src++)

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

>
> > Further testing shows that changing this query isn't enough.
> > There are three more places where pg_dump queries pg_init_privs
> > (in getAggregates, getFuncs and dumpTable).
> >
> > So we'd have to use a similarly ugly query in all these places,
> > which doesn't seem particularly attractive and introduces
> > considerable code duplication.
> >
> > One approach I can think of is to have a macro SAFE_INITPRIVS
> > that contains the ugly subquery and is used in all these places.
> >
> > The other idea is to do some post-processing of the aclitems
> > found, but they are in string form and would need to get parsed
> > again, which doesn't look attractive either.
>
>
> v4 applies the filter at all four sites where pg_dump queries pg_init_privs:
> - getAggregates() — WHERE clause comparison
> - getFuncs() — WHERE clause comparison
> - getAdditionalACLs() — SELECT expression (object-level initprivs)
> - PREPQUERY_GETCOLUMNACLS — SELECT expression (column-level initprivs,
> objsubid != 0)
>
> Secondly to avoid duplicating the multi-line subquery at every call
> site, I introduced a SAFE_INITPRIVS(col) macro.

One question from my side: can't we use function instead of macro ?
Would it be more accurate for future readers ?

Wanted to ask your opinion besides the v4 patch.

Regards,
Demir.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

> > > I'd say that your v6 patch is fine if the performance impact can be
> > > shown to be small.
> >
> > To keep the ball rolling, I ran a pg_upgrade --link from a debug-enabled
> > build on a database with 20000 empty tables.
> >
> > pg_upgrade took between 19 and 25 minutes, and I saw no noticeable effect
> > of the patch.  So I guess this is good to go.
> >
> > Yours,
> > Laurenz Albe
>
> Thanks for the pg_upgrade test.
>
> Appreciated.

Rebased v6 on current master.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

> I have a more pressing concern: has any performance testing been
> done on this?  It looks like it'd be absolutely catastrophic for
> pg_dump performance on databases with lots of objects.
>
> The implementation direction I'd been vaguely imagining was for
> pg_dump's buildACLCommands() to drop any AclItems that contain
> dangling role references (ie, numeric OIDs where a role name
> should be).  If the given role name contains any non-digit
> characters then it's certainly not dangling, so most of the time
> this'd be a very cheap check.  However, if somebody does
>
>         CREATE USER "007";
>         GRANT ALL ON TABLE mi6_operations TO "007";
>
> we mustn't get fooled by that.  The backend is doing us no favors by
> not making numeric OIDs visibly different from all-digit role names
> in AclItems.  In HEAD I'd advocate fixing that on the server side
> (as attached), but we can't assume that a back-branch server has such
> a fix.  What we could do with an old server is issue a query (once per
> pg_dump run) to collect all the valid all-digit role names, which
> should surely be a short list in most databases, and then filter
> against that within buildACLCommands().

I see the approach and it's a valid concern. I can create a v5 to
comply with your suggestion and can create a new patch to be applied
to the current HEAD.

After preparing the v5 I'm going to create a new CF to be patched to
HEAD since this fix will be backpatched.

Regards,
Demir.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Hi,

I worked on pg_dump and discussed it with Laurenz Albe. Created the
attached patch.

The fix filters dangling grantees out of each initprivs array at query
time, using NULLIF/ARRAY/NOT EXISTS against pg_authid. Entries for
grantee = 0 (PUBLIC) are never filtered. If all entries for an object
are dangling, NULL is returned and no ACL statement is emitted. Since
we cannot restore grants to non-existent roles. correct outcome,

The patch includes a TAP test (008_pg_dump_dangling_initprivs.pl) that
reproduces the scenario using allow_system_table_mods to create a
dangling pg_init_privs entry, then verifies pg_dump exits cleanly and
emits no invalid GRANT.

I have not prepared backpatch branches yet.

Regards.

Hüseyin Demir , 11 Haz 2026 Per, 07:49
tarihinde şunu yazdı:
>
> > The orphaned-rows problem shouldn't exist in v17 and later (see
> > 534287403, 35dd40d34, and related commits).  The OP is apparently
> > complaining about an upgrade from v14, where such rows could exist.
>
> Yes I was working on upgrading the PostgreSQL version from v14 to v18
> and was able to solve the problem by removing the danling records from
> pg_init_privs.
>
> > I wonder if it'd be sane for pg_dump to just skip dangling role
> > references in pg_init_privs.
>
> It will change the behavior of pg_dump and it's a general purpose tool
> because when we instruct pg_dump to filter orphan records it will
> change the content in the system catalogs.
>
> For now I suppose we have two options: either pg_upgrade or pg_dump.
>
> Regards.
>
>
> Tom Lane , 7 Haz 2026 Paz, 17:52 tarihinde şunu yazdı:
> >
> > Greg Sabino Mullane  writes:
> > >> 5. Verify orphan records remain in pg_init_privs:
> >
> > > Thanks for providing a failing use case. I ran this on a 18.3 server and
> > > found no orphaned rows - but I used the pg_stat_statements extension
> > > instead of pg_wait_sampling. Could you try your experiment using
> > > pg_stat_statements? And could you also show us the contents of the errant
> > > rows in pg_init_privs for the failing case?
> >
> > The orphaned-rows problem shouldn't exist in v17 and later (see
> > 534287403, 35dd40d34, and related commits).  The OP is apparently
> > complaining about an upgrade from v14, where such rows could exist.
> >
> > I don't especially care for the proposed fix of making pg_upgrade
> > refuse to run.  Manually correcting such situations would be tedious
> > and error-prone.  Plus, it's inconsistent with what we did about
> > related issues with role GRANTs (see 29d75b25b and 74b4438a7).
> > I wonder if it'd be sane for pg_dump to just skip dangling role
> > references in pg_init_privs.
> >
> >                         regards, tom lane

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Hi,

You can see the attached v5 patch. Basically, v5 solves the problem by
adding dangling-role filtering directly inside buildACLCommands(). A
role name that consists entirely of digits is potentially a dangling
OID reference and it resolves by querying pg_authid once at the start
of the dump for any legitimate all-digit role names. The sorted list
is passed as new digitRoles/nDigitRoles parameters to
buildACLCommands(), which skips any REVOKE or GRANT item whose grantee
or grantor matches a dangling reference.

I defined a static called is_dangling_role_ref in dumputils.c and
introduced two new parameters to buildACLCommands to eliminate the
dangling grantee or grantor objects.

When it comes to patching the HEAD I'm going to create and submit
another small patch.

Regards,
Demir.

Hüseyin Demir , 23 Haz 2026 Sal, 10:32
tarihinde şunu yazdı:
>
> > I have a more pressing concern: has any performance testing been
> > done on this?  It looks like it'd be absolutely catastrophic for
> > pg_dump performance on databases with lots of objects.
> >
> > The implementation direction I'd been vaguely imagining was for
> > pg_dump's buildACLCommands() to drop any AclItems that contain
> > dangling role references (ie, numeric OIDs where a role name
> > should be).  If the given role name contains any non-digit
> > characters then it's certainly not dangling, so most of the time
> > this'd be a very cheap check.  However, if somebody does
> >
> >         CREATE USER "007";
> >         GRANT ALL ON TABLE mi6_operations TO "007";
> >
> > we mustn't get fooled by that.  The backend is doing us no favors by
> > not making numeric OIDs visibly different from all-digit role names
> > in AclItems.  In HEAD I'd advocate fixing that on the server side
> > (as attached), but we can't assume that a back-branch server has such
> > a fix.  What we could do with an old server is issue a query (once per
> > pg_dump run) to collect all the valid all-digit role names, which
> > should surely be a short list in most databases, and then filter
> > against that within buildACLCommands().
>
> I see the approach and it's a valid concern. I can create a v5 to
> comply with your suggestion and can create a new patch to be applied
> to the current HEAD.
>
> After preparing the v5 I'm going to create a new CF to be patched to
> HEAD since this fix will be backpatched.
>
> Regards,
> Demir.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

> > This v6 patch adds a SAFE_INITPRIVS macro that filters aclitem[]
> > arrays server-side by checking that each entry's grantor and grantee
> > OID still exists in pg_roles. It is applied in exactly two queries:
> >
> > 1. getAdditionalACLs() -- the one-time fetch of pg_init_privs at startup
> > 2. dumpTable() column ACL prepared statement -- per-table column initprivs
>
> No, that's not good.  If you are running the complicated subquery for
> every table dumped, you are re-introducing the performance regression
> from the v4 patch that Tom justly complained about.

The correlated subquery in getAdditionalACLs adds overhead
proportional to pg_init_privs row count × aclitems per row, with each
check being an indexed OID lookup. For a database with 5,000
pg_init_privs rows this is probably a few extra milliseconds. I added
performance test results with v6. I think it's a sustainable operation
but If I'm missing sth. let me know.

> On the other hand, I agree with you that Tom's idea to make this fix
> depend on a minor update of the source server that fixes the string
> representation of aclitems is not so great.  Few people undergo the
> hassle of applying the latest minor update to a server they are about
> to update (and I am not even speaking about the users who keep running
> on the 14.3 they went into production with).  Yes, the problem that
> the present patch is trying to address is a rare one, and we should
> keep the maintenance and performance burden incurred moderate.
> But what good is a fix that won't work for a good percentage of the
> affected cases, even if they are few?
>
> Here is my latest idea (hold your noses):
> Instead of having pg_dump query "FROM pg_catalog.pg_init_privs pip",
> how about writing "(FROM (VALUES (...), (...), ...) AS pip", where the
> VALUES clause is composed from a query against pg_init_privs run once
> at the beginning of pg_dump that excludes the bad entries?
> Critizism I forsee is that a) this is ugly and b) very long VALUES
> statements might also constitute a performance regression.
> However, I have yet to see an extension that produces a hundred
> initial privilege entries.

I'm not against this approach but the tradeoff is the same with v6 I
suppose. Since the overhead of processing the full VALUES clause on
every EXECUTE is unnecessary. v6 goes directly to pg_init_privs
indexed by objoid = $1.

But definitely we can prepare a new patch to cover these expectations.

With that in mind, I would like to propose splitting the work into two
separate patches:

1. pg_dump fix (backpatch to 14): the v6 SQL OID-level filter. Works
on any server >= 9.6 without requiring a source-side update. Fixes the
known breakage for users upgrading from pre-17 clusters that
accumulated dangling entries via DROP ROLE.

2. putid() fix (HEAD only): improve aclitemout so that a dangling OID
is rendered in a form that is unambiguously distinct from a valid role
name.  This is the right long-term fix for the text representation and
would benefit any future tool that processes aclitem output.  It is
independent of the pg_dump fix and does not need to be backpatched for
patch 1 to be correct.

New dangling entries will not live on modern servers.  However, users
upgrading from pre-17 clusters may have years of accumulated dangling
entries that pg_upgrade will carry forward silently.  The pg_dump fix
is a one-time for that migration window.
Does this split make sense to the reviewers?  If so, I will prepare v7
of the pg_dump patch (incorporating any further feedback on v6) and a
separate patch for the putid() fix in HEAD.

Regards,
Demir.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Hi,

> So what I'm thinking today is we apply the putid() fix only in HEAD,
> and make dumputils.c ignore unquoted all-numeric roles only if
> server version >= 19.  This means we don't have a fix for the actually
> known problems with old server versions, which is kind of sad, but
> given Laurenz's point I don't think a reliable fix is possible with
> an unpatched server.
> An alternative answer is to back-patch the putid() fix and teach
> dumputils.c to consider the server minor version when deciding whether
> to reject unquoted all-numeric roles.  We don't typically make pg_dump
> pay attention to minor versions, but it would provide a pathway for
> users to deal with this problem: if you've got dangling grants then
> update the old server before dumping.

It will require an almost mandatory minor-version update prior to the
upgrade, which adds extra friction and a more complex upgrade story
for our users since both sides need to be updated.
Another point is that quoting all-digit roles alters the aclitem::text
representation across psql, extensions, and client apps giving it a
much broader behavioral impact than just pg_dump.

Finally, it introduces a bit of a maintenance headache for committers,
who will have to adjust the macro on HEAD after back-patching to v18.

Happy to hear other ideas.

Regards,
Demir.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Hi,

> Further testing shows that changing this query isn't enough.
> There are three more places where pg_dump queries pg_init_privs
> (in getAggregates, getFuncs and dumpTable).
>
> So we'd have to use a similarly ugly query in all these places,
> which doesn't seem particularly attractive and introduces
> considerable code duplication.
>
> One approach I can think of is to have a macro SAFE_INITPRIVS
> that contains the ugly subquery and is used in all these places.
>
> The other idea is to do some post-processing of the aclitems
> found, but they are in string form and would need to get parsed
> again, which doesn't look attractive either.

Thanks for the feedback.
I created a simple v4 patch to cover your feedback

v4 applies the filter at all four sites where pg_dump queries pg_init_privs:
- getAggregates() — WHERE clause comparison
- getFuncs() — WHERE clause comparison
- getAdditionalACLs() — SELECT expression (object-level initprivs)
- PREPQUERY_GETCOLUMNACLS — SELECT expression (column-level initprivs,
objsubid != 0)

Secondly to avoid duplicating the multi-line subquery at every call
site, I introduced a SAFE_INITPRIVS(col) macro.

I tried to add more clean and detailed comments for future commits but
please let me know if you have additional feedback on it.

Regards,
Demir.

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

>
> While the technique of fetching the all-numeric role names in advance
> is certainly much cheaper than running a complicated subquery for
> every object dumped, I have one remaining doubt:
>
> What if there is a dangling role OID 65432 in pg_init_privs *and*
> a valid role with the same name (but a different OID)?  Then the patch
> would tacitly restore the dangling reference to the latter role.
> Apart from the result being wrong, I wonder if that could be used for
> a privilege escalation attack: you detect that there are dangling
> pg_init_privs entries that grant high privileges.  Then you abuse your
> CREATEROLE to create a role with the same name as the dangling OID.
> After a dump and restore, your role has been assigned those privileges.
>
> Perhaps it would be a better approach to fetch the data from
> pg_init_privs once at the beginning of the dump, ignoring the entries
> with dangling OIDs?
>
> Yours,
> Laurenz Albe

Thank you for the reviews.

This v6 patch adds a SAFE_INITPRIVS macro that filters aclitem[]
arrays server-side by checking that each entry's grantor and grantee
OID still exists in pg_roles. It is applied in exactly two queries:

1. getAdditionalACLs() -- the one-time fetch of pg_init_privs at startup
2. dumpTable() column ACL prepared statement -- per-table column initprivs

Crucially, the WHERE clauses in getAggregates()/getFuncs() are NOT
modified. Those queries use raw pip.initprivs only for object
selection (not output), and any spuriously-selected objects produce
zero output because the stored initprivs (from getAdditionalACLs) is
already filtered.

pg_roles is used instead of pg_authid to support non-superuser pg_dump.

So this patch covers the security and performance concerns together I
believe. In addition to this, I tried to introduce different tests to
verify it.

When it comes to performance results I tried to draw a conclusion.
Performance testing shows the overhead is ~1-2ms on the one-time
pg_init_privs fetch (249-749 rows) and zero measurable impact on a
database with 10,000 functions + 500 aggregates.

The filtering adds ~1ms (249 rows) to ~2ms (749 rows) to the one-time
getAdditionalACLs() query that runs at pg_dump startup. This is a
fixed cost that does NOT scale with the number of functions,
aggregates, or other objects in the database.

Let me know if you have additional feedback and concerns.

Regards,
Demir

Re: Improve CREATE/ALTER PUBLICATION syntax for EXCEPT

On Thu, Jul 23, 2026 at 10:01 AM Peter Smith  wrote:
> To summarise.
> - The patch-suggested list flattening for `except_table_list` looks
> OK, but the status-quo also looks OK to me and is consistent with
> other terms
> - I think removing these double lists now may need to be reverted
> sometime in the future

Fair enough. I may be the only one who prefers the proposed synopsis,
and the current form is probably better suited to future extensions,
so I've dropped that part for now.

The attached patch removes the synopsis changes and keeps
the other documentation improvements.

Regards,

-- 
Fujii Masao

Improve CREATE/ALTER PUBLICATION syntax for EXCEPT

Hi,

Attached is a small docs patch to improve the syntax and description of
the EXCEPT clause for CREATE/ALTER PUBLICATION.

The current synopsis for EXCEPT is a bit confusing because both the EXCEPT
clause and except_table_object contain repeated lists:

    ALL TABLES [ EXCEPT ( except_table_object [, ... ] ) ]
    where except_table_object is:
        TABLE table_object [, ... ]

The patch simplifies this by describing the syntax directly:

    ALL TABLES [ EXCEPT ( TABLE table_object [, [ TABLE ] table_object ] ... ) ]

and removes the separate except_table_object production. I think
this is simpler and easier to understand. Thought?

The patch also updates the ALTER PUBLICATION description. The current
wording says "adding tables/except tables/schemas", but EXCEPT entries
are not added with ADD; they are replaced or cleared with SET ALL TABLES.
The updated text reflects that and also notes that
ALTER SUBSCRIPTION ... REFRESH PUBLICATION is required for such changes
to take effect on subscribers.

It also includes a few minor wording and markup fixes nearby.

Regards,

-- 
Fujii Masao

Re: Improve CREATE/ALTER PUBLICATION syntax for EXCEPT

On Wed, Jul 22, 2026 at 1:43 PM Fujii Masao  wrote:
>
> Hi,
>
> Attached is a small docs patch to improve the syntax and description of
> the EXCEPT clause for CREATE/ALTER PUBLICATION.
>
> The current synopsis for EXCEPT is a bit confusing because both the EXCEPT
> clause and except_table_object contain repeated lists:
>
>     ALL TABLES [ EXCEPT ( except_table_object [, ... ] ) ]
>     where except_table_object is:
>         TABLE table_object [, ... ]
>
> The patch simplifies this by describing the syntax directly:
>
>     ALL TABLES [ EXCEPT ( TABLE table_object [, [ TABLE ] table_object ] ... ) ]
>
> and removes the separate except_table_object production. I think
> this is simpler and easier to understand. Thought?
>
> The patch also updates the ALTER PUBLICATION description. The current
> wording says "adding tables/except tables/schemas", but EXCEPT entries
> are not added with ADD; they are replaced or cleared with SET ALL TABLES.
> The updated text reflects that and also notes that
> ALTER SUBSCRIPTION ... REFRESH PUBLICATION is required for such changes
> to take effect on subscribers.
>
> It also includes a few minor wording and markup fixes nearby.
>

+1 for all the description and markup changes.

But for the synopsis to change, I am not so sure...

~~~

Some might say,

ALL TABLES [ EXCEPT ( TABLE table_object [, [ TABLE ] table_object ] ... ) ]

is simpler than:

ALL TABLES [ EXCEPT ( except_table_object [, ... ] ) ]

where except_table_object is:
    TABLE table_object [, ... ]

~~~

But, soon [1] there will also be ALL SCHEMAS EXCEPT which can re-use
that same `table_object`

Is the expanded style,

ALL TABLES [ EXCEPT ( TABLE table_object [, [ TABLE ] table_object ] ... ) ]
TABLES IN SCHEMA [ EXCEPT ( TABLE table_object [, [ TABLE ]
table_object ] ... ) ]

still better than:

ALL TABLES [ EXCEPT ( except_table_object [, ... ] ) ]
TABLES IN SCHEMA [ EXCEPT ( except_table_object [, ... ] ) ]

where except_table_object is:
    TABLE table_object [, ... ]

~~~

And, consider later when entire schemas may be excluded from FOR ALL TABLES

It will be easy to write in the synopsis as:

ALL TABLES [ EXCEPT ( except_table_object | except_schema_object [, ... ] ) ]

where except_table_object is:
    TABLE table_object [, ... ]
where except_schema_object is:
    SCHEMA schema_name [, ... ]

OTOH, it would be a big mess if you tried to expand that out without
having `except_table_object` and `except_schema_object`.

~~~

So, the suggested synopsis change might seem good today, but in a
couple of years IMO we'd probably want to change it back again.

======
[1] https://www.postgresql.org/message-id/flat/CABdArM5sw4Q1ZU8HGdo4BSc1A_%2B8xtUNq17j6wcir%3DyMUy19Cg%40mail.gmail.com

Kind Regards,
Peter Smith
Fujitsu Australia

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

I sat down to write the two PUBLIC cases I suggested, and hit something bigger
on the way. As it stands the test does not distinguish the fixed pg_dump from
the broken one.

I took v7, reverted just the two SAFE_INITPRIVS call sites in pg_dump.c, left
the rest in place, rebuilt, and re-ran the test as posted -- with nothing added
but the quote_ident() from my last mail, so that it runs at all:

    t/008_pg_dump_dangling_initprivs.pl .. ok
    All tests successful.
    Files=1, Tests=12

The objects that carry the dangling entries all keep their default ACL, so
pg_dump emits no GRANT/REVOKE for them at all, patched or not, and the
assertions that matter are all unlike() -- they hold vacuously.
--binary-upgrade doesn't change that.

One added line per case is enough to make it bite: give the object a non-default
ACL. For case 1,

    CREATE FUNCTION public.test_func_grantee() RETURNS int LANGUAGE
sql AS 'SELECT 1';
    REVOKE ALL ON FUNCTION public.test_func_grantee() FROM PUBLIC;

and case 1 then fails on the reverted build and passes on v7. The dump from the
reverted build is the reported breakage:

    REVOKE ALL ON FUNCTION public.test_func_grantee() FROM "16385";
    GRANT ALL ON FUNCTION public.test_func_grantee() TO "rui.zhao";

against v7's:

    REVOKE ALL ON FUNCTION public.test_func_grantee() FROM PUBLIC;

That also shows the catch-all assertion is too narrow: it looks for
GRANT ... TO "digits", but the bare OID here lands in a
REVOKE ... FROM "digits".

Built the same way, the two PUBLIC cases from my last mail do bite as well: a
PUBLIC entry with a dangling grantor fails on the reverted build, and a valid
PUBLIC entry stops being dumped if the "ace.grantee <> 0" guard is removed.

Attached is a test-only patch on top of v7 that does all of the above:
quote_ident() on the four literals, a non-default ACL for the objects in cases
1 to 3, the widened catch-all, and the two PUBLIC cases. With that patch in
place the same three builds give:

    v7's pg_dump as posted            14/14 pass
    without the SAFE_INITPRIVS calls  cases 1, 2, 3, 6 and the catch-all fail
    without "ace.grantee <> 0"        case 7 fails

One I left alone: case 4, the spurious-REVOKE one. Its premise is that proacl is
NULL, and that is exactly what stops pg_dump emitting anything for the object,
so I could not give it a non-default ACL the way I did for the others without
turning it into case 1. I did try to reproduce the spurious REVOKE with proacl
NULL a few other ways -- on a plain function, on a pinned catalog function, and
on a real extension member, plain and with --binary-upgrade -- and got no ACL
output at all in any of them. Where a bare OID does turn up in a REVOKE is when
the object has a non-default ACL, which cases 1 to 3 now cover. So the
precondition in that part of the commit message may not be quite right; you'd
know better whether there's a path I'm missing.

Thanks,
Rui

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Hi Hüseyin,

I reviewed and tested v6. The filtering logic is correct and applied in the
right place: doing it in the source queries (getAdditionalACLs and the
column-level ACL query) means the dangling entries never reach the
binary-upgrade "SET SESSION AUTHORIZATION " path, which is where the
upgrade actually broke. Using pg_roles instead of pg_authid for the
existence check is also right for non-superuser pg_dump. With the test
running, all of its assertions pass.

Two things on the test:

1. The TAP test doesn't run for me at all -- it dies in setup with
   "role \"rui\" does not exist". The cause is that the aclitem literals
   are built by concatenating current_user unquoted, e.g.

       ARRAY[('ghost_grantee=X/' || current_user)::aclitem]

   My bootstrap superuser is "rui.zhao", so this becomes
   'ghost_grantee=X/rui.zhao', and aclitemin parses the grantor only up to
   the dot:

       =# SELECT ('g=X/' || 'a.b')::aclitem;
       ERROR:  role "a" does not exist

   So the test fails before any assertion runs on any cluster whose
   superuser name needs quoting (a dot, uppercase, etc.). Wrapping it as
   quote_ident(current_user) in the four aclitem literals fixes it (the
   test then passes 12/12 here). A bit ironic given the patch is about
   handling odd role names.

2. The PUBLIC case (grantee = 0) isn't covered. The "ace.grantee <> 0"
   branch is what keeps PUBLIC grants from being filtered, but there's no
   test for either direction: a valid PUBLIC grant ("=r/validgrantor")
   being kept, or a PUBLIC grant whose grantor is dangling ("=r/ghost")
   being dropped. Worth a case or two.

Thanks,
Rui

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Fri, 2026-06-19 at 09:40 +0200, Hüseyin Demir wrote:
> Found a problem and fixed it quickly.

I think that we need something like this fix, because a failing
upgrade is a bug.  For the same reason I think that the fix
should be backpatched.

I looked at your patch and found that the query you added doesn't
cover the important case where the grantor is a non-existing role
(that is, the original extension owner was dropped).
The grantor appears in SET SESSION AUTHORIZATION commands in the
dump, which make the restore and consequently the upgrade fail.

I suggest a query like this one:

SELECT pip.objoid, pip.classoid, pip.objsubid, pip.privtype,
  NULLIF(
    ARRAY(
      SELECT elt FROM pg_catalog.unnest(pip.initprivs) AS elt
      /* that is valid, that is, there is not ... */
      WHERE NOT EXISTS (
        /* ... a non-existing grantor ... */
        SELECT 1 FROM pg_catalog.aclexplode(ARRAY[elt]) ace
        WHERE NOT EXISTS (
          SELECT 1 FROM pg_catalog.pg_roles AS r1
          WHERE r1.oid = ace.grantor
        )
        /* ... or a non-existing grantee that isn't 0 */
        OR ace.grantee <> 0
          AND NOT EXISTS (
            SELECT 1 FROM pg_catalog.pg_roles AS r2
            WHERE r2.oid = ace.grantee
          )
      )
    ),
    ARRAY[]::pg_catalog.aclitem[]
  ) AS initprivs
FROM pg_catalog.pg_init_privs pip;

You see that I added some comments, because the query is almost
incomprehensible.  I couldn't think of a more elegant solution.

I think that you also should add an extensive code comment that
explains why this hack is needed.

I am undecided if the regression test with the artificially created
broken initial privileges is a good idea or not.  After all, we are
not testing the real thing here (for example, the test didn't catch
the omission described above).

I am attaching a test extension that I installed in a v14 database
to test your patch; perhaps you'll find it useful.  It creates all
kinds of objects that have an ACL.  Dropping the role that created
the extension leaves various junk entries in pg_init_privs that you
can use to test your patch.

Yours,
Laurenz Albe

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Wed, 2026-06-24 at 14:19 +0200, Hüseyin Demir wrote:
> 
> This v6 patch adds a SAFE_INITPRIVS macro that filters aclitem[]
> arrays server-side by checking that each entry's grantor and grantee
> OID still exists in pg_roles. It is applied in exactly two queries:
> 
> 1. getAdditionalACLs() -- the one-time fetch of pg_init_privs at startup
> 2. dumpTable() column ACL prepared statement -- per-table column initprivs

No, that's not good.  If you are running the complicated subquery for
every table dumped, you are re-introducing the performance regression
from the v4 patch that Tom justly complained about.

On the other hand, I agree with you that Tom's idea to make this fix
depend on a minor update of the source server that fixes the string
representation of aclitems is not so great.  Few people undergo the
hassle of applying the latest minor update to a server they are about
to update (and I am not even speaking about the users who keep running
on the 14.3 they went into production with).  Yes, the problem that
the present patch is trying to address is a rare one, and we should
keep the maintenance and performance burden incurred moderate.
But what good is a fix that won't work for a good percentage of the
affected cases, even if they are few?

Here is my latest idea (hold your noses):
Instead of having pg_dump query "FROM pg_catalog.pg_init_privs pip",
how about writing "(FROM (VALUES (...), (...), ...) AS pip", where the
VALUES clause is composed from a query against pg_init_privs run once
at the beginning of pg_dump that excludes the bad entries?
Critizism I forsee is that a) this is ugly and b) very long VALUES
statements might also constitute a performance regression.
However, I have yet to see an extension that produces a hundred
initial privilege entries.

Yours,
Laurenz Albe

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Fri, 2026-06-26 at 08:56 +0200, Hüseyin Demir wrote:
> > > This v6 patch adds a SAFE_INITPRIVS macro that filters aclitem[]
> > > arrays server-side by checking that each entry's grantor and grantee
> > > OID still exists in pg_roles. It is applied in exactly two queries:
> > > 
> > > 1. getAdditionalACLs() -- the one-time fetch of pg_init_privs at startup
> > > 2. dumpTable() column ACL prepared statement -- per-table column initprivs
> > 
> > No, that's not good.  If you are running the complicated subquery for
> > every table dumped, you are re-introducing the performance regression
> > from the v4 patch that Tom justly complained about.
> 
> The correlated subquery in getAdditionalACLs adds overhead
> proportional to pg_init_privs row count × aclitems per row, with each
> check being an indexed OID lookup. For a database with 5,000
> pg_init_privs rows this is probably a few extra milliseconds. I added
> performance test results with v6. I think it's a sustainable operation
> but If I'm missing sth. let me know.

I have no problem with the subquery being in getAdditionalACLs().
But dumpTable() is executed once per table that gets dumped, right?
So the complicated subquery will run once per table.

> > Here is my latest idea (hold your noses):
> > Instead of having pg_dump query "FROM pg_catalog.pg_init_privs pip",
> > how about writing "(FROM (VALUES (...), (...), ...) AS pip", where the
> > VALUES clause is composed from a query against pg_init_privs run once
> > at the beginning of pg_dump that excludes the bad entries?
> > Critizism I forsee is that a) this is ugly and b) very long VALUES
> > statements might also constitute a performance regression.
> > However, I have yet to see an extension that produces a hundred
> > initial privilege entries.
> 
> I'm not against this approach but the tradeoff is the same with v6 I
> suppose. Since the overhead of processing the full VALUES clause on
> every EXECUTE is unnecessary. v6 goes directly to pg_init_privs
> indexed by objoid = $1.

Ah, so you are saying that the effort per table will be much less.

Could you measure the difference with a database with - say - 10000 tables?

> But definitely we can prepare a new patch to cover these expectations.

Let's agree on the proper approach before you go to the effort of writing
another patch.

> With that in mind, I would like to propose splitting the work into two
> separate patches:
> 
> 1. pg_dump fix (backpatch to 14): the v6 SQL OID-level filter. Works
> on any server >= 9.6 without requiring a source-side update. Fixes the
> known breakage for users upgrading from pre-17 clusters that
> accumulated dangling entries via DROP ROLE.
> 
> 2. putid() fix (HEAD only): improve aclitemout so that a dangling OID
> is rendered in a form that is unambiguously distinct from a valid role
> name.  This is the right long-term fix for the text representation and
> would benefit any future tool that processes aclitem output.  It is
> independent of the pg_dump fix and does not need to be backpatched for
> patch 1 to be correct.

I think that we all agree on that.

> New dangling entries will not live on modern servers.  However, users
> upgrading from pre-17 clusters may have years of accumulated dangling
> entries that pg_upgrade will carry forward silently.

... or rather fail doing so ...

>                                                       The pg_dump fix
> is a one-time for that migration window.

Right.

> Does this split make sense to the reviewers?  If so, I will prepare v7
> of the pg_dump patch (incorporating any further feedback on v6) and a
> separate patch for the putid() fix in HEAD.

It makes sense to me.

I'd say that your v6 patch is fine if the performance impact can be
shown to be small.

Yours,
Laurenz Albe

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Fri, 2026-06-26 at 10:30 +0200, Laurenz Albe wrote:
> I'd say that your v6 patch is fine if the performance impact can be
> shown to be small.

To keep the ball rolling, I ran a pg_upgrade --link from a debug-enabled
build on a database with 20000 empty tables.

pg_upgrade took between 19 and 25 minutes, and I saw no noticeable effect
of the patch.  So I guess this is good to go.

Yours,
Laurenz Albe

Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

On Wed, 2026-06-24 at 08:14 +0200, Hüseyin Demir wrote:
> You can see the attached v5 patch. Basically, v5 solves the problem by
> adding dangling-role filtering directly inside buildACLCommands(). A
> role name that consists entirely of digits is potentially a dangling
> OID reference and it resolves by querying pg_authid once at the start
> of the dump for any legitimate all-digit role names.

The patch looks good and passes my tests.

It fails the regression tests on my system with

  pg_dump: error: query failed: ERROR:  permission denied for table pg_authid

I think you should use pg_roles rather than pg_authid, so that it
remains possible to use pg_dump with a non-superuser.

While the technique of fetching the all-numeric role names in advance
is certainly much cheaper than running a complicated subquery for
every object dumped, I have one remaining doubt:

What if there is a dangling role OID 65432 in pg_init_privs *and*
a valid role with the same name (but a different OID)?  Then the patch
would tacitly restore the dangling reference to the latter role.
Apart from the result being wrong, I wonder if that could be used for
a privilege escalation attack: you detect that there are dangling
pg_init_privs entries that grant high privileges.  Then you abuse your
CREATEROLE to create a role with the same name as the dangling OID.
After a dump and restore, your role has been assigned those privileges.

Perhaps it would be a better approach to fetch the data from
pg_init_privs once at the beginning of the dump, ignoring the entries
with dangling OIDs?

Yours,
Laurenz Albe