Re: pg_restore --no-policies should not restore policies' comment
Fujii Masao <masao.fujii@gmail.com>
From: Fujii Masao <masao.fujii@gmail.com>
To: jian he <jian.universality@gmail.com>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2025-08-26T11:43:45Z
Lists: pgsql-hackers
Attachments
- v3-0001-pg_restore-Fix-handling-of-comments-on-policies-p.patch (application/octet-stream) patch v3-0001
On Thu, Jul 3, 2025 at 11:32 PM jian he <jian.universality@gmail.com> wrote: > > On Wed, Jul 2, 2025 at 5:18 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > > > hi. > > > > > > I’ve tested the pg_restore options --no-policies, --no-publications, and > > > --no-subscriptions locally. > > > > Thanks for updating the patch! Could you add it to the next CommitFest > > so we don't forget about it? > > > sure. > > > > > > However, I haven’t tested --no-security-labels option, so no changes were > > > made for it. Testing --no-security-labels appears to need more setup, which > > > didn’t seem trivial. > > > > You're checking whether pg_restore --no-publications --no-subscriptions correctly > > skips security labels for publications and subscriptions, and if not, > > you'll prepare a patch. Right? I'm not sure how common it is to define security > > labels on publications or subscriptions, but if the behavior is unexpected (i.e., > > the security labels are not skipped in that case), it's worth fixing. > > It would probably be better to handle that in a separate patch from the one > > for comments. > > > > To set up a security label for testing, you can use the > > src/test/modules/dummy_seclabel module. > > > > Thanks! > Using dummy_seclabel helped me test the pg_restore --no-security-labels > behavior. All the attached patches have now been tested locally. I’ll need help > writing the Perl tests.... > > I found out another problem while playing with pg_restore --no-security-labels: > pg_dump does not dump security labels on global objects like > subscriptions or roles. > > summary of attached patch: Thanks for the patches! > 01: make pg_restore not restore comments if comments associated > objects are excluded. > TODO: need perl tests How about adding tests for pg_restore --no-policies in 002_pg_dump.pl, as in the attached patch? Since --no-publications and --no-subscriptions have been around for a long time, while --no-policies was added in v18, I wonder if it makes sense to first fix the publications and subscriptions cases (and add tests for them) and back-patch to all supported versions. Then we can handle the policies case and back-patch it only to v18. Does that sound reasonable? > 02: make pg_dump dump security label for shared database objects, like > subscription, roles. > 03: make pg_restore not restore security labels if the associated > object is excluded. Will review them later. Regards, -- Fujii Masao
Commits
-
pg_dump: Remove unnecessary code for security labels on extensions.
- 762faf702c6f 19 (unreleased) landed
-
pg_restore: Fix security label handling with --no-publications/subscriptions.
- a4dbb11bb9ac 13.23 landed
- bc476f8b8b64 14.20 landed
- 5f42008f9030 15.15 landed
- 0870397ccfbd 16.11 landed
- dc8aa2f586d0 17.7 landed
- 7aecc00b3d66 18.0 landed
- 45f50c995fb6 19 (unreleased) landed
-
pg_dump: Fix dumping of security labels on subscriptions and event triggers.
- dff7591a7041 13.23 landed
- 295c0a644af5 14.20 landed
- 165b07efe4e2 15.15 landed
- 20b23784fc31 16.11 landed
- 968141898777 17.7 landed
- 176002c5bfa6 18.0 landed
- 8e5b92928d28 19 (unreleased) landed
-
pg_restore: Fix comment handling with --no-policies.
- 2ddbfede0c61 18.0 landed
- 66dabc06b1b0 19 (unreleased) landed
-
pg_restore: Fix comment handling with --no-publications / --no-subscriptions.
- 8fbd1f8ea2e1 13.23 landed
- db900ec358bd 14.20 landed
- c8ed16050564 15.15 landed
- 97527a5e68d5 16.11 landed
- f7f9c5d65816 17.7 landed
- 77d2b155caf7 18.0 landed
- b54e8dbfe3b1 19 (unreleased) landed
-
Core support for "extensions", which are packages of SQL objects.
- d9572c4e3b47 9.1.0 cited