MIME-Version: 1.0
References: <19483-80de42dc4e62cfd6@postgresql.org>
 <CAKAnmmLYXcBSV8i5MVuiwu8tkX-JDGmZtJ38_YbdUTwoDW7xKg@mail.gmail.com>
 <104812.1780847519@sss.pgh.pa.us>
 <CAB5wL7bo-7okBCJ24fdVn4UhnuPLPG22eQjaLJZ9GJtv0FEWMg@mail.gmail.com>
 <CAB5wL7bunPCi93Bwi7zN_1gFAZhKLwS=Fsub91SW417xujGtNA@mail.gmail.com>
 <CAB5wL7aKGFFEF1SM54GxvaQPH=7Bs=HuUCqi7F4Ve025+Z950A@mail.gmail.com>
 <CAB5wL7aK6UaukMSegzEG68y0VrKOg7wFnPNRaDaHreBf2iVWag@mail.gmail.com>
 <d305af372949be0c2c4155168ca7ba97e160bc52.camel@cybertec.at>
 <5184088806e70ed5db7e83bd6540a016b3802c1d.camel@cybertec.at>
 <CAB5wL7ZuFwu2Sd-xmn-w5-BbrKk_Mu_1ubYow_qaqFGhJOiMXw@mail.gmail.com>
 <CAB5wL7ai8AiaFV6B8J=w6vK1Q6Z9Hmp_y3PFL2Zzpx6Y8xPZ2A@mail.gmail.com>
 <0d7531c192859057c6aad54c89a8d453376bd00c.camel@cybertec.at>
 <592839.1782153054@sss.pgh.pa.us>
 <CAB5wL7btFp-XRBfv_J-YperoK+e0uYje3ZYsfTzCuHcUWWrvbw@mail.gmail.com>
 <CAB5wL7YFeL0VP7EZu6wtWx=UhXuDp3dTBVJwFvSRzBi7qNscfQ@mail.gmail.com>
 <f55bf7dc0a503f82abfb6f02ac91278fe59d40d9.camel@cybertec.at>
 <1030965.1782313078@sss.pgh.pa.us>
 <CAB5wL7Zmy9PZ_TeEyzN0__vm2S4FUQPTqZZsT1rbnWa7WUGKLA@mail.gmail.com>
In-Reply-To: 
 <CAB5wL7Zmy9PZ_TeEyzN0__vm2S4FUQPTqZZsT1rbnWa7WUGKLA@mail.gmail.com>
From: Rui Zhao <zhaorui126@gmail.com>
Date: Thu, 25 Jun 2026 18:44:38 +0800
Message-ID: 
 <CAHWVJhHB+tunzZLJcbERKb2G+t-uw53MP3znU57STVa+sZdQ2Q@mail.gmail.com>
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv
 catalog table
To: =?UTF-8?Q?H=C3=BCseyin_Demir?= <huseyin.d3r@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Laurenz Albe <laurenz.albe@cybertec.at>,
	Greg Sabino Mullane <htamfids@gmail.com>, pgsql-bugs@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAHWVJhHB%2BtunzZLJcbERKb2G%2Bt-uw53MP3znU57STVa%2BsZdQ2Q%40mail.gmail.com>
Precedence: bulk

Hi H=C3=BCseyin,

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=3DX/' || current_user)::aclitem]

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

       =3D# SELECT ('g=3DX/' || '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 =3D 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 ("=3Dr/validgrantor")
   being kept, or a PUBLIC grant whose grantor is dangling ("=3Dr/ghost")
   being dropped. Worth a case or two.

Thanks,
Rui