On Wed, Jul 2, 2025 at 5:18 PM Fujii Masao <masao.fu...@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: 01: make pg_restore not restore comments if comments associated objects are excluded. 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.
From 62055ddac9cf73def22227e9d3af275f58180707 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 3 Jul 2025 11:18:06 +0800 Subject: [PATCH v2 2/3] make pg_dump dump security label for shared database objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As noted in [1], SUBSCRIPTION and similar objects are treated as global objects. Therefore, we should use pg_catalog.pg_seclabels in collectSecLabels instead of pg_catalog.pg_seclabel. The database’s own security label is handled via dumpDatabase, so this change should be appropriate. [1] https://www.postgresql.org/docs/devel/catalog-pg-shseclabel.html discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dqxxqacj0xydhc...@mail.gmail.com commitfest entry: https://commitfest.postgresql.org/ --- src/bin/pg_dump/pg_dump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 1937997ea67..17cca2dd667 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -16476,7 +16476,7 @@ collectSecLabels(Archive *fout) appendPQExpBufferStr(query, "SELECT label, provider, classoid, objoid, objsubid " - "FROM pg_catalog.pg_seclabel " + "FROM pg_catalog.pg_seclabels " "ORDER BY classoid, objoid, objsubid"); res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); -- 2.34.1
From 74a92cfbf38872945ed9362bbbd4e90fb3639787 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 3 Jul 2025 22:13:43 +0800 Subject: [PATCH v2 3/3] not restore security labels if the associated object is excluded When --no-publications or --no-subscriptions is specified in pg_restore, security labels associated with those objects should be excluded. pg_dump.c, dumpSecLabel function and the Security Label synopsis section [1] helps verify that the TocEntry->tag string for a security label begins with the object type it is associated with. So this patch change should be fine. [1] https://www.postgresql.org/docs/current/sql-security-label.html discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dqxxqacj0xydhc...@mail.gmail.com commitfest entry: https://commitfest.postgresql.org/ --- src/bin/pg_dump/pg_backup_archiver.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 44bcce71712..f407f9fc280 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -3051,6 +3051,21 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) if (ropt->no_security_labels && strcmp(te->desc, "SECURITY LABEL") == 0) return 0; + + if (strcmp(te->desc, "SECURITY LABEL") == 0) + { + /* + * If --no-publications, or --no-subscriptions is specified, security + * label related to those objects should also be skipped during restore. + * see dumpSecLabel also. + */ + if (ropt->no_publications && strncmp(te->tag, "PUBLICATION", strlen("PUBLICATION")) == 0) + return 0; + + if (ropt->no_subscriptions && strncmp(te->tag, "SUBSCRIPTION", strlen("SUBSCRIPTION")) == 0) + return 0; + } + /* If it's a subscription, maybe ignore it */ if (ropt->no_subscriptions && strcmp(te->desc, "SUBSCRIPTION") == 0) return 0; -- 2.34.1
From eeaff76aea6dc92a122b8187248773c72ffba691 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 3 Jul 2025 22:16:05 +0800 Subject: [PATCH v2 1/3] not restore comments if the associated object is excluded If certain objects are excluded from restoration in pg_restore, then comments associated with those objects should also not be restored. So this patch applies to the following pg_restore option: --no-policies, --no-publications, --no-subscriptions TODO: need perl tests discussion: https://postgr.es/m/18970-a7d1cfe1f8d5d...@postgresql.org --- src/bin/pg_dump/pg_backup_archiver.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 197c1295d93..44bcce71712 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -3020,6 +3020,23 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) strcmp(te->desc, "ROW SECURITY") == 0)) return 0; + if (strcmp(te->desc, "COMMENT") == 0) + { + /* + * If --no-policies, --no-publications, or --no-subscriptions is + * specified, comments related to those objects should also be skipped + * during restore. + */ + if (ropt->no_policies && strncmp(te->tag, "POLICY", strlen("POLICY")) == 0) + return 0; + + if (ropt->no_publications && strncmp(te->tag, "PUBLICATION", strlen("PUBLICATION")) == 0) + return 0; + + if (ropt->no_subscriptions && strncmp(te->tag, "SUBSCRIPTION", strlen("SUBSCRIPTION")) == 0) + return 0; + } + /* * If it's a publication or a table part of a publication, maybe ignore * it. -- 2.34.1
