From: Peter Eisentraut Date: Wed, 12 Apr 2017 02:02:59 +0000 (-0400) Subject: pg_dump: Dump subscriptions by default X-Git-Tag: REL_10_BETA1~276 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c31671f9b5f6eee9b6726baad2db1795c94839d1;p=postgresql pg_dump: Dump subscriptions by default Dump subscriptions if the current user is a superuser, otherwise write a warning and skip them. Remove the pg_dump option --include-subscriptions. Discussion: https://www.postgresql.org/message-id/e4fbfad5-c6ac-fd50-6777-18c84b34eb2f@2ndquadrant.com --- diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 48db9cd08b..8c70ce3b6e 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -167,9 +167,10 @@ - Subscriptions are not dumped by pg_dump by default, but - this can be requested using the command-line - option . + Subscriptions are dumped by pg_dump if the current user + is a superuser. Otherwise a warning is written and subscriptions are + skipped, because non-superusers cannot read all subscription information + from the pg_subscription catalog. diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index 4f19b89232..53b5dd5239 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -755,15 +755,6 @@ PostgreSQL documentation - - - - - Include logical replication subscriptions in the dump. - - - - diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index d82938141e..1d14b68983 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -119,7 +119,6 @@ typedef struct _restoreOptions bool *idWanted; /* array showing which dump IDs to emit */ int enable_row_security; int sequence_data; /* dump sequence data even in schema-only mode */ - int include_subscriptions; int binary_upgrade; } RestoreOptions; @@ -154,7 +153,6 @@ typedef struct _dumpOptions int outputNoTablespaces; int use_setsessauth; int enable_row_security; - int include_subscriptions; int no_subscription_connect; /* default, if no "inclusion" switches appear, is to dump everything */ diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 79bfbdf1a1..b622506a00 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -171,7 +171,6 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt) dopt->include_everything = ropt->include_everything; dopt->enable_row_security = ropt->enable_row_security; dopt->sequence_data = ropt->sequence_data; - dopt->include_subscriptions = ropt->include_subscriptions; return dopt; } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 5bd4af22c9..dcefe975d8 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -342,7 +342,6 @@ main(int argc, char **argv) {"enable-row-security", no_argument, &dopt.enable_row_security, 1}, {"exclude-table-data", required_argument, NULL, 4}, {"if-exists", no_argument, &dopt.if_exists, 1}, - {"include-subscriptions", no_argument, &dopt.include_subscriptions, 1}, {"inserts", no_argument, &dopt.dump_inserts, 1}, {"lock-wait-timeout", required_argument, NULL, 2}, {"no-tablespaces", no_argument, &dopt.outputNoTablespaces, 1}, @@ -868,7 +867,6 @@ main(int argc, char **argv) ropt->include_everything = dopt.include_everything; ropt->enable_row_security = dopt.enable_row_security; ropt->sequence_data = dopt.sequence_data; - ropt->include_subscriptions = dopt.include_subscriptions; ropt->binary_upgrade = dopt.binary_upgrade; if (compressLevel == -1) @@ -951,7 +949,6 @@ help(const char *progname) " access to)\n")); printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n")); printf(_(" --if-exists use IF EXISTS when dropping objects\n")); - printf(_(" --include-subscriptions dump logical replication subscriptions\n")); printf(_(" --inserts dump data as INSERT commands, rather than COPY\n")); printf(_(" --no-security-labels do not dump security label assignments\n")); printf(_(" --no-subscription-connect dump subscriptions so they don't connect on restore\n")); @@ -3641,6 +3638,22 @@ dumpPublicationTable(Archive *fout, PublicationRelInfo *pubrinfo) destroyPQExpBuffer(query); } +/* + * Is the currently connected user a superuser? + */ +static bool +is_superuser(Archive *fout) +{ + ArchiveHandle *AH = (ArchiveHandle *) fout; + const char *val; + + val = PQparameterStatus(AH->connection, "is_superuser"); + + if (val && strcmp(val, "on") == 0) + return true; + + return false; +} /* * getSubscriptions @@ -3649,7 +3662,6 @@ dumpPublicationTable(Archive *fout, PublicationRelInfo *pubrinfo) void getSubscriptions(Archive *fout) { - DumpOptions *dopt = fout->dopt; PQExpBuffer query; PGresult *res; SubscriptionInfo *subinfo; @@ -3664,9 +3676,25 @@ getSubscriptions(Archive *fout) int i, ntups; - if (!dopt->include_subscriptions || fout->remoteVersion < 100000) + if (fout->remoteVersion < 100000) return; + if (!is_superuser(fout)) + { + int n; + + res = ExecuteSqlQuery(fout, + "SELECT count(*) FROM pg_subscription " + "WHERE subdbid = (SELECT oid FROM pg_catalog.pg_database" + " WHERE datname = current_database())", + PGRES_TUPLES_OK); + n = atoi(PQgetvalue(res, 0, 0)); + if (n > 0) + write_msg(NULL, "WARNING: subscriptions not dumped because current user is not a superuser\n"); + PQclear(res); + return; + } + query = createPQExpBuffer(); resetPQExpBuffer(query); @@ -3714,6 +3742,9 @@ getSubscriptions(Archive *fout) if (strlen(subinfo[i].rolname) == 0) write_msg(NULL, "WARNING: owner of subscription \"%s\" appears to be invalid\n", subinfo[i].dobj.name); + + /* Decide whether we want to dump it */ + selectDumpableObject(&(subinfo[i].dobj), fout); } PQclear(res); @@ -3735,7 +3766,7 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo) int npubnames = 0; int i; - if (dopt->dataOnly) + if (!(subinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)) return; delq = createPQExpBuffer(); diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index 5f61fb2764..ddd79429b4 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -67,7 +67,6 @@ main(int argc, char **argv) char *inputFileSpec; static int disable_triggers = 0; static int enable_row_security = 0; - static int include_subscriptions = 0; static int if_exists = 0; static int no_data_for_failed_tables = 0; static int outputNoTablespaces = 0; @@ -112,7 +111,6 @@ main(int argc, char **argv) {"disable-triggers", no_argument, &disable_triggers, 1}, {"enable-row-security", no_argument, &enable_row_security, 1}, {"if-exists", no_argument, &if_exists, 1}, - {"include-subscriptions", no_argument, &include_subscriptions, 1}, {"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1}, {"no-tablespaces", no_argument, &outputNoTablespaces, 1}, {"role", required_argument, NULL, 2}, @@ -353,7 +351,6 @@ main(int argc, char **argv) opts->disable_triggers = disable_triggers; opts->enable_row_security = enable_row_security; - opts->include_subscriptions = include_subscriptions; opts->noDataForFailedTables = no_data_for_failed_tables; opts->noTablespace = outputNoTablespaces; opts->use_setsessauth = use_setsessauth; diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index cccad04ab6..1db3767f46 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -43,7 +43,6 @@ my %pgdump_runs = ( '--format=custom', "--file=$tempdir/binary_upgrade.dump", '-w', - '--include-subscriptions', # XXX Should not be necessary? '--schema-only', '--binary-upgrade', '-d', 'postgres', # alternative way to specify database @@ -58,7 +57,6 @@ my %pgdump_runs = ( 'pg_dump', '--no-sync', "--file=$tempdir/clean.sql", - '--include-subscriptions', '-c', '-d', 'postgres', # alternative way to specify database ], }, @@ -67,7 +65,6 @@ my %pgdump_runs = ( 'pg_dump', '--no-sync', "--file=$tempdir/clean_if_exists.sql", - '--include-subscriptions', '-c', '--if-exists', '--encoding=UTF8', # no-op, just tests that option is accepted @@ -85,7 +82,6 @@ my %pgdump_runs = ( 'pg_dump', '--no-sync', "--file=$tempdir/createdb.sql", - '--include-subscriptions', '-C', '-R', # no-op, just for testing '-v', @@ -95,7 +91,6 @@ my %pgdump_runs = ( 'pg_dump', '--no-sync', "--file=$tempdir/data_only.sql", - '--include-subscriptions', '-a', '--superuser=test_superuser', '--disable-triggers', @@ -253,7 +248,6 @@ my %pgdump_runs = ( section_pre_data => { dump_cmd => [ 'pg_dump', "--file=$tempdir/section_pre_data.sql", - '--include-subscriptions', '--section=pre-data', '--no-sync', 'postgres', ], }, section_data => { dump_cmd => [ @@ -271,7 +265,7 @@ my %pgdump_runs = ( with_oids => { dump_cmd => [ 'pg_dump', '--oids', - '--include-subscriptions', '--no-sync', + '--no-sync', "--file=$tempdir/with_oids.sql", 'postgres', ], },); ############################################################### @@ -1405,7 +1399,7 @@ my %tests = ( # catch-all for ALTER ... OWNER (except LARGE OBJECTs and PUBLICATIONs) 'ALTER ... OWNER commands (except LARGE OBJECTs and PUBLICATIONs)' => { all_runs => 0, # catch-all - regexp => qr/^ALTER (?!LARGE OBJECT|PUBLICATION)(.*) OWNER TO .*;/m, + regexp => qr/^ALTER (?!LARGE OBJECT|PUBLICATION|SUBSCRIPTION)(.*) OWNER TO .*;/m, like => {}, # use more-specific options above unlike => { column_inserts => 1, @@ -4318,25 +4312,25 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog clean => 1, clean_if_exists => 1, createdb => 1, - with_oids => 1, }, - unlike => { defaults => 1, exclude_test_table_data => 1, exclude_dump_test_schema => 1, exclude_test_table => 1, - section_pre_data => 1, no_blobs => 1, no_privs => 1, no_owner => 1, + pg_dumpall_dbprivs => 1, + schema_only => 1, + section_post_data => 1, + with_oids => 1, }, + unlike => { + section_pre_data => 1, only_dump_test_schema => 1, only_dump_test_table => 1, - pg_dumpall_dbprivs => 1, pg_dumpall_globals => 1, pg_dumpall_globals_clean => 1, - schema_only => 1, # XXX Should be like? role => 1, - section_pre_data => 1, # XXX Should be like? - section_post_data => 1, + section_pre_data => 1, test_schema_plus_blobs => 1, }, }, 'ALTER PUBLICATION pub1 ADD TABLE test_table' => {