public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hüseyin Demir <[email protected]>
To: Laurenz Albe <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Greg Sabino Mullane <[email protected]>
Cc: [email protected]
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table
Date: Fri, 26 Jun 2026 08:56:00 +0200
Message-ID: <CAB5wL7YQs7CDduxx0cv3iKPnQ7fxT-mj0AqPWv2WPO3aaF_VeQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAKAnmmLYXcBSV8i5MVuiwu8tkX-JDGmZtJ38_YbdUTwoDW7xKg@mail.gmail.com>
<[email protected]>
<CAB5wL7bo-7okBCJ24fdVn4UhnuPLPG22eQjaLJZ9GJtv0FEWMg@mail.gmail.com>
<CAB5wL7bunPCi93Bwi7zN_1gFAZhKLwS=Fsub91SW417xujGtNA@mail.gmail.com>
<CAB5wL7aKGFFEF1SM54GxvaQPH=7Bs=HuUCqi7F4Ve025+Z950A@mail.gmail.com>
<CAB5wL7aK6UaukMSegzEG68y0VrKOg7wFnPNRaDaHreBf2iVWag@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAB5wL7ZuFwu2Sd-xmn-w5-BbrKk_Mu_1ubYow_qaqFGhJOiMXw@mail.gmail.com>
<CAB5wL7ai8AiaFV6B8J=w6vK1Q6Z9Hmp_y3PFL2Zzpx6Y8xPZ2A@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAB5wL7btFp-XRBfv_J-YperoK+e0uYje3ZYsfTzCuHcUWWrvbw@mail.gmail.com>
<CAB5wL7YFeL0VP7EZu6wtWx=UhXuDp3dTBVJwFvSRzBi7qNscfQ@mail.gmail.com>
<[email protected]>
<CAB5wL7Zy89DeVXHFvTL72KChkq=2jLXhH14zrC2n5sutLTqZDg@mail.gmail.com>
<[email protected]>
> > 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.
view thread (27+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table
In-Reply-To: <CAB5wL7YQs7CDduxx0cv3iKPnQ7fxT-mj0AqPWv2WPO3aaF_VeQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox