On Wed, Aug 27, 2025 at 3:18 PM jian he <[email protected]> wrote: > > 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? > > > works for me.
So I've split your v2-0001 patch into two patches: * v4-0001 handles comments on publications and subscriptions when --no-publications / --no-subscriptions are specified. This will be backpatched to all supported versions. * v4-0002 handles comments on policies when --no-policies is specified. This will be backpatched to v18, where --no-policies was added. Both v4-0001 and v4-0002 are based on your patch, but I added regression tests for them. > > > 02: make pg_dump dump security label for shared database objects, like > > > subscription, roles. As I understand it, shared objects like roles are handled by pg_dumpall, which already dumps their security labels via pg_shseclabel. Subscriptions are an exception: pg_dump dumps them (and should dump their security labels), but those labels are stored in pg_shseclabel, which pg_dump doesn't query. To fix this, making pg_dump query also pg_shseclabel when dumping subscriptions would work. But your approach, having pg_dump query pg_seclabels (covering both pg_seclabel and pg_shseclabel), is simpler and sufficient. So I like your approach for now. I also noticed pg_dump didn't dump security labels on event triggers, so I extended your patch as v4-0003 to handle those as well. > > > 03: make pg_restore not restore security labels if the associated > > > object is excluded. This patch looks good. I only applied minor cosmetic changes and attached it as v4-0004. Regards, -- Fujii Masao
From 6d85d536c37cbce55fa07fe9300c15372ea48151 Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Wed, 3 Sep 2025 10:18:19 +0900 Subject: [PATCH v17 1/3] pg_restore: Fix comment handling with --no-publications / --no-subscriptions. Previously, pg_restore did not skip comments on publications or subscriptions even when --no-publications or --no-subscriptions was specified. As a result, it could issue COMMENT commands for objects that were never created, causing those commands to fail. This commit fixes the issue by ensuring that comments on publications and subscriptions are also skipped when the corresponding options are used. Backpatch to all supported versions. Author: Jian He <[email protected]> Co-authored-by: Fujii Masao <[email protected]> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dqxxqacj0xydhc...@mail.gmail.com Backpatch-through: 13 --- src/bin/pg_dump/pg_backup_archiver.c | 14 ++++++++++ src/bin/pg_dump/t/002_pg_dump.pl | 41 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index f830704cf37..b07ee7bb87d 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2960,6 +2960,20 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) if (ropt->no_comments && strcmp(te->desc, "COMMENT") == 0) return 0; + /* + * If it's a comment on a publication or a subscription, maybe ignore it. + */ + if (strcmp(te->desc, "COMMENT") == 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. diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 8d5b2e1dc45..cd69ae7479e 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -537,6 +537,29 @@ my %pgdump_runs = ( 'postgres', ], }, + no_subscriptions => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--file' => "$tempdir/no_subscriptions.sql", + '--no-subscriptions', + 'postgres', + ], + }, + no_subscriptions_restore => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.dump", + 'postgres', + ], + restore_cmd => [ + 'pg_restore', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.sql", + '--no-subscriptions', + "$tempdir/no_subscriptions_restore.dump", + ], + }, no_table_access_method => { dump_cmd => [ 'pg_dump', '--no-sync', @@ -711,6 +734,8 @@ my %full_runs = ( no_large_objects => 1, no_owner => 1, no_privs => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, @@ -1532,6 +1557,10 @@ my %tests = ( regexp => qr/^COMMENT ON SUBSCRIPTION sub1 IS 'comment on subscription';/m, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'COMMENT ON TEXT SEARCH CONFIGURATION dump_test.alt_ts_conf1' => { @@ -3016,6 +3045,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'CREATE SUBSCRIPTION sub2' => { @@ -3027,6 +3060,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub2 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub2', origin = none);\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'CREATE SUBSCRIPTION sub3' => { @@ -3038,6 +3075,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub3 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub3');\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'ALTER PUBLICATION pub1 ADD TABLE test_table' => { -- 2.50.1
v4-0001-PG18-master-pg_restore-Fix-comment-handling-with-no-publicati.patch
Description: Binary data
From 4162391fc112756c91cde59fce728bed576ce05e Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Wed, 3 Sep 2025 10:18:19 +0900 Subject: [PATCH v16 1/3] pg_restore: Fix comment handling with --no-publications / --no-subscriptions. Previously, pg_restore did not skip comments on publications or subscriptions even when --no-publications or --no-subscriptions was specified. As a result, it could issue COMMENT commands for objects that were never created, causing those commands to fail. This commit fixes the issue by ensuring that comments on publications and subscriptions are also skipped when the corresponding options are used. Backpatch to all supported versions. Author: Jian He <[email protected]> Co-authored-by: Fujii Masao <[email protected]> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dqxxqacj0xydhc...@mail.gmail.com Backpatch-through: 13 --- src/bin/pg_dump/pg_backup_archiver.c | 14 +++++++++ src/bin/pg_dump/t/002_pg_dump.pl | 45 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index b0ef1980360..93026a8ddc5 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2855,6 +2855,20 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) if (ropt->no_comments && strcmp(te->desc, "COMMENT") == 0) return 0; + /* + * If it's a comment on a publication or a subscription, maybe ignore it. + */ + if (strcmp(te->desc, "COMMENT") == 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. diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index b491e461158..5a69e58f217 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -548,6 +548,29 @@ my %pgdump_runs = ( 'postgres', ], }, + no_subscriptions => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--file' => "$tempdir/no_subscriptions.sql", + '--no-subscriptions', + 'postgres', + ], + }, + no_subscriptions_restore => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.dump", + 'postgres', + ], + restore_cmd => [ + 'pg_restore', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.sql", + '--no-subscriptions', + "$tempdir/no_subscriptions_restore.dump", + ], + }, no_table_access_method => { dump_cmd => [ 'pg_dump', '--no-sync', @@ -722,6 +745,8 @@ my %full_runs = ( no_large_objects => 1, no_owner => 1, no_privs => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, @@ -1546,6 +1571,10 @@ my %tests = ( regexp => qr/^COMMENT ON SUBSCRIPTION sub1 IS 'comment on subscription';/m, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'COMMENT ON TEXT SEARCH CONFIGURATION dump_test.alt_ts_conf1' => { @@ -2988,6 +3017,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'CREATE SUBSCRIPTION sub2' => { @@ -2999,6 +3032,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub2 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub2', origin = none);\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'CREATE SUBSCRIPTION sub3' => { @@ -3010,6 +3047,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub3 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub3');\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'ALTER PUBLICATION pub1 ADD TABLE test_table' => { @@ -3862,6 +3903,8 @@ my %tests = ( no_large_objects => 1, no_privs => 1, no_owner => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, only_dump_test_schema => 1, pg_dumpall_dbprivs => 1, @@ -3954,6 +3997,8 @@ my %tests = ( no_large_objects => 1, no_privs => 1, no_owner => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, -- 2.50.1
From 68e0ef94f5d1d4303296a0dcb647f082ef7c3461 Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Wed, 3 Sep 2025 10:18:19 +0900 Subject: [PATCH v15 1/3] pg_restore: Fix comment handling with --no-publications / --no-subscriptions. Previously, pg_restore did not skip comments on publications or subscriptions even when --no-publications or --no-subscriptions was specified. As a result, it could issue COMMENT commands for objects that were never created, causing those commands to fail. This commit fixes the issue by ensuring that comments on publications and subscriptions are also skipped when the corresponding options are used. Backpatch to all supported versions. Author: Jian He <[email protected]> Co-authored-by: Fujii Masao <[email protected]> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dqxxqacj0xydhc...@mail.gmail.com Backpatch-through: 13 --- src/bin/pg_dump/pg_backup_archiver.c | 14 +++++++++++ src/bin/pg_dump/t/002_pg_dump.pl | 37 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index b81788f72cb..61beeea77cb 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2875,6 +2875,20 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) if (ropt->no_comments && strcmp(te->desc, "COMMENT") == 0) return 0; + /* + * If it's a comment on a publication or a subscription, maybe ignore it. + */ + if (strcmp(te->desc, "COMMENT") == 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. diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 70d2922db3d..bef145c3382 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -336,6 +336,29 @@ my %pgdump_runs = ( 'postgres', ], }, + no_subscriptions => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--file' => "$tempdir/no_subscriptions.sql", + '--no-subscriptions', + 'postgres', + ], + }, + no_subscriptions_restore => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.dump", + 'postgres', + ], + restore_cmd => [ + 'pg_restore', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.sql", + '--no-subscriptions', + "$tempdir/no_subscriptions_restore.dump", + ], + }, no_table_access_method => { dump_cmd => [ 'pg_dump', '--no-sync', @@ -496,6 +519,8 @@ my %full_runs = ( no_blobs => 1, no_owner => 1, no_privs => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, @@ -1245,6 +1270,10 @@ my %tests = ( regexp => qr/^COMMENT ON SUBSCRIPTION sub1 IS 'comment on subscription';/m, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'COMMENT ON TEXT SEARCH CONFIGURATION dump_test.alt_ts_conf1' => { @@ -2557,6 +2586,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'ALTER PUBLICATION pub1 ADD TABLE test_table' => { @@ -3251,6 +3284,8 @@ my %tests = ( no_blobs => 1, no_privs => 1, no_owner => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, only_dump_test_schema => 1, pg_dumpall_dbprivs => 1, @@ -3324,6 +3359,8 @@ my %tests = ( no_blobs => 1, no_privs => 1, no_owner => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, no_table_access_method => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, -- 2.50.1
From fc6a1f024b534331808214e3e73eb960b2eb8d8f Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Wed, 3 Sep 2025 10:18:19 +0900 Subject: [PATCH v14 1/3] pg_restore: Fix comment handling with --no-publications / --no-subscriptions. Previously, pg_restore did not skip comments on publications or subscriptions even when --no-publications or --no-subscriptions was specified. As a result, it could issue COMMENT commands for objects that were never created, causing those commands to fail. This commit fixes the issue by ensuring that comments on publications and subscriptions are also skipped when the corresponding options are used. Backpatch to all supported versions. Author: Jian He <[email protected]> Co-authored-by: Fujii Masao <[email protected]> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dqxxqacj0xydhc...@mail.gmail.com Backpatch-through: 13 --- src/bin/pg_dump/pg_backup_archiver.c | 14 +++++++++++ src/bin/pg_dump/t/002_pg_dump.pl | 37 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 6f2cd8c21ad..99c1e2d6f9a 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2858,6 +2858,20 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) if (ropt->no_comments && strcmp(te->desc, "COMMENT") == 0) return 0; + /* + * If it's a comment on a publication or a subscription, maybe ignore it. + */ + if (strcmp(te->desc, "COMMENT") == 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. diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index c38448153d2..09f6c230c47 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -268,6 +268,29 @@ my %pgdump_runs = ( 'postgres', ], }, + no_subscriptions => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--file' => "$tempdir/no_subscriptions.sql", + '--no-subscriptions', + 'postgres', + ], + }, + no_subscriptions_restore => { + dump_cmd => [ + 'pg_dump', '--no-sync', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.dump", + 'postgres', + ], + restore_cmd => [ + 'pg_restore', + '--format' => 'custom', + '--file' => "$tempdir/no_subscriptions_restore.sql", + '--no-subscriptions', + "$tempdir/no_subscriptions_restore.dump", + ], + }, only_dump_test_schema => { dump_cmd => [ 'pg_dump', '--no-sync', @@ -420,6 +443,8 @@ my %full_runs = ( no_blobs => 1, no_owner => 1, no_privs => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, schema_only => 1,); @@ -1131,6 +1156,10 @@ my %tests = ( regexp => qr/^COMMENT ON SUBSCRIPTION sub1 IS 'comment on subscription';/m, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'COMMENT ON TEXT SEARCH CONFIGURATION dump_test.alt_ts_conf1' => { @@ -2401,6 +2430,10 @@ my %tests = ( \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E /xm, like => { %full_runs, section_post_data => 1, }, + unlike => { + no_subscriptions => 1, + no_subscriptions_restore => 1, + }, }, 'ALTER PUBLICATION pub1 ADD TABLE test_table' => { @@ -2969,6 +3002,8 @@ my %tests = ( no_blobs => 1, no_privs => 1, no_owner => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, only_dump_test_schema => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, @@ -3040,6 +3075,8 @@ my %tests = ( no_blobs => 1, no_privs => 1, no_owner => 1, + no_subscriptions => 1, + no_subscriptions_restore => 1, pg_dumpall_dbprivs => 1, pg_dumpall_exclude => 1, role => 1, -- 2.50.1
v4-0004-pg_restore-Fix-security-label-handling-with-no-pu.patch
Description: Binary data
v4-0003-pg_dump-Fix-dumping-of-security-labels-on-subscri.patch
Description: Binary data
v4-0002-pg_restore-Fix-comment-handling-with-no-policies.patch
Description: Binary data
