Re: pg_restore --no-policies should not restore policies' comment

On Thu, Jul 3, 2025 at 11:32 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Wed, Jul 2, 2025 at 5:18 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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