From c31671f9b5f6eee9b6726baad2db1795c94839d1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 11 Apr 2017 22:02:59 -0400
Subject: [PATCH] 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
---
doc/src/sgml/logical-replication.sgml | 7 +++--
doc/src/sgml/ref/pg_dump.sgml | 9 ------
src/bin/pg_dump/pg_backup.h | 2 --
src/bin/pg_dump/pg_backup_archiver.c | 1 -
src/bin/pg_dump/pg_dump.c | 43 +++++++++++++++++++++++----
src/bin/pg_dump/pg_restore.c | 3 --
src/bin/pg_dump/t/002_pg_dump.pl | 24 ++++++---------
7 files changed, 50 insertions(+), 39 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 48db9cd08b7..8c70ce3b6e4 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -167,9 +167,10 @@
</para>
<para>
- Subscriptions are not dumped by <command>pg_dump</command> by default, but
- this can be requested using the command-line
- option <option>--include-subscriptions</option>.
+ Subscriptions are dumped by <command>pg_dump</command> 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 <structname>pg_subscription</structname> catalog.
</para>
<para>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 4f19b892321..53b5dd52394 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -755,15 +755,6 @@ PostgreSQL documentation
</listitem>
</varlistentry>
- <varlistentry>
- <term><option>--include-subscriptions</option></term>
- <listitem>
- <para>
- Include logical replication subscriptions in the dump.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><option>--inserts</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index d82938141e4..1d14b689837 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 79bfbdf1a1d..b622506a00d 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 5bd4af22c97..dcefe975d89 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 5f61fb2764e..ddd79429b4b 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 cccad04ab62..1db3767f46d 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' => {
--
GitLab