public inbox for [email protected]
help / color / mirror / Atom feedFrom: Rui Zhao <[email protected]>
To: Hüseyin Demir <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Laurenz Albe <[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: Thu, 25 Jun 2026 18:44:38 +0800
Message-ID: <CAHWVJhHB+tunzZLJcbERKb2G+t-uw53MP3znU57STVa+sZdQ2Q@mail.gmail.com> (raw)
In-Reply-To: <CAB5wL7Zmy9PZ_TeEyzN0__vm2S4FUQPTqZZsT1rbnWa7WUGKLA@mail.gmail.com>
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]>
<[email protected]>
<CAB5wL7Zmy9PZ_TeEyzN0__vm2S4FUQPTqZZsT1rbnWa7WUGKLA@mail.gmail.com>
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 <oid>" 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
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], [email protected]
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table
In-Reply-To: <CAHWVJhHB+tunzZLJcbERKb2G+t-uw53MP3znU57STVa+sZdQ2Q@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