SUBSCRIPTIONS and pg_upgrade
Peter,
* Peter Eisentraut (peter_e@gmx.net) wrote:
Logical replication
- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workers
I'm not entirely sure about the reasoning behind requiring a flag to
include subscriptions in pg_dump output, as the documentation doesn't
actually provide one, but if we are going to require that, shouldn't
pg_upgrade use it, to make sure that the subscriptions are pulled
forward to the upgraded cluster?
Also, we should probably discuss that default in pg_dump to not include
something in the database by default as that's not something we've ever
done before. We made a very deliberate point to make sure that RLS
didn't work by default with pg_dump to avoid the risk that we might not
dump include everything in the database in the pg_dump output. I agree
that it's not exactly the same, but even so, I was surprised to find
out that subscriptions aren't included by default and I doubt I'd be
alone.
If this was all already discussed, I'm happy to go review the relevant
thread(s). I'll admit that I didn't follow all of that discussion very
closely, I'm just going through parts of pg_dump which are not being
tested in our regression tests currently and discovered that dumping out
subscriptions is not tested at all.
Thanks!
Stephen
Peter,
* Stephen Frost (sfrost@snowman.net) wrote:
* Peter Eisentraut (peter_e@gmx.net) wrote:
Logical replication
- Add PUBLICATION catalogs and DDL
- Add SUBSCRIPTION catalog and DDL
- Define logical replication protocol and output plugin
- Add logical replication workersI'm not entirely sure about the reasoning behind requiring a flag to
include subscriptions in pg_dump output, as the documentation doesn't
actually provide one, but if we are going to require that, shouldn't
pg_upgrade use it, to make sure that the subscriptions are pulled
forward to the upgraded cluster?Also, we should probably discuss that default in pg_dump to not include
something in the database by default as that's not something we've ever
done before. We made a very deliberate point to make sure that RLS
didn't work by default with pg_dump to avoid the risk that we might not
dump include everything in the database in the pg_dump output. I agree
that it's not exactly the same, but even so, I was surprised to find
out that subscriptions aren't included by default and I doubt I'd be
alone.If this was all already discussed, I'm happy to go review the relevant
thread(s). I'll admit that I didn't follow all of that discussion very
closely, I'm just going through parts of pg_dump which are not being
tested in our regression tests currently and discovered that dumping out
subscriptions is not tested at all.
We should probably also throw an error if --include-subscriptions and
--data-only are used together, instead of just not dumping out the
subscriptions in that case, which is what happens now.
Surely, if the user asked for subscriptions to be included, we should
either throw an error or actually include them.
Thanks!
Stephen
On 2/16/17 21:04, Stephen Frost wrote:
I'm not entirely sure about the reasoning behind requiring a flag to
include subscriptions in pg_dump output,
One reason is that pg_subscription is only readable by a superuser, so
we can't even dump them unless we're superuser.
Also, restoring a subscription will immediately attempt to start
replication, which might be kind of surprising.
We had pondered this issue extensively during early review. I don't
think we're all happy with the current behavior, but it's probably the
safest and easiest one for now.
Better ideas are welcome.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/16/17 21:04, Stephen Frost wrote:
I'm not entirely sure about the reasoning behind requiring a flag to
include subscriptions in pg_dump output,One reason is that pg_subscription is only readable by a superuser, so
we can't even dump them unless we're superuser.
Sure, but that's true of roles and other things.. On a system with RLS,
likely only the database superuser can actually read everything.
Also, restoring a subscription will immediately attempt to start
replication, which might be kind of surprising.
Isn't that what the 'disabled' option is for? Also, when it comes to
pg_upgrade, isn't that what you would probably want?
Perhaps it shouldn't start immediately, but rather create them as
disabled at first and then start them at the end of the restore.
We had pondered this issue extensively during early review. I don't
think we're all happy with the current behavior, but it's probably the
safest and easiest one for now.Better ideas are welcome.
I don't think we can simply punt entirely on this, particularly for the
pg_upgrade case. At the least, I think we should be exporting the
subscriptions in a disabled fashion, so that at least the user *has*
them in the backup, even if they aren't enabled. With pg_upgrade, I
would think we'd want to have some option to tell pg_upgrade to include
subscriptions, or to enable them afterwards if they're included but off
by default.
I've added it to the open items for PG10. Perhaps we can get some other
input/suggestions on it.
Thanks!
Stephen
On 2/17/17 09:33, Stephen Frost wrote:
One reason is that pg_subscription is only readable by a superuser, so
we can't even dump them unless we're superuser.Sure, but that's true of roles and other things.. On a system with RLS,
likely only the database superuser can actually read everything.
There are some concurrent discussions about getting a list of
subscriptions without having superuser. Once that is resolved, we'll
likely have more options here, such as dumping only subscriptions you
are owner of or something like that.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/17/17 09:33, Stephen Frost wrote:
One reason is that pg_subscription is only readable by a superuser, so
we can't even dump them unless we're superuser.Sure, but that's true of roles and other things.. On a system with RLS,
likely only the database superuser can actually read everything.There are some concurrent discussions about getting a list of
subscriptions without having superuser. Once that is resolved, we'll
likely have more options here, such as dumping only subscriptions you
are owner of or something like that.
I certainly agree that we should provide a way to view subscriptions,
and perhaps do other things with them, without being a superuser.
I was going to suggest that it might make sense to have a role attribute
or a default role created for managing publications/subscriptions. It
seems like this should be independent of the existing replicaiton role
attribute, which is about physical replication.
Thanks!
Stephen
On Fri, Feb 17, 2017 at 7:34 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm not entirely sure about the reasoning behind requiring a flag to
include subscriptions in pg_dump output, as the documentation doesn't
actually provide one, but if we are going to require that, shouldn't
pg_upgrade use it, to make sure that the subscriptions are pulled
forward to the upgraded cluster?
Subscriptions are different from other objects in that whether or not
you want them in your dump output depends on how you plan to use the
dump. If your goal is to create a server to replace the original
server, you want the subscriptions. If you goal is to take a static
copy of the data, you don't. Subscriptions aren't really data in the
sense that table data is data, or even in the sense that functions are
data. Granted, you can execute a function, but a subscription is
self-executing.
That having been said, I share your discomfort with not dumping these
by default. I think it would be good to dump and restore them by
default, but maybe restore them in disabled mode as you suggest
downthread, and document that if you want them enabled you have to
turn them on after doing the restore. Otherwise, you could have
logical replication start up before the dump restore even completes,
which seems like it could cause all sorts of problems.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 17, 2017 at 09:33:32AM -0500, Stephen Frost wrote:
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/16/17 21:04, Stephen Frost wrote:
I'm not entirely sure about the reasoning behind requiring a flag to
include subscriptions in pg_dump output,One reason is that pg_subscription is only readable by a superuser, so
we can't even dump them unless we're superuser.Sure, but that's true of roles and other things.. On a system with RLS,
likely only the database superuser can actually read everything.Also, restoring a subscription will immediately attempt to start
replication, which might be kind of surprising.Isn't that what the 'disabled' option is for? Also, when it comes to
pg_upgrade, isn't that what you would probably want?Perhaps it shouldn't start immediately, but rather create them as
disabled at first and then start them at the end of the restore.We had pondered this issue extensively during early review. I don't
think we're all happy with the current behavior, but it's probably the
safest and easiest one for now.Better ideas are welcome.
I don't think we can simply punt entirely on this, particularly for the
pg_upgrade case. At the least, I think we should be exporting the
subscriptions in a disabled fashion, so that at least the user *has*
them in the backup, even if they aren't enabled. With pg_upgrade, I
would think we'd want to have some option to tell pg_upgrade to include
subscriptions, or to enable them afterwards if they're included but off
by default.I've added it to the open items for PG10. Perhaps we can get some other
input/suggestions on it.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
OK, we need to come to a conclusion here. To summarize:
Problem 1: pg_subscription.subconninfo can only be read by superuser.
So non-superusers cannot dump subscriptions.
Precedent is pg_user_mapping. In that case, we just omit the
user-mapping options if we're not a superuser. Pretty dubious, but in
any case that won't work here, because you cannot create a subscription
without a CONNECTION clause.
Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.
Problem 2: Restoring a subscription immediately starts replication.
Maybe you want that or maybe you don't. We could dump all subscriptions
in DISABLED state. But then after restoring you have to go and manually
enable all subscriptions. We don't have a command to turn all
subscriptions back on at once. Maybe that is not a terrible issue,
since one wouldn't normally have many subscriptions.
Proposal: Dump all subscriptions in DISABLED state. Remove current
pg_dump --no-subscription-connect option.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
Problem 1: pg_subscription.subconninfo can only be read by superuser.
So non-superusers cannot dump subscriptions.
I'm not particularly happy with this.
Precedent is pg_user_mapping. In that case, we just omit the
user-mapping options if we're not a superuser. Pretty dubious, but in
any case that won't work here, because you cannot create a subscription
without a CONNECTION clause.
Given that you can create a disabled subscription, why is a connection
clause required..? I agree that simply excluding pg_user_mapping isn't
great but I don't really think we want to use something which we agree
isn't ideal as the best approach for this.
Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.
That option was added, iirc, in part because pg_dump was including
subscriptions in things like explicit single-table dumps. I certainly
don't think we should start including all subscriptions in all dumps.
The question becomes if it's useful to include subscriptions when only
dumping a subset of objects or if they should *only* be included when
doing a full dump. My gut feeling is that if we care enough about blobs
to have an explicit option to include them, even when we aren't dumping
out everything, then having something similar for subscriptions makes
sense.
Problem 2: Restoring a subscription immediately starts replication.
Maybe you want that or maybe you don't. We could dump all subscriptions
in DISABLED state. But then after restoring you have to go and manually
enable all subscriptions. We don't have a command to turn all
subscriptions back on at once. Maybe that is not a terrible issue,
since one wouldn't normally have many subscriptions.
Having a way to turn them all on would be nice.
Proposal: Dump all subscriptions in DISABLED state. Remove current
pg_dump --no-subscription-connect option.
I like this idea in general, I'm just not sure if it's the right answer
when we're talking about pg_upgrade. At a minimum, if we go with this
approach, we should produce a warning when subscriptions exist during
the pg_upgrade that the user will need to go re-enable them.
Thanks!
Stephen
On 4/10/17 20:55, Stephen Frost wrote:
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
Problem 1: pg_subscription.subconninfo can only be read by superuser.
So non-superusers cannot dump subscriptions.I'm not particularly happy with this.
Why? How? Alternatives?
Precedent is pg_user_mapping. In that case, we just omit the
user-mapping options if we're not a superuser. Pretty dubious, but in
any case that won't work here, because you cannot create a subscription
without a CONNECTION clause.Given that you can create a disabled subscription, why is a connection
clause required..? I agree that simply excluding pg_user_mapping isn't
great but I don't really think we want to use something which we agree
isn't ideal as the best approach for this.
Well, I suppose you could somehow make it work so that a connection
clause is not required. But then why dump the subscription at all? You
start stripping off information and it becomes less useful.
Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.That option was added, iirc, in part because pg_dump was including
subscriptions in things like explicit single-table dumps.
No, you are thinking of publications.
The option was added because at some point during the review process of
the early patches, pg_dump for a non-superuser would just fail outright
as it was trying to read pg_subscription.
The question becomes if it's useful to include subscriptions when only
dumping a subset of objects or if they should *only* be included when
doing a full dump.
I think we'd handle that similar to publications.
Proposal: Dump all subscriptions in DISABLED state. Remove current
pg_dump --no-subscription-connect option.I like this idea in general, I'm just not sure if it's the right answer
when we're talking about pg_upgrade. At a minimum, if we go with this
approach, we should produce a warning when subscriptions exist during
the pg_upgrade that the user will need to go re-enable them.
Sure, that's doable. It's the way of pg_upgrade in general to give you
a bunch of notes and scripts afterwards, so it wouldn't be too strange
to add that in.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 4/10/17 20:55, Stephen Frost wrote:
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
Problem 1: pg_subscription.subconninfo can only be read by superuser.
So non-superusers cannot dump subscriptions.I'm not particularly happy with this.
Why? How? Alternatives?
Generally speaking, we should be trying to move away from superuser-only
anything, not introducing more of it. If the connection string can have
sensitive data in it, I'd be happier if we could pull that sensitive
information out into its own column to allow the rest to be included,
but if we have to exclude the connection string then we have to, but the
rest should be available to individuals with appropriate rights. That
doesn't necessairly mean "public" should be allowed to see it, but we
should have a role who can who isn't superuser.
Precedent is pg_user_mapping. In that case, we just omit the
user-mapping options if we're not a superuser. Pretty dubious, but in
any case that won't work here, because you cannot create a subscription
without a CONNECTION clause.Given that you can create a disabled subscription, why is a connection
clause required..? I agree that simply excluding pg_user_mapping isn't
great but I don't really think we want to use something which we agree
isn't ideal as the best approach for this.Well, I suppose you could somehow make it work so that a connection
clause is not required. But then why dump the subscription at all? You
start stripping off information and it becomes less useful.
Isn't there other information associated with the subscription beyond
just the connection string? Even if there isn't today, I certainly
expect there will be in the future (at a minimum, mapping between the
foreign table and a local table with a different name really should be
supported...).
Consider the recently added option to not include passwords for roles
being dumped through pg_dumpall. That's useful, for multiple reasons,
even if the roles don't have any objects which depend on them.
Including this information is required to rebuild the system as closely
as possible to the original state.
Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.That option was added, iirc, in part because pg_dump was including
subscriptions in things like explicit single-table dumps.No, you are thinking of publications.
Even so, the other comments apply equally here, from what I can tell.
The option was added because at some point during the review process of
the early patches, pg_dump for a non-superuser would just fail outright
as it was trying to read pg_subscription.
That's certainly an issue in general.
The question becomes if it's useful to include subscriptions when only
dumping a subset of objects or if they should *only* be included when
doing a full dump.I think we'd handle that similar to publications.
Which is to say that they'd only be included in a 'full' dump? I'd like
a way to do better than that in general, but having them only included
in full dumps would also be an option, at least for PG10.
Proposal: Dump all subscriptions in DISABLED state. Remove current
pg_dump --no-subscription-connect option.I like this idea in general, I'm just not sure if it's the right answer
when we're talking about pg_upgrade. At a minimum, if we go with this
approach, we should produce a warning when subscriptions exist during
the pg_upgrade that the user will need to go re-enable them.Sure, that's doable. It's the way of pg_upgrade in general to give you
a bunch of notes and scripts afterwards, so it wouldn't be too strange
to add that in.
Right, that's why I was suggesting it.
Thanks!
Stephen
On Mon, Apr 10, 2017 at 10:08 PM, Stephen Frost <sfrost@snowman.net> wrote:
Generally speaking, we should be trying to move away from superuser-only
anything, not introducing more of it.
I totally agree, which is why I was rather surprised when you
vigorously objected to my attempts to replace the remainder of the
main tree's superuser checks that completely block execution of
certain SQL functions with privilege grants. The parameters within
which you find explicit superuser checks tolerable are extremely murky
to me.
If the connection string can have
sensitive data in it, ...
I would argue that a great deal of what's in a connection string could
potentially be sensitive. Do you want to disclose to unprivileged
users potentially-useful host names, IP addresses, port numbers, user
names, passwords, and/or required SSL settings? I don't think we
should assume any of that stuff to be generally OK to disclose to
non-superusers. It could be OK to disclose to everyone in some
installations, or to some people even in highly secure installations,
but the idea that there is nobody who cares about obscuring the
majority of what goes into a connection string doesn't sound right to
me.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert,
* Robert Haas (robertmhaas@gmail.com) wrote:
On Mon, Apr 10, 2017 at 10:08 PM, Stephen Frost <sfrost@snowman.net> wrote:
Generally speaking, we should be trying to move away from superuser-only
anything, not introducing more of it.I totally agree, which is why I was rather surprised when you
vigorously objected to my attempts to replace the remainder of the
main tree's superuser checks that completely block execution of
certain SQL functions with privilege grants. The parameters within
which you find explicit superuser checks tolerable are extremely murky
to me.
Filesystem-level access seems reasonable to restrict to superusers.
If the connection string can have
sensitive data in it, ...I would argue that a great deal of what's in a connection string could
potentially be sensitive. Do you want to disclose to unprivileged
users potentially-useful host names, IP addresses, port numbers, user
names, passwords, and/or required SSL settings? I don't think we
should assume any of that stuff to be generally OK to disclose to
non-superusers. It could be OK to disclose to everyone in some
installations, or to some people even in highly secure installations,
but the idea that there is nobody who cares about obscuring the
majority of what goes into a connection string doesn't sound right to
me.
I specifically made a point to not question what was or wasn't
considered sensitive as it can certainly vary. The point I was making
is that if there's sensitive information there then we can exclude just
that information but allow a pg_dump-using user to see that a
subscription exists without it.
I agree that it might be interesting to discuss which information should
be limited to superusers only, which information should be available to
privileged non-superusers (pg_read_all_settings, for example, allows
visibility to information that we previously limited to superusers) and
what information should be available to the 'public' user, but this
isn't the place to discuss any of that- this is about how to address the
issues which currently exist around pg_dump'ing of subscriptions (and,
perhaps, publications and they're related). I don't believe we want to
de-rail this into a largely independent discussion, given that it's an
open item that needs to be addressed shortly if we're going to get beta
out on time.
Thanks!
Stephen
On Tue, Apr 11, 2017 at 9:48 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 10, 2017 at 1:58 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:OK, we need to come to a conclusion here. To summarize:
Problem 1: pg_subscription.subconninfo can only be read by superuser.
So non-superusers cannot dump subscriptions.Precedent is pg_user_mapping. In that case, we just omit the
user-mapping options if we're not a superuser. Pretty dubious, but in
any case that won't work here, because you cannot create a subscription
without a CONNECTION clause.Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.+1. I don't totally love it, but I don't have a better idea.
Problem 2: Restoring a subscription immediately starts replication.
Maybe you want that or maybe you don't. We could dump all subscriptions
in DISABLED state. But then after restoring you have to go and manually
enable all subscriptions. We don't have a command to turn all
subscriptions back on at once. Maybe that is not a terrible issue,
since one wouldn't normally have many subscriptions.Proposal: Dump all subscriptions in DISABLED state. Remove current
pg_dump --no-subscription-connect option.+1. I like this a lot.
Oops, forgot to copy the list.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: CA+TgmoaDhMpTxkWddW79zqKnR0L79Q+5fwZgYehR+T15dJN-A@mail.gmail.com
On Tue, Apr 11, 2017 at 10:13 AM, Stephen Frost <sfrost@snowman.net> wrote:
I totally agree, which is why I was rather surprised when you
vigorously objected to my attempts to replace the remainder of the
main tree's superuser checks that completely block execution of
certain SQL functions with privilege grants. The parameters within
which you find explicit superuser checks tolerable are extremely murky
to me.Filesystem-level access seems reasonable to restrict to superusers.
I think in an ideal world we wouldn't have any hard-coded superuser()
checks at all, because all privileges would be able to be delegated in
a fine-grained manner. But even if I were to agree with you on this
point, you've argued for a fairly inconsistent application of that
principle. However, I do agree with your remarks later in the email
that it's best not to derail this thread to discuss that issue in
depth, so I'll shut up about this for now.
I specifically made a point to not question what was or wasn't
considered sensitive as it can certainly vary. The point I was making
is that if there's sensitive information there then we can exclude just
that information but allow a pg_dump-using user to see that a
subscription exists without it.
OK, apparently I misread your remarks. I thought you were arguing
that the data wasn't sensitive, which seemed like an odd take.
I don't think it's the purpose of pg_dump to reveal what objects
exist, but rather to create a dump file that can be used to recreate
the state of the database. To the extent that the user lacks
permissions necessary to dump the objects, they can't be dumped.
Maybe modifying subscriptions so that they can exist without a
connection and "half-dumping" them is better than not dumping them at
all, but my initial impression is to think that it's worse. A user
may be more likely to notice something that's missing altogether than
something that exists but in a non-working state. You mentioned
--no-role-passwords, but that's a nasty kludge in my book and I'm not
in favor of making something like that the default behavior. Peter's
approach looks like less work, and helps us get beta out the door.
I can't claim to be an expert on all of this, though.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/10/17 13:58, Peter Eisentraut wrote:
Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.
Patch for this part.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-pg_dump-Dump-subscriptions-by-default.patchinvalid/octet-stream; name=0001-pg_dump-Dump-subscriptions-by-default.patchDownload
From a1f0ed38d858c22d2621b2bba18f86d622f072e6 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 48db9cd08b..8c70ce3b6e 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -167,9 +167,10 @@ <title>Subscription</title>
</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 4f19b89232..53b5dd5239 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -756,15 +756,6 @@ <title>Options</title>
</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>
<para>
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 65a2f2307a..19bab693b7 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 @@
'--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 @@
'pg_dump',
'--no-sync',
"--file=$tempdir/clean.sql",
- '--include-subscriptions',
'-c',
'-d', 'postgres', # alternative way to specify database
], },
@@ -67,7 +65,6 @@
'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 @@
'pg_dump',
'--no-sync',
"--file=$tempdir/createdb.sql",
- '--include-subscriptions',
'-C',
'-R', # no-op, just for testing
'-v',
@@ -95,7 +91,6 @@
'pg_dump',
'--no-sync',
"--file=$tempdir/data_only.sql",
- '--include-subscriptions',
'-a',
'--superuser=test_superuser',
'--disable-triggers',
@@ -253,7 +248,6 @@
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 @@
with_oids => {
dump_cmd => [
'pg_dump', '--oids',
- '--include-subscriptions', '--no-sync',
+ '--no-sync',
"--file=$tempdir/with_oids.sql", 'postgres', ], },);
###############################################################
@@ -1405,7 +1399,7 @@
# 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 @@
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' => {
--
2.12.2
On 4/10/17 20:55, Stephen Frost wrote:
Proposal: Dump all subscriptions in DISABLED state. Remove current
pg_dump --no-subscription-connect option.I like this idea in general, I'm just not sure if it's the right answer
when we're talking about pg_upgrade. At a minimum, if we go with this
approach, we should produce a warning when subscriptions exist during
the pg_upgrade that the user will need to go re-enable them.
It's not clear what to do with a subscription after a dump/restore or a
pg_upgrade anyway. You can't just continue streaming where you left
off. Most likely, you will want to truncate the target tables and
resync them. In some cases, you might just accept the data gap and
continue streaming at the current state. But in any case you'll need to
decide based on what you're actually doing. Just telling the user, turn
it back on and you're ready to go isn't necessarily the right answer.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
By what day should the community look for your next update?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/11/17 23:41, Noah Misch wrote:
On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
By what day should the community look for your next update?
tomorrow
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/11/17 22:16, Peter Eisentraut wrote:
On 4/10/17 13:58, Peter Eisentraut wrote:
Proposal: Dump subscriptions if running as superuser. If not, check if
there are subscriptions and warn about that. Remove current pg_dump
--include-subscriptions option.Patch for this part.
And patch for the second part.
I'll commit those in a day or two if there are no new insights.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0002-pg_dump-Always-dump-subscriptions-NOCONNECT.patchinvalid/octet-stream; name=0002-pg_dump-Always-dump-subscriptions-NOCONNECT.patchDownload
From 82f6c494524954ba4cfd669b97944398e2b3b888 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 12 Apr 2017 22:12:30 -0400
Subject: [PATCH 2/2] pg_dump: Always dump subscriptions NOCONNECT
This removes the pg_dump option --no-subscription-connect and makes it
the default. Dumping a subscription so that it activates right away
when restored is not very useful, because the state of the publication
server is unclear.
Discussion: https://www.postgresql.org/message-id/e4fbfad5-c6ac-fd50-6777-18c84b34eb2f@2ndquadrant.com
---
doc/src/sgml/ref/pg_dump.sgml | 26 +++++++++++++-------------
src/bin/pg_dump/pg_backup.h | 1 -
src/bin/pg_dump/pg_dump.c | 22 ++--------------------
src/bin/pg_dump/pg_dump.h | 1 -
src/bin/pg_dump/t/002_pg_dump.pl | 4 ++--
5 files changed, 17 insertions(+), 37 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 53b5dd5239..6cf7e570ef 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -799,19 +799,6 @@ <title>Options</title>
</varlistentry>
<varlistentry>
- <term><option>--no-subscription-connect</option></term>
- <listitem>
- <para>
- When dumping logical replication subscriptions,
- generate <command>CREATE SUBSCRIPTION</command> commands that do not
- make remote connections for creating replication slot or initial table
- copy. That way, the dump can be restored without requiring network
- access to the remote servers.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
<term><option>--no-synchronized-snapshots</></term>
<listitem>
<para>
@@ -1235,6 +1222,19 @@ <title>Notes</title>
in cross-version cases, as it can prevent problems arising from varying
reserved-word lists in different <productname>PostgreSQL</> versions.
</para>
+
+ <para>
+ When dumping logical replication subscriptions,
+ <application>pg_dump</application> will generate <command>CREATE
+ SUBSCRIPTION</command> commands that use the <literal>NOCONNECT</literal>
+ option, so that restoring the subscription does not make remote connections
+ for creating a replication slot or for initial table copy. That way, the
+ dump can be restored without requiring network access to the remote
+ servers. It is then up to the user to reactivate the subscriptions in a
+ suitable way. If the involved hosts have changed, the connection
+ information might have to be changed. It might also be appropriate to
+ truncate the target tables before initiating a new full table copy.
+ </para>
</refsect1>
<refsect1 id="pg-dump-examples">
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 1d14b68983..08b883efb0 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -153,7 +153,6 @@ typedef struct _dumpOptions
int outputNoTablespaces;
int use_setsessauth;
int enable_row_security;
- int no_subscription_connect;
/* default, if no "inclusion" switches appear, is to dump everything */
bool include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dcefe975d8..14dc1b2423 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -353,7 +353,6 @@ main(int argc, char **argv)
{"strict-names", no_argument, &strict_names, 1},
{"use-set-session-authorization", no_argument, &dopt.use_setsessauth, 1},
{"no-security-labels", no_argument, &dopt.no_security_labels, 1},
- {"no-subscription-connect", no_argument, &dopt.no_subscription_connect, 1},
{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
{"no-sync", no_argument, NULL, 7},
@@ -951,7 +950,6 @@ help(const char *progname)
printf(_(" --if-exists use IF EXISTS when dropping objects\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"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
printf(_(" --no-unlogged-table-data do not dump unlogged table data\n"));
@@ -3669,7 +3667,6 @@ getSubscriptions(Archive *fout)
int i_oid;
int i_subname;
int i_rolname;
- int i_subenabled;
int i_subconninfo;
int i_subslotname;
int i_subpublications;
@@ -3702,7 +3699,7 @@ getSubscriptions(Archive *fout)
/* Get the subscriptions in current database. */
appendPQExpBuffer(query,
"SELECT s.tableoid, s.oid, s.subname,"
- "(%s s.subowner) AS rolname, s.subenabled, "
+ "(%s s.subowner) AS rolname, "
" s.subconninfo, s.subslotname, s.subpublications "
"FROM pg_catalog.pg_subscription s "
"WHERE s.subdbid = (SELECT oid FROM pg_catalog.pg_database"
@@ -3716,7 +3713,6 @@ getSubscriptions(Archive *fout)
i_oid = PQfnumber(res, "oid");
i_subname = PQfnumber(res, "subname");
i_rolname = PQfnumber(res, "rolname");
- i_subenabled = PQfnumber(res, "subenabled");
i_subconninfo = PQfnumber(res, "subconninfo");
i_subslotname = PQfnumber(res, "subslotname");
i_subpublications = PQfnumber(res, "subpublications");
@@ -3732,8 +3728,6 @@ getSubscriptions(Archive *fout)
AssignDumpId(&subinfo[i].dobj);
subinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_subname));
subinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
- subinfo[i].subenabled =
- (strcmp(PQgetvalue(res, i, i_subenabled), "t") == 0);
subinfo[i].subconninfo = pg_strdup(PQgetvalue(res, i, i_subconninfo));
subinfo[i].subslotname = pg_strdup(PQgetvalue(res, i, i_subslotname));
subinfo[i].subpublications =
@@ -3758,7 +3752,6 @@ getSubscriptions(Archive *fout)
static void
dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
{
- DumpOptions *dopt = fout->dopt;
PQExpBuffer delq;
PQExpBuffer query;
PQExpBuffer publications;
@@ -3799,19 +3792,8 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
appendPQExpBufferStr(publications, fmtId(pubnames[i]));
}
- appendPQExpBuffer(query, " PUBLICATION %s WITH (", publications->data);
-
- if (subinfo->subenabled)
- appendPQExpBufferStr(query, "ENABLED");
- else
- appendPQExpBufferStr(query, "DISABLED");
-
- appendPQExpBufferStr(query, ", SLOT NAME = ");
+ appendPQExpBuffer(query, " PUBLICATION %s WITH (NOCONNECT, SLOT NAME = ", publications->data);
appendStringLiteralAH(query, subinfo->subslotname, fout);
-
- if (dopt->no_subscription_connect)
- appendPQExpBufferStr(query, ", NOCONNECT");
-
appendPQExpBufferStr(query, ");\n");
ArchiveEntry(fout, subinfo->dobj.catId, subinfo->dobj.dumpId,
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 61097e6d99..ba85392f11 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -614,7 +614,6 @@ typedef struct _SubscriptionInfo
{
DumpableObject dobj;
char *rolname;
- bool subenabled;
char *subconninfo;
char *subslotname;
char *subpublications;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 1db3767f46..e0d1ce6232 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4303,9 +4303,9 @@
create_order => 50,
create_sql => 'CREATE SUBSCRIPTION sub1
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
- WITH (DISABLED, NOCONNECT);',
+ WITH (NOCONNECT);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (DISABLED, SLOT NAME = 'sub1');\E
+ \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (NOCONNECT, SLOT NAME = 'sub1');\E
/xm,
like => {
binary_upgrade => 1,
--
2.12.2
On 4/12/17 18:31, Peter Eisentraut wrote:
On 4/11/17 23:41, Noah Misch wrote:
On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
By what day should the community look for your next update?
tomorrow
Everything has been committed, and this thread can be closed.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 13, 2017 at 12:05 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 4/12/17 18:31, Peter Eisentraut wrote:
On 4/11/17 23:41, Noah Misch wrote:
On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
By what day should the community look for your next update?
tomorrow
Everything has been committed, and this thread can be closed.
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 13/04/17 18:11, Robert Haas wrote:
On Thu, Apr 13, 2017 at 12:05 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 4/12/17 18:31, Peter Eisentraut wrote:
On 4/11/17 23:41, Noah Misch wrote:
On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
By what day should the community look for your next update?
tomorrow
Everything has been committed, and this thread can be closed.
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.
+1, I'll look into writing patch for that
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/13/17 12:11, Robert Haas wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.
What purpose would that serve in practice? The subscriptions are now
dumped disabled, so if they hang around, there is not much impact.
Perhaps an option that also omits publications would make sense?
Or a general filtering facility based on the object tag?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 4/13/17 12:11, Robert Haas wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.What purpose would that serve in practice? The subscriptions are now
dumped disabled, so if they hang around, there is not much impact.
Will they be allowed to be created as a non-superuser when loading into
another cluster? Even if you might have superuser rights on one
cluster, you may not on another.
Perhaps an option that also omits publications would make sense?
Yes.
Or a general filtering facility based on the object tag?
Perhaps.. I'd hate for it to end up being overly complicated though.
The general '--no-security-lables', '--no-tablespaces' and other options
are pretty straight-forward and I think that's a good thing.
Thanks!
Stephen
On Thu, Apr 13, 2017 at 12:11:30PM -0400, Robert Haas wrote:
On Thu, Apr 13, 2017 at 12:05 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 4/12/17 18:31, Peter Eisentraut wrote:
On 4/11/17 23:41, Noah Misch wrote:
On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
On 4/9/17 22:16, Noah Misch wrote:
[Action required within three days. This is a generic notification.]
Patches have been posted. Discussion is still going on a bit.
By what day should the community look for your next update?
tomorrow
Everything has been committed, and this thread can be closed.
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/2/17 21:44, Noah Misch wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
I consider this item low priority and don't plan to work on it before
all the other open items under logical replication are addressed.
(Here, working on it would include thinking further about whether it is
necessary at all or what alternatives might look like.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, May 05, 2017 at 11:01:57AM -0400, Peter Eisentraut wrote:
On 5/2/17 21:44, Noah Misch wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.I consider this item low priority and don't plan to work on it before
all the other open items under logical replication are addressed.(Here, working on it would include thinking further about whether it is
necessary at all or what alternatives might look like.)
That's informative, but it's not a valid status update. This PostgreSQL 10
open item is past due for your status update. Kindly send a valid status
update within 24 hours. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, May 06, 2017 at 11:50:16AM -0700, Noah Misch wrote:
On Fri, May 05, 2017 at 11:01:57AM -0400, Peter Eisentraut wrote:
On 5/2/17 21:44, Noah Misch wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.I consider this item low priority and don't plan to work on it before
all the other open items under logical replication are addressed.(Here, working on it would include thinking further about whether it is
necessary at all or what alternatives might look like.)That's informative, but it's not a valid status update. This PostgreSQL 10
open item is past due for your status update. Kindly send a valid status
update within 24 hours. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due
for your status update. Please reacquaint yourself with the policy on open
item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and then reply immediately. If I do not hear from you by
2017-05-09 07:00 UTC, I will transfer this item to release management team
ownership without further notice.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/6/17 14:50, Noah Misch wrote:
I consider this item low priority and don't plan to work on it before
all the other open items under logical replication are addressed.(Here, working on it would include thinking further about whether it is
necessary at all or what alternatives might look like.)That's informative, but it's not a valid status update. This PostgreSQL 10
open item is past due for your status update. Kindly send a valid status
update within 24 hours. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
Fair enough. I have set myself a reminder to report back on May 30.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, May 6, 2017 at 12:01 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 5/2/17 21:44, Noah Misch wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.
Reading the thread for the first time, I am +1 on that. It feels more
natural to have subscriptions by default when taking a dump, and it is
useful as well to be able to override the default so as they are not
included.
On Tue, May 9, 2017 at 1:26 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 5/6/17 14:50, Noah Misch wrote:
I consider this item low priority and don't plan to work on it before
all the other open items under logical replication are addressed.(Here, working on it would include thinking further about whether it is
necessary at all or what alternatives might look like.)That's informative, but it's not a valid status update. This PostgreSQL 10
open item is past due for your status update. Kindly send a valid status
update within 24 hours. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.comFair enough. I have set myself a reminder to report back on May 30.
I think that it would be nice to fix that even before beta, so
attached is a patch to add --no-subscriptions to pg_dump, pg_dumpall
and pg_restore.
--
Michael
Attachments:
dump-no-subs.patchapplication/octet-stream; name=dump-no-subs.patchDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 6cf7e570ef..d326f08b07 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -799,6 +799,15 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--no-subscriptions</option></term>
+ <listitem>
+ <para>
+ Do not dump subscriptions.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-synchronized-snapshots</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 070b902487..60e67a2c7b 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -355,6 +355,15 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--no-subscriptions</option></term>
+ <listitem>
+ <para>
+ Do not dump subscriptions.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-sync</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 44f0515066..943378530b 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -592,6 +592,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--no-subscriptions</option></term>
+ <listitem>
+ <para>
+ Do not output commands to restore subscriptions, even if the archive
+ contains them.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-tablespaces</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 08b883efb0..fe3ee9ec8d 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -75,6 +75,7 @@ typedef struct _restoreOptions
int column_inserts;
int if_exists;
int no_security_labels; /* Skip security label entries */
+ int no_subscription; /* Skip subscription entries */
int strict_names;
const char *filename;
@@ -145,6 +146,7 @@ typedef struct _dumpOptions
int column_inserts;
int if_exists;
int no_security_labels;
+ int no_subscription;
int no_synchronized_snapshots;
int no_unlogged_table_data;
int serializable_deferrable;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index b622506a00..afa6f6460b 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -167,6 +167,7 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt)
dopt->disable_dollar_quoting = ropt->disable_dollar_quoting;
dopt->dump_inserts = ropt->dump_inserts;
dopt->no_security_labels = ropt->no_security_labels;
+ dopt->no_subscription = ropt->no_subscription;
dopt->lockWaitTimeout = ropt->lockWaitTimeout;
dopt->include_everything = ropt->include_everything;
dopt->enable_row_security = ropt->enable_row_security;
@@ -2795,6 +2796,10 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
if (ropt->no_security_labels && strcmp(te->desc, "SECURITY LABEL") == 0)
return 0;
+ /* If it's a subcription, maybe ignore it */
+ if (ropt->no_subscription && strcmp(te->desc, "SUBSCRIPTION") == 0)
+ return 0;
+
/* Ignore it if section is not to be dumped/restored */
switch (curSection)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index af84c25093..e6115af32d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -355,6 +355,7 @@ main(int argc, char **argv)
{"no-security-labels", no_argument, &dopt.no_security_labels, 1},
{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
+ {"no-subscriptions", no_argument, &dopt.no_subscription, 1},
{"no-sync", no_argument, NULL, 7},
{NULL, 0, NULL, 0}
@@ -862,6 +863,7 @@ main(int argc, char **argv)
ropt->disable_dollar_quoting = dopt.disable_dollar_quoting;
ropt->dump_inserts = dopt.dump_inserts;
ropt->no_security_labels = dopt.no_security_labels;
+ ropt->no_subscription = dopt.no_subscription;
ropt->lockWaitTimeout = dopt.lockWaitTimeout;
ropt->include_everything = dopt.include_everything;
ropt->enable_row_security = dopt.enable_row_security;
@@ -950,6 +952,7 @@ help(const char *progname)
printf(_(" --if-exists use IF EXISTS when dropping objects\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-subscriptions do not dump subscriptions\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
printf(_(" --no-unlogged-table-data do not dump unlogged table data\n"));
@@ -3674,6 +3677,7 @@ is_superuser(Archive *fout)
void
getSubscriptions(Archive *fout)
{
+ DumpOptions *dopt = fout->dopt;
PQExpBuffer query;
PGresult *res;
SubscriptionInfo *subinfo;
@@ -3688,7 +3692,7 @@ getSubscriptions(Archive *fout)
int i,
ntups;
- if (fout->remoteVersion < 100000)
+ if (dopt->no_subscription || fout->remoteVersion < 100000)
return;
if (!is_superuser(fout))
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 0bf5b7c666..d91c4e1cd3 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -75,6 +75,7 @@ static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
static int no_security_labels = 0;
+static int no_subscriptions = 0;
static int no_unlogged_table_data = 0;
static int no_role_passwords = 0;
static int server_version;
@@ -129,6 +130,7 @@ main(int argc, char *argv[])
{"role", required_argument, NULL, 3},
{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
{"no-security-labels", no_argument, &no_security_labels, 1},
+ {"no-subscriptions", no_argument, &no_subscriptions, 1},
{"no-sync", no_argument, NULL, 4},
{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
{"no-role-passwords", no_argument, &no_role_passwords, 1},
@@ -385,6 +387,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --use-set-session-authorization");
if (no_security_labels)
appendPQExpBufferStr(pgdumpopts, " --no-security-labels");
+ if (no_subscriptions)
+ appendPQExpBufferStr(pgdumpopts, " --no-subscriptions");
if (no_unlogged_table_data)
appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
@@ -591,6 +595,7 @@ help(void)
printf(_(" --if-exists use IF EXISTS when dropping objects\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-subscriptions do not dump subscriptions\n"));
printf(_(" --no-sync do not wait for changes to be written safely to disk\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
printf(_(" --no-unlogged-table-data do not dump unlogged table data\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index ddd79429b4..7710bed961 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -72,6 +72,7 @@ main(int argc, char **argv)
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
static int no_security_labels = 0;
+ static int no_subscriptions = 0;
static int strict_names = 0;
struct option cmdopts[] = {
@@ -118,6 +119,7 @@ main(int argc, char **argv)
{"strict-names", no_argument, &strict_names, 1},
{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
{"no-security-labels", no_argument, &no_security_labels, 1},
+ {"no-subscriptions", no_argument, &no_subscriptions, 1},
{NULL, 0, NULL, 0}
};
@@ -355,6 +357,7 @@ main(int argc, char **argv)
opts->noTablespace = outputNoTablespaces;
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ opts->no_subscription = no_subscriptions;
if (if_exists && !opts->dropSchema)
{
@@ -477,6 +480,7 @@ usage(const char *progname)
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
+ printf(_(" --no-subscriptions do not restore subscriptions\n"));
printf(_(" --no-tablespaces do not restore tablespace assignments\n"));
printf(_(" --section=SECTION restore named section (pre-data, data, or post-data)\n"));
printf(_(" --strict-names require table and/or schema include patterns to\n"
On 09/05/17 07:24, Michael Paquier wrote:
On Sat, May 6, 2017 at 12:01 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 5/2/17 21:44, Noah Misch wrote:
I wonder if we should have an --no-subscriptions option, now that they
are dumped by default, just like we have --no-blobs, --no-owner,
--no-password, --no-privileges, --no-acl, --no-tablespaces, and
--no-security-labels. It seems like there is probably a fairly large
use case for excluding subscriptions even if you have sufficient
permissions to dump them.Reading the thread for the first time, I am +1 on that. It feels more
natural to have subscriptions by default when taking a dump, and it is
useful as well to be able to override the default so as they are not
included.
Yes I agree. There was also talk of generalizing the --no-* stuff so we
could filter arbitrary objects which is something I would prefer as
solution, but at this point that's potential PG11 feature not PG10 one.
On Tue, May 9, 2017 at 1:26 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 5/6/17 14:50, Noah Misch wrote:
I consider this item low priority and don't plan to work on it before
all the other open items under logical replication are addressed.(Here, working on it would include thinking further about whether it is
necessary at all or what alternatives might look like.)That's informative, but it's not a valid status update. This PostgreSQL 10
open item is past due for your status update. Kindly send a valid status
update within 24 hours. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.comFair enough. I have set myself a reminder to report back on May 30.
I think that it would be nice to fix that even before beta, so
attached is a patch to add --no-subscriptions to pg_dump, pg_dumpall
and pg_restore.
Looks okay to me, it's simple enough patch.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/9/17 04:54, Petr Jelinek wrote:
I think that it would be nice to fix that even before beta, so
attached is a patch to add --no-subscriptions to pg_dump, pg_dumpall
and pg_restore.Looks okay to me, it's simple enough patch.
Committed, thanks.
(There was some inconsistent variable naming no_subscription vs
no_subscriptions, which I sorted out.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 10, 2017 at 12:00 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 5/9/17 04:54, Petr Jelinek wrote:
I think that it would be nice to fix that even before beta, so
attached is a patch to add --no-subscriptions to pg_dump, pg_dumpall
and pg_restore.Looks okay to me, it's simple enough patch.
Committed, thanks.
(There was some inconsistent variable naming no_subscription vs
no_subscriptions, which I sorted out.)
Thanks.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers