On Fri, Aug 22, 2025 at 10:20:19PM -0700, Noah Misch wrote: > On Mon, Aug 11, 2025 at 12:20:09AM +0500, Kirill Reshke wrote: > > On Sun, 10 Aug 2025 at 21:37, Noah Misch <n...@leadboat.com> wrote: > > > Thanks. Could you make src/test/regress create regression database > > > objects so > > > the code addition has coverage? Using pg_signal_backend and > > > pg_read_all_settings as the default ACL role names should avoid that > > > suite's > > > limitations. (The suite must run under any role name and must drop any > > > roles > > > it creates, so it can't assume any particular non-system role name > > > survives > > > the suite.) > > > > Here is my attempt at implementing necessary legwork. It's v3 because > > I accidentally cleared the CC list in my previous attempt. Noah kindly > > explained to me how additions to the regress test will cause pg_dump > > logic to be tested as well. > > TIL 002_pg_upgarde.pl runs a regression suite, so if we create any > > database objects in it, it will end up being dumped and restored in > > that test. > > So, I checked that without changes in pg_dump_sort.c, 002_pg_upgarde > > fails and with changes it does not. > > Great. > > > PFA. I am not horribly sure about my additions to the > > `src/test/regress/sql/privileges.sql` file, maybe appending SQL to the > > end of the file is not the best option and there is a better place. > > I like how src/test/regress/sql/collate.icu.utf8.sql puts that kind of thing > just after cleanup, so I put it there. Pushed as b61a5c4 with a few other > cosmetic changes. Thanks.
TestUpgradeXversion fails: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=copperhead&dt=2025-08-23%2007%3A34%3A38 --- /home/pgbf/buildroot/upgrade.copperhead/REL_18_STABLE/origin-REL_16_STABLE.sql.fixed 2025-08-23 10:28:16.464887433 +0200 +++ /home/pgbf/buildroot/upgrade.copperhead/REL_18_STABLE/converted-REL_16_STABLE-to-REL_18_STABLE.sql.fixed 2025-08-23 10:28:16.508887289 +0200 @@ -606490,13 +606490,13 @@ -- -- Name: DEFAULT PRIVILEGES FOR TABLES; Type: DEFAULT ACL; Schema: -; Owner: pg_read_all_settings -- -ALTER DEFAULT PRIVILEGES FOR ROLE pg_read_all_settings REVOKE SELECT,INSERT,REFERENCES,DELETE,TRIGGER,TRUNCATE,UPDATE ON TABLES FROM pg_read_all_settings; -ALTER DEFAULT PRIVILEGES FOR ROLE pg_read_all_settings GRANT SELECT,REFERENCES,DELETE,TRIGGER,TRUNCATE,UPDATE ON TABLES TO pg_read_all_settings; +ALTER DEFAULT PRIVILEGES FOR ROLE pg_read_all_settings REVOKE ALL ON TABLES FROM pg_read_all_settings; +ALTER DEFAULT PRIVILEGES FOR ROLE pg_read_all_settings GRANT SELECT,REFERENCES,DELETE,TRIGGER,TRUNCATE,MAINTAIN,UPDATE ON TABLES TO pg_read_all_settings; Crossing the boundary of the MAINTAIN privilege existing seems relevant. Will fix. (My checklist did tell me to do a local run of TestUpgradeXversion. I skipped it, betting this patch wouldn't break that test. I lost that bet.)
