Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax. Logical replication
seems to have added a whole bunch of syntax that involves prefixing
words with "no". In various places, there's NODROP, NOREFRESH, NOCOPY
DATA, NOCONNECT, and NOPUBLISH. But "NO" is not an English prefix,
and there's no precedent of which I'm aware for such SQL syntax. In
most places, we've chosen to name the option and then the user set it
to "on" or "off". So for example you type EXPLAIN (ANALYZE, TIMING
OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
NOTIMING). I think most of the logical replication stuff could have
been done this way pretty easily, but for some reason it picked a
completely different approach.
--
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 2 May 2017 at 12:55, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax. Logical replication
seems to have added a whole bunch of syntax that involves prefixing
words with "no". In various places, there's NODROP, NOREFRESH, NOCOPY
DATA, NOCONNECT, and NOPUBLISH. But "NO" is not an English prefix,
and there's no precedent of which I'm aware for such SQL syntax. In
most places, we've chosen to name the option and then the user set it
to "on" or "off". So for example you type EXPLAIN (ANALYZE, TIMING
OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
NOTIMING). I think most of the logical replication stuff could have
been done this way pretty easily, but for some reason it picked a
completely different approach.
+1 for not upsetting my OCD.
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax. Logical replication
seems to have added a whole bunch of syntax that involves prefixing
words with "no". In various places, there's NODROP, NOREFRESH, NOCOPY
DATA, NOCONNECT, and NOPUBLISH. But "NO" is not an English prefix,
and there's no precedent of which I'm aware for such SQL syntax. In
most places, we've chosen to name the option and then the user set it
to "on" or "off". So for example you type EXPLAIN (ANALYZE, TIMING
OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
NOTIMING). I think most of the logical replication stuff could have
been done this way pretty easily, but for some reason it picked a
completely different approach.
I tend to agree with this criticism, but it's not quite true that we
don't do this anywhere else --- see CREATE USER for one example, where
we have monstrosities like NOBYPASSRLS. Still, at least those are single
words without arguments. "NODROP SLOT" is pretty ugly, not least
because if you want to claim CREATE USER as syntax precedent, you ought
to spell it NODROPSLOT.
Having said that, I doubt that anyone would argue that CREATE USER is
anything but legacy syntax, or that our more recent syntax designs aren't
better models to follow.
It's not quite too late to revisit the syntax of the log rep commands
... shall we add this as an open item?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/05/17 14:13, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax. Logical replication
seems to have added a whole bunch of syntax that involves prefixing
words with "no". In various places, there's NODROP, NOREFRESH, NOCOPY
DATA, NOCONNECT, and NOPUBLISH. But "NO" is not an English prefix,
and there's no precedent of which I'm aware for such SQL syntax. In
most places, we've chosen to name the option and then the user set it
to "on" or "off". So for example you type EXPLAIN (ANALYZE, TIMING
OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
NOTIMING). I think most of the logical replication stuff could have
been done this way pretty easily, but for some reason it picked a
completely different approach.I tend to agree with this criticism, but it's not quite true that we
don't do this anywhere else --- see CREATE USER for one example, where
we have monstrosities like NOBYPASSRLS. Still, at least those are single
words without arguments. "NODROP SLOT" is pretty ugly, not least
because if you want to claim CREATE USER as syntax precedent, you ought
to spell it NODROPSLOT.Having said that, I doubt that anyone would argue that CREATE USER is
anything but legacy syntax, or that our more recent syntax designs aren't
better models to follow.It's not quite too late to revisit the syntax of the log rep commands
... shall we add this as an open item?
I am happy to implement something different, it's quite trivial to
change. But I am not going to propose anything different as I can't
think of better syntax (if I could I would have done it). I don't like
the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
it also seems to not map very well to action (as opposed to output
option as it is in EXPLAIN). It might not be very close to SQL way but
that's because SQL way would be do not do any of those default actions
unless they are actually asked for (ie NODROP SLOT would be default and
DROP SLOT would be the option) but that's IMHO less user friendly.
--
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
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax.
Actually, this command has got much worse problems than whether you like
the spelling of its option: it shouldn't have an option in the first
place. I put it to you as an inviolable rule that no object DROP command
should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
If it does, then you don't know what to do when the object is recursed
to by a cascaded drop.
It's possible that we could allow exceptions to this rule for object types
that can never have any dependencies. But a subscription doesn't qualify
--- it's got an owner, so DROP OWNED BY already poses this problem. Looks
to me like it's got a dependency on a database, too. (BTW, if
subscriptions are per-database, why is pg_subscription a shared catalog?
There were some pretty schizophrenic decisions here.)So ISTM we need to get rid of the above-depicted syntax. We could instead
have what-to-do-with-the-slot as a property of the subscription,
established at CREATE SUBSCRIPTION and possibly changed later by ALTER
SUBSCRIPTION. Not quite sure what to call the option, maybe something
based on the concept of the subscription "owning" the slot.
I think this is a must-fix issue, and will put it on the open items
list.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/05/17 15:10, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax.
Actually, this command has got much worse problems than whether you like
the spelling of its option: it shouldn't have an option in the first
place. I put it to you as an inviolable rule that no object DROP command
should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
If it does, then you don't know what to do when the object is recursed
to by a cascaded drop.It's possible that we could allow exceptions to this rule for object types that can never have any dependencies. But a subscription doesn't qualify --- it's got an owner, so DROP OWNED BY already poses this problem. Looks to me like it's got a dependency on a database, too. (BTW, if subscriptions are per-database, why is pg_subscription a shared catalog? There were some pretty schizophrenic decisions here.)
Because otherwise we would need launcher process per database, not pretty.
So ISTM we need to get rid of the above-depicted syntax. We could instead
have what-to-do-with-the-slot as a property of the subscription,
established at CREATE SUBSCRIPTION and possibly changed later by ALTER
SUBSCRIPTION. Not quite sure what to call the option, maybe something
based on the concept of the subscription "owning" the slot.
So what do you do if the upstream does not exist anymore when you are
dropping subscription?
--
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 02/05/17 15:14, Petr Jelinek wrote:
On 02/05/17 15:10, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax.
Actually, this command has got much worse problems than whether you like
the spelling of its option: it shouldn't have an option in the first
place. I put it to you as an inviolable rule that no object DROP command
should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
If it does, then you don't know what to do when the object is recursed
to by a cascaded drop.It's possible that we could allow exceptions to this rule for object types that can never have any dependencies. But a subscription doesn't qualify --- it's got an owner, so DROP OWNED BY already poses this problem. Looks to me like it's got a dependency on a database, too. (BTW, if subscriptions are per-database, why is pg_subscription a shared catalog? There were some pretty schizophrenic decisions here.)Because otherwise we would need launcher process per database, not pretty.
So ISTM we need to get rid of the above-depicted syntax. We could instead
have what-to-do-with-the-slot as a property of the subscription,
established at CREATE SUBSCRIPTION and possibly changed later by ALTER
SUBSCRIPTION. Not quite sure what to call the option, maybe something
based on the concept of the subscription "owning" the slot.So what do you do if the upstream does not exist anymore when you are
dropping subscription?
Let me expand, if we don't drop the slot by default when dropping
subscription, we'll have a lot of users with dead slots laying around
holding back WAL/catalog_xmin, that's really bad. If we do drop by
default like now, we need option to not do that, neither RESTRICT, nor
CASCADE fit 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
Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
Let me expand, if we don't drop the slot by default when dropping
subscription, we'll have a lot of users with dead slots laying around
holding back WAL/catalog_xmin, that's really bad. If we do drop by
default like now, we need option to not do that, neither RESTRICT, nor
CASCADE fit that.
I'm not sure which part of "you can't have an option in DROP" isn't
clear to you. Whatever the default behavior is always has to work,
because that is what's going to happen during DROP OWNED BY, or
DROP DATABASE. If you want more than one behavior during DROP,
you need to drive that off something represented as a persistent
property of the object, not off an option in the DROP command.
I agree that RESTRICT/CASCADE don't fit this, unless the model
is that the slot is always owned by the subscription, which might
be too restrictive.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/02/2017 05:13 AM, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
Having said that, I doubt that anyone would argue that CREATE USER is
anything but legacy syntax, or that our more recent syntax designs aren't
better models to follow.It's not quite too late to revisit the syntax of the log rep commands
... shall we add this as an open item?
I would think so. Just in reading this, even if we keep a similar syntax
it should be DROP SLOT NO or DROP SLOT FALSE.
JD
regards, tom lane
--
Command Prompt, Inc. http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/05/17 15:31, Tom Lane wrote:
Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
Let me expand, if we don't drop the slot by default when dropping
subscription, we'll have a lot of users with dead slots laying around
holding back WAL/catalog_xmin, that's really bad. If we do drop by
default like now, we need option to not do that, neither RESTRICT, nor
CASCADE fit that.I'm not sure which part of "you can't have an option in DROP" isn't
clear to you. Whatever the default behavior is always has to work,
because that is what's going to happen during DROP OWNED BY, or
DROP DATABASE.
You are saying it like there was some guarantee that those commands
can't fail because of other objects. AFAIK for example drop database can
trivially fail just because there is replication slot in it before PG10
(IIRC Craig managed to fix that in 10 for unrelated reasons).
If you want more than one behavior during DROP,
you need to drive that off something represented as a persistent
property of the object, not off an option in the DROP command.
I don't see how changing behavior as object property will change
anything in terms of not failing to cascade. If you use CREATE or ALTER
to say that subscription must drop slot and that fails then the cascade
will still fail so then you need to run ALTER again to change that
property. I fail to see how that's easier than running the DROP directly.
So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.
We should also not create the slot (at least by default) on CREATE
SUBSCRIPTION to have some symmetry.
--
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
Petr Jelinek wrote:
So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.
I don't understand why isn't the default behavior to unconditionally
drop the slot. Why do we ever want the slot to be kept?
--
�lvaro Herrera https://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, May 2, 2017 at 11:49 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Petr Jelinek wrote:
So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.I don't understand why isn't the default behavior to unconditionally
drop the slot. Why do we ever want the slot to be kept?
What if the remote server doesn't exist any more?
--
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 02/05/17 18:00, Robert Haas wrote:
On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Petr Jelinek wrote:
So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.I don't understand why isn't the default behavior to unconditionally
drop the slot. Why do we ever want the slot to be kept?What if the remote server doesn't exist any more?
Or what if the slot is used by other subscription (because you restored
dump containing subscriptions to another server for example), or you
have server that does not have outside network access anymore, or many
other reasons.
--
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
Petr Jelinek wrote:
On 02/05/17 18:00, Robert Haas wrote:
On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Petr Jelinek wrote:
So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.I don't understand why isn't the default behavior to unconditionally
drop the slot. Why do we ever want the slot to be kept?What if the remote server doesn't exist any more?
Or what if the slot is used by other subscription (because you restored
dump containing subscriptions to another server for example), or you
have server that does not have outside network access anymore, or many
other reasons.
So there are two different scenarios: one is that you expect the slot
drop to fail for whatever reason; the other is that you want the slot to
be kept because it's needed for something else. Maybe those two should
be considered separately.
1) keep the slot because it's needed for something else.
I see two options:
a) change the something else so that it uses another slot with the
same location. Do we have some sort of "clone slot" operation?
b) have an option to dissociate the slot from the subscription prior
to the drop.
2) don't drop because we know it won't work. I see two options:
c) ignore a drop slot failure, i.e. don't cause a transaction abort.
An easy way to implement this is just add a PG_TRY block, but we
dislike adding those and not re-throwing the error.
d) rethink drop slot completely; maybe instead of doing it
immediately, it should be a separate task, so we first close the
current transaction (which dropped the subscription) and then we open
a second one to drop the slot, so that if the drop slot fails, the
subscription does not come back to life.
--
�lvaro Herrera https://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, May 2, 2017 at 12:25 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
2) don't drop because we know it won't work. I see two options:
c) ignore a drop slot failure, i.e. don't cause a transaction abort.
An easy way to implement this is just add a PG_TRY block, but we
dislike adding those and not re-throwing the error.
Dislike doesn't seem like the right word. Unless you rollback a
(sub)transaction, none of the cleanup that would normally do is done,
so you might leak buffer pins, memory, or other resources. Unless the
code that can be run in the TRY/CATCH block is sufficiently restricted
as to make that a non-issue, which is rarely the case, it's not going
to work reliably at all. I wonder why this API was even designed in a
way that made not re-throwing the error an option.
(I've wondered whether we should have some kind of mini-transaction
that is cheaper to abort but does only a critical subset of the
cleanup, but I haven't been able to figure out how you'd know whether
you only need to blow up the mini-transaction or whether you need to
kill the enclosing real (sub)transaction.)
d) rethink drop slot completely; maybe instead of doing it
immediately, it should be a separate task, so we first close the
current transaction (which dropped the subscription) and then we open
a second one to drop the slot, so that if the drop slot fails, the
subscription does not come back to life.
Something like this might work, although it's not clear how it
interacts with DROP .. CASCADE. See
/messages/by-id/CA+Tgmob_hy0uQS9vq_9rDBgjpww3D3jBZ6twAKZOwaZigo4C3g@mail.gmail.com
for a very related point about adding subscriptions.
--
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 Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
I am happy to implement something different, it's quite trivial to
change. But I am not going to propose anything different as I can't
think of better syntax (if I could I would have done it). I don't like
the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
it also seems to not map very well to action (as opposed to output
option as it is in EXPLAIN). It might not be very close to SQL way but
that's because SQL way would be do not do any of those default actions
unless they are actually asked for (ie NODROP SLOT would be default and
DROP SLOT would be the option) but that's IMHO less user friendly.
So the cases where this "NO" prefixing comes up are:
1. CREATE SUBSCRIPTION
<phrase>where <replaceable class="PARAMETER">option</replaceable> can
be:</phrase>
| ENABLED | DISABLED
| CREATE SLOT | NOCREATE SLOT
| SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
| COPY DATA | NOCOPY DATA
| SYNCHRONOUS_COMMIT = <replaceable
class="PARAMETER">synchronous_commit</replaceable>
| NOCONNECT
I think it would have been a lot better to use the extensible options
syntax for this instead of inventing something new that's not even
consistent with itself. You've got SYNCHRONOUS_COMMIT with a hyphen
and CREATE SLOT with no hyphen and NOCOPY DATA with no hyphen and a
space left out. With the extensible options syntax, this would be
(enabled true/false, create_slot true/false, slot_name whatever,
synchronous_commit true/false, connect true/false). If we're going to
keep the present monstrosity, we can I think still change NOCONNECT to
NO CONNECT, but there's no fixing NOCOPY DATA in this syntax model.
2. ALTER SUBSCRIPTION
ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] {
REFRESH WITH ( puboption [, ... ] ) | NOREFRESH }
There is no obvious reason why this could not have been spelled NO
REFRESH instead of adding a new keyword.
3. DROP SUBSCRIPTION
DROP SUBSCRIPTION [ IF EXISTS ] name [ DROP SLOT | NODROP SLOT ]
This is where we started, and I have nothing to add to what I (and
Tom) have already said.
4. CREATE PUBLICATION
CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
[ FOR TABLE [ ONLY ] <replaceable
class="parameter">table_name</replaceable> [ * ] [, ...]
| FOR ALL TABLES ]
[ WITH ( <replaceable class="parameter">option</replaceable> [, ... ] ) ]
<phrase>where <replaceable class="parameter">option</replaceable> can
be:</phrase>
PUBLISH INSERT | NOPUBLISH INSERT
| PUBLISH UPDATE | NOPUBLISH UPDATE
| PUBLISH DELETE | NOPUBLISH DELETE
Again, the extensible options syntax like we use for EXPLAIN would
have been better here. You could have said (publish_insert
true/false, publish_update true/false, publish_delete true/false), for
instance, or combined them into a single option like (publish
'insert,update') to omit deletes.
So it doesn't actually look hard to get rid of all of the NO prefixes.
--
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 02/05/17 19:42, Robert Haas wrote:
On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:I am happy to implement something different, it's quite trivial to
change. But I am not going to propose anything different as I can't
think of better syntax (if I could I would have done it). I don't like
the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
it also seems to not map very well to action (as opposed to output
option as it is in EXPLAIN). It might not be very close to SQL way but
that's because SQL way would be do not do any of those default actions
unless they are actually asked for (ie NODROP SLOT would be default and
DROP SLOT would be the option) but that's IMHO less user friendly.So the cases where this "NO" prefixing comes up are:
1. CREATE SUBSCRIPTION
<phrase>where <replaceable class="PARAMETER">option</replaceable> can
be:</phrase>| ENABLED | DISABLED
| CREATE SLOT | NOCREATE SLOT
| SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
| COPY DATA | NOCOPY DATA
| SYNCHRONOUS_COMMIT = <replaceable
class="PARAMETER">synchronous_commit</replaceable>
| NOCONNECTI think it would have been a lot better to use the extensible options
syntax for this instead of inventing something new that's not even
consistent with itself.
I am not sure what you mean by this, we always have to invent option
names if they are new options, even if we use generic options (which I
guess is what you mean by "extensible options syntax"). I used the
definitions instead of generic options, this means that the supported
syntax also includes COPY DATA = true/false, CREATE SLOT = true/false
etc, the NO* are just shorthands, it's quite simple to remove those.
You've got SYNCHRONOUS_COMMIT with a hyphen
and CREATE SLOT with no hyphen and NOCOPY DATA with no hyphen and a
space left out. With the extensible options syntax, this would be
(enabled true/false, create_slot true/false, slot_name whatever,
synchronous_commit true/false, connect true/false). If we're going to
keep the present monstrosity, we can I think still change NOCONNECT to
NO CONNECT, but there's no fixing NOCOPY DATA in this syntax model.
See above.
2. ALTER SUBSCRIPTION
ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] {
REFRESH WITH ( puboption [, ... ] ) | NOREFRESH }There is no obvious reason why this could not have been spelled NO
REFRESH instead of adding a new keyword.3. DROP SUBSCRIPTION
DROP SUBSCRIPTION [ IF EXISTS ] name [ DROP SLOT | NODROP SLOT ]
This is where we started, and I have nothing to add to what I (and
Tom) have already said.4. CREATE PUBLICATION
CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
[ FOR TABLE [ ONLY ] <replaceable
class="parameter">table_name</replaceable> [ * ] [, ...]
| FOR ALL TABLES ]
[ WITH ( <replaceable class="parameter">option</replaceable> [, ... ] ) ]<phrase>where <replaceable class="parameter">option</replaceable> can
be:</phrase>PUBLISH INSERT | NOPUBLISH INSERT
| PUBLISH UPDATE | NOPUBLISH UPDATE
| PUBLISH DELETE | NOPUBLISH DELETEAgain, the extensible options syntax like we use for EXPLAIN would
have been better here. You could have said (publish_insert
true/false, publish_update true/false, publish_delete true/false), for
instance, or combined them into a single option like (publish
'insert,update') to omit deletes.So it doesn't actually look hard to get rid of all of the NO prefixes.
That sounds okay. I know PeterE didn't like the lower case and
underscore separated words for options in the original patch, so I'd
like to hear his opinion on this. I am not sure how much advantage is
there in removing the '=' in between the key and value. That's the main
difference between generic options and definitions (well and definitions
can have 2 words for key, but that's something I have added anyway), I
don't really understand why we have both and use one for some commends
and the other for others btw.
--
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 Tue, May 2, 2017 at 3:02 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
That sounds okay. I know PeterE didn't like the lower case and
underscore separated words for options in the original patch, so I'd
like to hear his opinion on this. I am not sure how much advantage is
there in removing the '=' in between the key and value. That's the main
difference between generic options and definitions (well and definitions
can have 2 words for key, but that's something I have added anyway), I
don't really understand why we have both and use one for some commends
and the other for others btw.
I don't care all *that* much about the equals sign, although I think
it's marginally more elegant without it, and VACUUM, EXPLAIN, and COPY
all do it that way. But I think allowing two-word names is just a
pile of parser nastiness that we'd be far better off without. If you
use the same syntax as VACUUM, EXPLAIN, and COPY, all options are a
single identifier. If it's got to be multiple words, they can be
separated by underscores. But with what you've got right now, it's
sometimes one identifier even when it's two words, and sometimes two
identifiers. When it's three words, it's two identifiers, with two of
them concatenated.
--
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 Haas wrote:
On Tue, May 2, 2017 at 12:25 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:2) don't drop because we know it won't work. I see two options:
c) ignore a drop slot failure, i.e. don't cause a transaction abort.
An easy way to implement this is just add a PG_TRY block, but we
dislike adding those and not re-throwing the error.Dislike doesn't seem like the right word. Unless you rollback a
(sub)transaction, none of the cleanup that would normally do is done,
True. So one possible implementation is that we open a subtransaction
before dropping the slot, and we abort it if things go south. This is a
bit slower, but not critically so.
--
�lvaro Herrera https://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, May 2, 2017 at 5:15 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Robert Haas wrote:
On Tue, May 2, 2017 at 12:25 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:2) don't drop because we know it won't work. I see two options:
c) ignore a drop slot failure, i.e. don't cause a transaction abort.
An easy way to implement this is just add a PG_TRY block, but we
dislike adding those and not re-throwing the error.Dislike doesn't seem like the right word. Unless you rollback a
(sub)transaction, none of the cleanup that would normally do is done,True. So one possible implementation is that we open a subtransaction
before dropping the slot, and we abort it if things go south. This is a
bit slower, but not critically so.
I think that could work. Subtransaction abort isn't as fast as I
would sometimes like, but for a DDL command the overhead is pretty
insignificant.
--
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 02/05/17 22:40, Robert Haas wrote:
On Tue, May 2, 2017 at 3:02 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:That sounds okay. I know PeterE didn't like the lower case and
underscore separated words for options in the original patch, so I'd
like to hear his opinion on this. I am not sure how much advantage is
there in removing the '=' in between the key and value. That's the main
difference between generic options and definitions (well and definitions
can have 2 words for key, but that's something I have added anyway), I
don't really understand why we have both and use one for some commends
and the other for others btw.I don't care all *that* much about the equals sign, although I think
it's marginally more elegant without it, and VACUUM, EXPLAIN, and COPY
all do it that way. But I think allowing two-word names is just a
pile of parser nastiness that we'd be far better off without. If you
use the same syntax as VACUUM, EXPLAIN, and COPY, all options are a
single identifier.
Ok, Let me be clear, I actually happen to agree with your proposal. The
reason I am moaning is that I always seem to find myself doing tons of
mechanical work to rewrite some cosmetic aspect of some patch based on
which committer is paying attention in a specific week. So while I am
for doing exactly what you proposed, I'd like to see couple of +1s first
(Peter?) since I don't want to rewrite it to something different again
and it's also long past freeze.
To repeat the proposal:
- change the WITH (...) clauses in subscription/publication commands to:
(create_slot true/false, connect true/false, slot_name 'something',
copy_data true/false, etc)
- change the NOREFRESH to NO REFRESH in ALTER SUBSCRIPTION name SET
PUBLICATION (btw I originally had SKIP REFRESH there but changed it to
NOREFRESH for consistency with the other NO* stuff, wonder if SKIP would
sound more english).
- change the (publish insert/nopublish insert/publish update/nopublish
update), etc options to (publish 'update,insert').
And one question, if we are not using the definitions (key = value)
should we keep the WITH keyword in the syntax, would it be confusing?
If it's got to be multiple words, they can be
separated by underscores. But with what you've got right now, it's
sometimes one identifier even when it's two words, and sometimes two
identifiers.
The main inconsistency is the synchronous_commit which is that way to
make it clear it's same as the GUC it changes, but looks like that was a
mistake.
--
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 02/05/17 18:25, Alvaro Herrera wrote:
Petr Jelinek wrote:
On 02/05/17 18:00, Robert Haas wrote:
On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Petr Jelinek wrote:
So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.I don't understand why isn't the default behavior to unconditionally
drop the slot. Why do we ever want the slot to be kept?What if the remote server doesn't exist any more?
Or what if the slot is used by other subscription (because you restored
dump containing subscriptions to another server for example), or you
have server that does not have outside network access anymore, or many
other reasons.So there are two different scenarios: one is that you expect the slot
drop to fail for whatever reason; the other is that you want the slot to
be kept because it's needed for something else. Maybe those two should
be considered separately.1) keep the slot because it's needed for something else.
I see two options:
a) change the something else so that it uses another slot with the
same location. Do we have some sort of "clone slot" operation?
Nope.
b) have an option to dissociate the slot from the subscription prior
to the drop.
We do have ALTER SUBSCRIPTION bla WITH (SLOT NAME = 'something') so this
is definitely doable but ALTER SUBSCRIPTION bla WITH (SLOT NAME = '') is
not very nice syntax, maybe something like RESET SLOT NAME?
2) don't drop because we know it won't work. I see two options:
c) ignore a drop slot failure, i.e. don't cause a transaction abort.
An easy way to implement this is just add a PG_TRY block, but we
dislike adding those and not re-throwing the error.
d) rethink drop slot completely; maybe instead of doing it
immediately, it should be a separate task, so we first close the
current transaction (which dropped the subscription) and then we open
a second one to drop the slot, so that if the drop slot fails, the
subscription does not come back to life.
Yeah I was thinking about ignoring failures with WARNING as well, but
never seemed right because it would not solve the case 1), but I didn't
think of b) which might solve it.
I was also thinking on how to map the behavior to RESTRICT vs CASCADE,
wonder if RESTRICT should check for slot existence and refuse to drop if
the slot exists (question is then should it bail on connection failure
or ignore it?) and CASCADE should try to drop the slot and only warn on
failure then.
--
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 02/05/17 16:14, Petr Jelinek wrote:
On 02/05/17 15:31, Tom Lane wrote:
Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
Let me expand, if we don't drop the slot by default when dropping
subscription, we'll have a lot of users with dead slots laying around
holding back WAL/catalog_xmin, that's really bad. If we do drop by
default like now, we need option to not do that, neither RESTRICT, nor
CASCADE fit that.I'm not sure which part of "you can't have an option in DROP" isn't
clear to you. Whatever the default behavior is always has to work,
because that is what's going to happen during DROP OWNED BY, or
DROP DATABASE.You are saying it like there was some guarantee that those commands
can't fail because of other objects. AFAIK for example drop database can
trivially fail just because there is replication slot in it before PG10
(IIRC Craig managed to fix that in 10 for unrelated reasons).
Btw looks like I already forgot how this stuff behaves. Existence of
subscription currently also prevents DROP DATABASE (for same reason
active backends do).
DROP OWNED BY ignores SUBSCRIPTION too now, although I think it might
not be strictly necessary to stay that way.
But if we keep this behavior then the point about these two commands
cascading to subscriptions is largely moot.
--
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 Wed, May 3, 2017 at 12:38 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
Ok, Let me be clear, I actually happen to agree with your proposal. The
reason I am moaning is that I always seem to find myself doing tons of
mechanical work to rewrite some cosmetic aspect of some patch based on
which committer is paying attention in a specific week. So while I am
for doing exactly what you proposed, I'd like to see couple of +1s first
(Peter?) since I don't want to rewrite it to something different again
and it's also long past freeze.
So, Tom Lane and Thom Brown and Josh Drake all seemed generally in
favor of cleaning this up. Perhaps they could opine on this
particular proposal.
To repeat the proposal:
- change the WITH (...) clauses in subscription/publication commands to:
(create_slot true/false, connect true/false, slot_name 'something',
copy_data true/false, etc)- change the NOREFRESH to NO REFRESH in ALTER SUBSCRIPTION name SET
PUBLICATION (btw I originally had SKIP REFRESH there but changed it to
NOREFRESH for consistency with the other NO* stuff, wonder if SKIP would
sound more english).- change the (publish insert/nopublish insert/publish update/nopublish
update), etc options to (publish 'update,insert').And one question, if we are not using the definitions (key = value)
should we keep the WITH keyword in the syntax, would it be confusing?
No opinion on that.
--
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 Haas <robertmhaas@gmail.com> writes:
On Wed, May 3, 2017 at 12:38 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:Ok, Let me be clear, I actually happen to agree with your proposal. The
reason I am moaning is that I always seem to find myself doing tons of
mechanical work to rewrite some cosmetic aspect of some patch based on
which committer is paying attention in a specific week. So while I am
for doing exactly what you proposed, I'd like to see couple of +1s first
(Peter?) since I don't want to rewrite it to something different again
and it's also long past freeze.
So, Tom Lane and Thom Brown and Josh Drake all seemed generally in
favor of cleaning this up. Perhaps they could opine on this
particular proposal.
It seems like there's some remaining indecision between "make it look
like the options in EXPLAIN, VACUUM, etc" and "make it look like the
WITH options found in some other statements". I do not have a strong
opinion which one to do, but I'd definitely say that you should use WITH
in the latter case but not in the former. I think this mostly boils down
to whether to use "=" or not; you've got "not" in the proposal, which
means you are following the EXPLAIN precedent and should not use WITH.
I'm okay with the other specifics mentioned.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/05/17 23:29, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, May 3, 2017 at 12:38 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:Ok, Let me be clear, I actually happen to agree with your proposal. The
reason I am moaning is that I always seem to find myself doing tons of
mechanical work to rewrite some cosmetic aspect of some patch based on
which committer is paying attention in a specific week. So while I am
for doing exactly what you proposed, I'd like to see couple of +1s first
(Peter?) since I don't want to rewrite it to something different again
and it's also long past freeze.So, Tom Lane and Thom Brown and Josh Drake all seemed generally in
favor of cleaning this up. Perhaps they could opine on this
particular proposal.It seems like there's some remaining indecision between "make it look
like the options in EXPLAIN, VACUUM, etc" and "make it look like the
WITH options found in some other statements". I do not have a strong
opinion which one to do, but I'd definitely say that you should use WITH
in the latter case but not in the former. I think this mostly boils down
to whether to use "=" or not; you've got "not" in the proposal, which
means you are following the EXPLAIN precedent and should not use WITH.
Okay, here is my initial attempt on changing this. I opted for WITH and
"=" as I like it slightly better (also the generic_options expect values
to be quoted which I don't like and then I would have to again invent
something like generic_options but not quite the same).
Most of the changes go to doc and tests, not that much code has changed
as I already used the definiton (which is the parser's name for these
WITH things). Except that I removed the NO shorthands and changed
publish_insert,etc to publish = 'insert,...'. I also changed the
NOREFRESH to SKIP REFRESH.
I didn't touch the DROP SUBSCRIPTION slot handling so far, that needs to
be separate patch as there is behavior change there, while this is
purely cosmetic and IMHO it's better to not mix those. (I plan to send
patch for that which changes the behavior heavily soonish as well)
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
Rework-the-options-for-logical-replication.patchbinary/octet-stream; name=Rework-the-options-for-logical-replication.patchDownload
From 8e1d58598a429f0ba0d68ba37a562678de1dfe4e Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Wed, 3 May 2017 11:39:22 +0200
Subject: [PATCH] Rework the options for logical replication
Use similar option style as other statements which use WITH clause for
options.
---
doc/src/sgml/ref/alter_publication.sgml | 25 +--
doc/src/sgml/ref/alter_subscription.sgml | 72 ++++---
doc/src/sgml/ref/create_publication.sgml | 57 +++---
doc/src/sgml/ref/create_subscription.sgml | 210 +++++++++++----------
src/backend/commands/publicationcmds.c | 133 +++++--------
src/backend/commands/subscriptioncmds.c | 42 +----
src/backend/parser/gram.y | 8 +-
src/bin/pg_dump/pg_dump.c | 36 ++--
src/bin/pg_dump/t/002_pg_dump.pl | 12 +-
src/include/parser/kwlist.h | 1 -
.../dummy_seclabel/expected/dummy_seclabel.out | 2 +-
.../modules/dummy_seclabel/sql/dummy_seclabel.sql | 2 +-
src/test/regress/expected/object_address.out | 2 +-
src/test/regress/expected/publication.out | 10 +-
src/test/regress/expected/subscription.out | 20 +-
src/test/regress/sql/object_address.sql | 2 +-
src/test/regress/sql/publication.sql | 10 +-
src/test/regress/sql/subscription.sql | 18 +-
src/test/subscription/t/001_rep_changes.pl | 8 +-
src/test/subscription/t/002_types.pl | 2 +-
src/test/subscription/t/003_constraints.pl | 2 +-
src/test/subscription/t/004_sync.pl | 2 +-
22 files changed, 311 insertions(+), 365 deletions(-)
diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml
index 05bd57d..00e5cfe 100644
--- a/doc/src/sgml/ref/alter_publication.sgml
+++ b/doc/src/sgml/ref/alter_publication.sgml
@@ -21,14 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">option</replaceable> [, ... ] )
-
-<phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
-
- PUBLISH INSERT | NOPUBLISH INSERT
- | PUBLISH UPDATE | NOPUBLISH UPDATE
- | PUBLISH DELETE | NOPUBLISH DELETE
-
+ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> ADD TABLE [ ONLY ] <replaceable class="PARAMETER">table_name</replaceable> [ * ] [, ...]
ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> SET TABLE [ ONLY ] <replaceable class="PARAMETER">table_name</replaceable> [ * ] [, ...]
ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> DROP TABLE [ ONLY ] <replaceable class="PARAMETER">table_name</replaceable> [ * ] [, ...]
@@ -44,8 +37,7 @@ ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> RENAME TO <r
The first variant of this command listed in the synopsis can change
all of the publication properties specified in
<xref linkend="sql-createpublication">. Properties not mentioned in the
- command retain their previous settings. Database superusers can change any
- of these settings for any role.
+ command retain their previous settings.
</para>
<para>
@@ -80,15 +72,10 @@ ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> RENAME TO <r
</varlistentry>
<varlistentry>
- <term><literal>PUBLISH INSERT</literal></term>
- <term><literal>NOPUBLISH INSERT</literal></term>
- <term><literal>PUBLISH UPDATE</literal></term>
- <term><literal>NOPUBLISH UPDATE</literal></term>
- <term><literal>PUBLISH DELETE</literal></term>
- <term><literal>NOPUBLISH DELETE</literal></term>
+ <term><literal>publish</literal></term>
<listitem>
<para>
- These clauses alter properties originally set by
+ This clause alters property originally set by
<xref linkend="SQL-CREATEPUBLICATION">. See there for more information.
</para>
</listitem>
@@ -131,9 +118,9 @@ ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> RENAME TO <r
<title>Examples</title>
<para>
- Change the publication to not publish inserts:
+ Change the publication to publish only deletes and updates:
<programlisting>
-ALTER PUBLICATION noinsert WITH (NOPUBLISH INSERT);
+ALTER PUBLICATION noinsert OPTIONS (publish 'update, delete');
</programlisting>
</para>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 5dae4ae..57652d1 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -21,20 +21,9 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">suboption</replaceable> [, ... ] )
-
-<phrase>where <replaceable class="PARAMETER">suboption</replaceable> can be:</phrase>
-
- SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
- | SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable>
-
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] { REFRESH WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] ) | NOREFRESH }
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> REFRESH PUBLICATION [ WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] ) ]
-
-<phrase>where <replaceable class="PARAMETER">puboption</replaceable> can be:</phrase>
-
- COPY DATA | NOCOPY DATA
-
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] { REFRESH WITH ( <replaceable class="PARAMETER">refresh_option</replaceable> <replaceable class="PARAMETER">value</replaceable> [, ... ] ) | SKIP REFRESH }
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> REFRESH PUBLICATION [ WITH ( <replaceable class="PARAMETER">refresh_option</replaceable> <replaceable class="PARAMETER">value</replaceable> [, ... ] ) ]
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> CONNECTION '<replaceable>conninfo</replaceable>'
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> ENABLE
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> DISABLE
@@ -73,11 +62,9 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
<varlistentry>
<term><literal>CONNECTION '<replaceable class="parameter">conninfo</replaceable>'</literal></term>
- <term><literal>SLOT NAME = <replaceable class="parameter">slot_name</replaceable></literal></term>
- <term><literal>SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable></literal></term>
<listitem>
<para>
- These clauses alter properties originally set by
+ This clause alter property originally set by
<xref linkend="SQL-CREATESUBSCRIPTION">. See there for more
information.
</para>
@@ -85,6 +72,18 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
</varlistentry>
<varlistentry>
+ <term><literal>WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )</literal></term>
+ <listitem>
+ <para>
+ This clause alter property originally set by
+ <xref linkend="SQL-CREATESUBSCRIPTION">. See there for more
+ information. The allowed options are <literal>slot_name</literal> and
+ <literal>synchronous_commit</literal>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>SET PUBLICATION <replaceable class="parameter">publication_name</replaceable></literal></term>
<listitem>
<para>
@@ -97,7 +96,12 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
<literal>NOREFRESH</literal> is specified, the comamnd will not try to
refresh table information.
</para>
- </listitem>
+ <para>
+ The <literal>refresh_option</literal> is an additional option for the
+ refresh operation. See
+ <xref linkend="sql-altersubscription-refresh-options"> for details.
+ </para>
+ </listitem>
</varlistentry>
<varlistentry>
@@ -110,10 +114,9 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
since <command>CREATE SUBSCRIPTION</command>.
</para>
<para>
- The <literal>COPY DATA</literal> and <literal>NOCOPY DATA</literal>
- options specify if the existing data in the publications that are being
- subscribed to should be copied. <literal>COPY DATA</literal> is the
- default.
+ The <literal>refresh_option</literal> is an additional option for the
+ refresh operation. See
+ <xref linkend="sql-altersubscription-refresh-options"> for details.
</para>
</listitem>
</varlistentry>
@@ -156,6 +159,31 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
</listitem>
</varlistentry>
</variablelist>
+
+ <refsect2 id="sql-altersubscription-refresh-options">
+ <title id="sql-altersubscription-refresh-options-title">Refresh options</title>
+
+ <para>
+ The optional <literal>WITH</> clause for
+ <literal>SET PUBLICATION</literal> and
+ <literal>REFRESH PUBLICATION</literal> clauses specifies behavior of the
+ refresh operation. The supported options are:
+ </para>
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>copy_data</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies if the existing data in the publications that are being
+ subscribed to should be copied once the replication starts.
+ The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect2>
+
</refsect1>
<refsect1>
diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml
index 521376e..76f4250 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -24,13 +24,8 @@ PostgreSQL documentation
CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
[ FOR TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [, ...]
| FOR ALL TABLES ]
- [ WITH ( <replaceable class="parameter">option</replaceable> [, ... ] ) ]
+ [ WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
-<phrase>where <replaceable class="parameter">option</replaceable> can be:</phrase>
-
- PUBLISH INSERT | NOPUBLISH INSERT
- | PUBLISH UPDATE | NOPUBLISH UPDATE
- | PUBLISH DELETE | NOPUBLISH DELETE
</synopsis>
</refsynopsisdiv>
@@ -97,37 +92,29 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
</varlistentry>
<varlistentry>
- <term><literal>PUBLISH INSERT</literal></term>
- <term><literal>NOPUBLISH INSERT</literal></term>
- <listitem>
- <para>
- These clauses determine whether the new publication will send
- the <command>INSERT</command> operations to the subscribers.
- <literal>PUBLISH INSERT</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>PUBLISH UPDATE</literal></term>
- <term><literal>NOPUBLISH UPDATE</literal></term>
+ <term><literal>WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )</literal></term>
<listitem>
<para>
- These clauses determine whether the new publication will send
- the <command>UPDATE</command> operations to the subscribers.
- <literal>PUBLISH UPDATE</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
+ This clause specifies optional parameters for a pubication; the
+ following parameters are supported:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>publish</literal> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ This clause determine which DML operations will be published by
+ the new publication to the subscribers. The value is comma
+ separated list of actions. The allowed operations are
+ <literal>insert</literal>, <literal>update</literal>, and
+ <literal>delete</literal>. The default is to publish all actions so
+ the default value for this option is
+ <literal>'insert,update,delete'</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
- <varlistentry>
- <term><literal>PUBLISH DELETE</literal></term>
- <term><literal>NOPUBLISH DELETE</literal></term>
- <listitem>
- <para>
- These clauses determine whether the new publication will send
- the <command>DELETE</command> operations to the subscribers.
- <literal>PUBLISH DELETE</literal> is the default.
</para>
</listitem>
</varlistentry>
@@ -203,7 +190,7 @@ CREATE PUBLICATION alltables FOR ALL TABLES;
operations in one table:
<programlisting>
CREATE PUBLICATION insert_only FOR TABLE mydata
- WITH (NOPUBLISH UPDATE, NOPUBLISH DELETE);
+ WITH (publish = 'insert');
</programlisting>
</para>
</refsect1>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 3c51012..fcec254 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -24,16 +24,7 @@ PostgreSQL documentation
CREATE SUBSCRIPTION <replaceable class="PARAMETER">subscription_name</replaceable>
CONNECTION '<replaceable class="PARAMETER">conninfo</replaceable>'
PUBLICATION { <replaceable class="PARAMETER">publication_name</replaceable> [, ...] }
- [ WITH ( <replaceable class="PARAMETER">option</replaceable> [, ... ] ) ]
-
-<phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
-
- | ENABLED | DISABLED
- | CREATE SLOT | NOCREATE SLOT
- | SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
- | COPY DATA | NOCOPY DATA
- | SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable>
- | NOCONNECT
+ [ WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
</synopsis>
</refsynopsisdiv>
@@ -103,104 +94,117 @@ CREATE SUBSCRIPTION <replaceable class="PARAMETER">subscription_name</replaceabl
</varlistentry>
<varlistentry>
- <term><literal>ENABLED</literal></term>
- <term><literal>DISABLED</literal></term>
- <listitem>
- <para>
- Specifies whether the subscription should be actively replicating or
- if it should be just setup but not started yet. Note that the
- replication slot as described above is created in either case.
- <literal>ENABLED</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>CREATE SLOT</literal></term>
- <term><literal>NOCREATE SLOT</literal></term>
- <listitem>
- <para>
- Specifies whether the command should create the replication slot on the
- publisher. <literal>CREATE SLOT</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>SLOT NAME = <replaceable class="parameter">slot_name</replaceable></literal></term>
- <listitem>
- <para>
- Name of the replication slot to use. The default behavior is to use
- <literal>subscription_name</> for slot name.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>COPY DATA</literal></term>
- <term><literal>NOCOPY DATA</literal></term>
+ <term><literal>WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )</literal></term>
<listitem>
<para>
- Specifies if the existing data in the publications that are being
- subscribed to should be copied once the replication starts.
- <literal>COPY DATA</literal> is the default.
+ This clause specifies optional parameters for a subscription; the
+ following parameters are supported:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>enabled</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies whether the subscription should be actively replicating
+ or if it should be just setup but not started yet. The default is
+ <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>create_slot</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies whether the command should create the replication slot
+ on the publisher. The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>slot_name</literal> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ Name of the replication slot to use. The default behavior is to
+ use <literal>subscription_name</> for slot name.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>copy_data</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies if the existing data in the publications that are being
+ subscribed to should be copied once the replication starts.
+ The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>synchronous_commit</literal> (<type>enum</type>)</term>
+ <listitem>
+ <para>
+ The value of this parameter overrides the
+ <xref linkend="guc-synchronous-commit"> setting. The default
+ value is <literal>off</literal>.
+ </para>
+
+ <para>
+ It is safe to use <literal>off</literal> for logical replication:
+ If the subscriber loses transactions because of missing
+ synchronization, the data will be resent from the publisher.
+ </para>
+
+ <para>
+ A different setting might be appropriate when doing synchronous
+ logical replication. The logical replication workers report the
+ positions of writes and flushes to the publisher, and when using
+ synchronous replication, the publisher will wait for the actual
+ flush. This means that setting
+ <literal>synchronous_commit</literal> for the subscriber to
+ <literal>off</literal> when the subscription is used for
+ synchronous replication might increase the latency for
+ <command>COMMIT</command> on the publisher. In this scenario, it
+ can be advantageous to set <literal>synchronous_commit</literal>
+ to <literal>local</literal> or higher.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>connect</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Boolean specifying if the <command>CREATE SUBSCRIPTION</command>
+ should connect to the provider. Setting this to
+ <literal>false</literal> will change default values of
+ <literal>enabled</literal>, <literal>create_slot</literal> and
+ <literal>copy_data</literal> to <literal>false</literal>.
+ </para>
+ <para>
+ It's not allowed to combine <literal>connect</literal> set to
+ <literal>false</literal> and <literal>enabled</literal>,
+ <literal>create_slot</literal>, or <literal>copy_data</literal>
+ to <literal>true</literal>.
+ </para>
+ <para>
+ Since no connection is made when this option is specified, the
+ tables are not subscribed, so after you enable the subscription
+ nothing will be replicated. It is required to run
+ <literal>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</> in order
+ for tables to be subscribed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
</para>
</listitem>
</varlistentry>
- <varlistentry>
- <term><literal>SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable></literal></term>
- <listitem>
- <para>
- The value of this parameter overrides the
- <xref linkend="guc-synchronous-commit"> setting. The default value is
- <literal>off</literal>.
- </para>
-
- <para>
- It is safe to use <literal>off</literal> for logical replication: If the
- subscriber loses transactions because of missing synchronization, the
- data will be resent from the publisher.
- </para>
-
- <para>
- A different setting might be appropriate when doing synchronous logical
- replication. The logical replication workers report the positions of
- writes and flushes to the publisher, and when using synchronous
- replication, the publisher will wait for the actual flush. This means
- that setting <literal>SYNCHRONOUS_COMMIT</literal> for the subscriber
- to <literal>off</literal> when the subscription is used for synchronous
- replication might increase the latency for <command>COMMIT</command> on
- the publisher. In this scenario, it can be advantageous to set
- <literal>SYNCHRONOUS_COMMIT</literal> to <literal>local</literal> or
- higher.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>NOCONNECT</literal></term>
- <listitem>
- <para>
- Instructs <command>CREATE SUBSCRIPTION</command> to skip the initial
- connection to the provider. This will change default values of other
- options to <literal>DISABLED</literal>,
- <literal>NOCREATE SLOT</literal>, and <literal>NOCOPY DATA</literal>.
- </para>
- <para>
- It's not allowed to combine <literal>NOCONNECT</literal> and
- <literal>ENABLED</literal>, <literal>CREATE SLOT</literal>, or
- <literal>COPY DATA</literal>.
- </para>
- <para>
- Since no connection is made when this option is specified, the tables
- are not subscribed, so after you enable the subscription nothing will
- be replicated. It is required to run
- <literal>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</> in order for
- tables to be subscribed.
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</refsect1>
@@ -237,7 +241,7 @@ CREATE SUBSCRIPTION mysub
CREATE SUBSCRIPTION mysub
CONNECTION 'host=192.168.1.50 port=5432 user=foo dbname=foodb'
PUBLICATION insert_only
- WITH (DISABLED);
+ WITH (enabled = false);
</programlisting>
</para>
</refsect1>
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 541da7e..0ead996 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -46,6 +46,7 @@
#include "utils/lsyscache.h"
#include "utils/rel.h"
#include "utils/syscache.h"
+#include "utils/varlena.h"
/* Same as MAXNUMMESSAGES in sinvaladt.c */
#define MAX_RELCACHE_INVAL_MSGS 4096
@@ -58,18 +59,14 @@ static void PublicationDropTables(Oid pubid, List *rels, bool missing_ok);
static void
parse_publication_options(List *options,
- bool *publish_insert_given,
+ bool *publish_given,
bool *publish_insert,
- bool *publish_update_given,
bool *publish_update,
- bool *publish_delete_given,
bool *publish_delete)
{
ListCell *lc;
- *publish_insert_given = false;
- *publish_update_given = false;
- *publish_delete_given = false;
+ *publish_given = false;
/* Defaults are true */
*publish_insert = true;
@@ -81,65 +78,47 @@ parse_publication_options(List *options,
{
DefElem *defel = (DefElem *) lfirst(lc);
- if (strcmp(defel->defname, "publish insert") == 0)
+ if (strcmp(defel->defname, "publish") == 0)
{
- if (*publish_insert_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ char *publish;
+ List *publish_list;
+ ListCell *lc;
- *publish_insert_given = true;
- *publish_insert = defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "nopublish insert") == 0)
- {
- if (*publish_insert_given)
+ if (*publish_given)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
- *publish_insert_given = true;
- *publish_insert = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "publish update") == 0)
- {
- if (*publish_update_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ /*
+ * If publish option was given only the explicitly listed actions
+ * should be published.
+ */
+ *publish_insert = false;
+ *publish_update = false;
+ *publish_delete = false;
- *publish_update_given = true;
- *publish_update = defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "nopublish update") == 0)
- {
- if (*publish_update_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ *publish_given = true;
+ publish = defGetString(defel);
- *publish_update_given = true;
- *publish_update = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "publish delete") == 0)
- {
- if (*publish_delete_given)
+ if (!SplitIdentifierString(publish, ',', &publish_list))
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("invalid publish list")));
- *publish_delete_given = true;
- *publish_delete = defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "nopublish delete") == 0)
- {
- if (*publish_delete_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- *publish_delete_given = true;
- *publish_delete = !defGetBoolean(defel);
+ /* Process the option list. */
+ foreach (lc, publish_list)
+ {
+ char *publish_opt = (char *)lfirst(lc);
+
+ if (strcmp(publish_opt, "insert") == 0)
+ *publish_insert = true;
+ else if (strcmp(publish_opt, "update") == 0)
+ *publish_update = true;
+ else if (strcmp(publish_opt, "delete") == 0)
+ *publish_delete = true;
+ else
+ elog(ERROR, "unrecognized publish option: %s", publish_opt);
+ }
}
else
elog(ERROR, "unrecognized option: %s", defel->defname);
@@ -158,9 +137,7 @@ CreatePublication(CreatePublicationStmt *stmt)
bool nulls[Natts_pg_publication];
Datum values[Natts_pg_publication];
HeapTuple tup;
- bool publish_insert_given;
- bool publish_update_given;
- bool publish_delete_given;
+ bool publish_given;
bool publish_insert;
bool publish_update;
bool publish_delete;
@@ -199,9 +176,8 @@ CreatePublication(CreatePublicationStmt *stmt)
values[Anum_pg_publication_pubowner - 1] = ObjectIdGetDatum(GetUserId());
parse_publication_options(stmt->options,
- &publish_insert_given, &publish_insert,
- &publish_update_given, &publish_update,
- &publish_delete_given, &publish_delete);
+ &publish_given, &publish_insert,
+ &publish_update, &publish_delete);
values[Anum_pg_publication_puballtables - 1] =
BoolGetDatum(stmt->for_all_tables);
@@ -253,42 +229,31 @@ AlterPublicationOptions(AlterPublicationStmt *stmt, Relation rel,
bool nulls[Natts_pg_publication];
bool replaces[Natts_pg_publication];
Datum values[Natts_pg_publication];
- bool publish_insert_given;
- bool publish_update_given;
- bool publish_delete_given;
+ bool publish_given;
bool publish_insert;
bool publish_update;
bool publish_delete;
ObjectAddress obj;
parse_publication_options(stmt->options,
- &publish_insert_given, &publish_insert,
- &publish_update_given, &publish_update,
- &publish_delete_given, &publish_delete);
+ &publish_given, &publish_insert,
+ &publish_update, &publish_delete);
+
+ Assert(publish_given);
/* Everything ok, form a new tuple. */
memset(values, 0, sizeof(values));
memset(nulls, false, sizeof(nulls));
memset(replaces, false, sizeof(replaces));
- if (publish_insert_given)
- {
- values[Anum_pg_publication_pubinsert - 1] =
- BoolGetDatum(publish_insert);
- replaces[Anum_pg_publication_pubinsert - 1] = true;
- }
- if (publish_update_given)
- {
- values[Anum_pg_publication_pubupdate - 1] =
- BoolGetDatum(publish_update);
- replaces[Anum_pg_publication_pubupdate - 1] = true;
- }
- if (publish_delete_given)
- {
- values[Anum_pg_publication_pubdelete - 1] =
- BoolGetDatum(publish_delete);
- replaces[Anum_pg_publication_pubdelete - 1] = true;
- }
+ values[Anum_pg_publication_pubinsert - 1] = BoolGetDatum(publish_insert);
+ replaces[Anum_pg_publication_pubinsert - 1] = true;
+
+ values[Anum_pg_publication_pubupdate - 1] = BoolGetDatum(publish_update);
+ replaces[Anum_pg_publication_pubupdate - 1] = true;
+
+ values[Anum_pg_publication_pubdelete - 1] = BoolGetDatum(publish_delete);
+ replaces[Anum_pg_publication_pubdelete - 1] = true;
tup = heap_modify_tuple(tup, RelationGetDescr(rel), values, nulls,
replaces);
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index ee0983f..c00981e 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -89,7 +89,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
{
DefElem *defel = (DefElem *) lfirst(lc);
- if (strcmp(defel->defname, "noconnect") == 0 && connect)
+ if (strcmp(defel->defname, "connect") == 0 && connect)
{
if (connect_given)
ereport(ERROR,
@@ -97,7 +97,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
errmsg("conflicting or redundant options")));
connect_given = true;
- *connect = !defGetBoolean(defel);
+ *connect = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "enabled") == 0 && enabled)
{
@@ -109,17 +109,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
*enabled_given = true;
*enabled = defGetBoolean(defel);
}
- else if (strcmp(defel->defname, "disabled") == 0 && enabled)
- {
- if (*enabled_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- *enabled_given = true;
- *enabled = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "create slot") == 0 && create_slot)
+ else if (strcmp(defel->defname, "create_slot") == 0 && create_slot)
{
if (create_slot_given)
ereport(ERROR,
@@ -129,17 +119,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
create_slot_given = true;
*create_slot = defGetBoolean(defel);
}
- else if (strcmp(defel->defname, "nocreate slot") == 0 && create_slot)
- {
- if (create_slot_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- create_slot_given = true;
- *create_slot = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "slot name") == 0 && slot_name)
+ else if (strcmp(defel->defname, "slot_name") == 0 && slot_name)
{
if (*slot_name)
ereport(ERROR,
@@ -148,7 +128,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
*slot_name = defGetString(defel);
}
- else if (strcmp(defel->defname, "copy data") == 0 && copy_data)
+ else if (strcmp(defel->defname, "copy_data") == 0 && copy_data)
{
if (copy_data_given)
ereport(ERROR,
@@ -158,16 +138,6 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
copy_data_given = true;
*copy_data = defGetBoolean(defel);
}
- else if (strcmp(defel->defname, "nocopy data") == 0 && copy_data)
- {
- if (copy_data_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- copy_data_given = true;
- *copy_data = !defGetBoolean(defel);
- }
else if (strcmp(defel->defname, "synchronous_commit") == 0 &&
synchronous_commit)
{
@@ -309,7 +279,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
* replication slot.
*/
if (create_slot)
- PreventTransactionChain(isTopLevel, "CREATE SUBSCRIPTION ... CREATE SLOT");
+ PreventTransactionChain(isTopLevel, "CREATE SUBSCRIPTION ... (create_slot true)");
if (!superuser())
ereport(ERROR,
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 818d2c2..732647b 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -652,8 +652,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
MAPPING MATCH MATERIALIZED MAXVALUE METHOD MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NONE
- NOREFRESH NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
- NULLS_P NUMERIC
+ NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF NULLS_P NUMERIC
OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OPTIONS OR
ORDER ORDINALITY OUT_P OUTER_P OVER OVERLAPS OVERLAY OVERRIDING OWNED OWNER
@@ -5677,7 +5676,6 @@ def_elem: def_key '=' def_arg
def_key:
ColLabel { $$ = $1; }
- | ColLabel ColLabel { $$ = psprintf("%s %s", $1, $2); }
;
/* Note: any simple identifier will be returned as a type name! */
@@ -9155,6 +9153,7 @@ publication_for_tables:
}
;
+
/*****************************************************************************
*
* ALTER PUBLICATION name [ WITH ] options
@@ -9278,7 +9277,7 @@ AlterSubscriptionStmt:
n->options = $8;
$$ = (Node *)n;
}
- | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list NOREFRESH
+ | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
{
AlterSubscriptionStmt *n =
makeNode(AlterSubscriptionStmt);
@@ -14758,7 +14757,6 @@ unreserved_keyword:
| NEW
| NEXT
| NO
- | NOREFRESH
| NOTHING
| NOTIFY
| NOWAIT
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2fda350..2fdcd2d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3457,6 +3457,7 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
PQExpBuffer delq;
PQExpBuffer query;
PQExpBuffer labelq;
+ bool first = true;
if (!(pubinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
return;
@@ -3476,23 +3477,32 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
if (pubinfo->puballtables)
appendPQExpBufferStr(query, " FOR ALL TABLES");
- appendPQExpBufferStr(query, " WITH (");
+ appendPQExpBufferStr(query, " WITH (publish = '");
if (pubinfo->pubinsert)
- appendPQExpBufferStr(query, "PUBLISH INSERT");
- else
- appendPQExpBufferStr(query, "NOPUBLISH INSERT");
+ {
+ appendPQExpBufferStr(query, "insert");
+ first = false;
+ }
+
+ if (!first)
+ appendPQExpBufferChar(query, ',');
if (pubinfo->pubupdate)
- appendPQExpBufferStr(query, ", PUBLISH UPDATE");
- else
- appendPQExpBufferStr(query, ", NOPUBLISH UPDATE");
+ {
+ appendPQExpBufferStr(query, "update");
+ first = false;
+ }
+
+ if (!first)
+ appendPQExpBufferChar(query, ',');
if (pubinfo->pubdelete)
- appendPQExpBufferStr(query, ", PUBLISH DELETE");
- else
- appendPQExpBufferStr(query, ", NOPUBLISH DELETE");
+ {
+ appendPQExpBufferStr(query, "delete");
+ first = false;
+ }
- appendPQExpBufferStr(query, ");\n");
+ appendPQExpBufferStr(query, "');\n");
ArchiveEntry(fout, pubinfo->dobj.catId, pubinfo->dobj.dumpId,
pubinfo->dobj.name,
@@ -3813,11 +3823,11 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
appendPQExpBufferStr(publications, fmtId(pubnames[i]));
}
- appendPQExpBuffer(query, " PUBLICATION %s WITH (NOCONNECT, SLOT NAME = ", publications->data);
+ appendPQExpBuffer(query, " PUBLICATION %s WITH (connect = false, slot_name = ", publications->data);
appendStringLiteralAH(query, subinfo->subslotname, fout);
if (strcmp(subinfo->subsynccommit, "off") != 0)
- appendPQExpBuffer(query, ", SYNCHRONOUS_COMMIT = %s", fmtId(subinfo->subsynccommit));
+ appendPQExpBuffer(query, ", synchronous_commit = %s", fmtId(subinfo->subsynccommit));
appendPQExpBufferStr(query, ");\n");
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 0259db9..52e4c1a 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4317,7 +4317,7 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
create_order => 50,
create_sql => 'CREATE PUBLICATION pub1;',
regexp => qr/^
- \QCREATE PUBLICATION pub1 WITH (PUBLISH INSERT, PUBLISH UPDATE, PUBLISH DELETE);\E
+ \QCREATE PUBLICATION pub1 WITH (publish = 'insert,update,delete');\E
/xm,
like => {
binary_upgrade => 1,
@@ -4350,11 +4350,9 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
create_order => 50,
create_sql => 'CREATE PUBLICATION pub2
FOR ALL TABLES
- WITH (NOPUBLISH INSERT,
- NOPUBLISH UPDATE,
- NOPUBLISH DELETE);',
+ WITH (publish = \'\');',
regexp => qr/^
- \QCREATE PUBLICATION pub2 FOR ALL TABLES WITH (NOPUBLISH INSERT, NOPUBLISH UPDATE, NOPUBLISH DELETE);\E
+ \QCREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish = '');\E
/xm,
like => {
binary_upgrade => 1,
@@ -4387,9 +4385,9 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
create_order => 50,
create_sql => 'CREATE SUBSCRIPTION sub1
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
- WITH (NOCONNECT);',
+ WITH (connect = false);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (NOCONNECT, SLOT NAME = 'sub1');\E
+ \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E
/xm,
like => {
binary_upgrade => 1,
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 37542aa..b161335 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -259,7 +259,6 @@ PG_KEYWORD("new", NEW, UNRESERVED_KEYWORD)
PG_KEYWORD("next", NEXT, UNRESERVED_KEYWORD)
PG_KEYWORD("no", NO, UNRESERVED_KEYWORD)
PG_KEYWORD("none", NONE, COL_NAME_KEYWORD)
-PG_KEYWORD("norefresh", NOREFRESH, UNRESERVED_KEYWORD)
PG_KEYWORD("not", NOT, RESERVED_KEYWORD)
PG_KEYWORD("nothing", NOTHING, UNRESERVED_KEYWORD)
PG_KEYWORD("notify", NOTIFY, UNRESERVED_KEYWORD)
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index 27c8ec5..f6fc29a 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -69,7 +69,7 @@ CREATE SCHEMA dummy_seclabel_test;
SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
index 8d43244..d7795bd 100644
--- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
+++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
@@ -73,7 +73,7 @@ SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 814e05e..0849715 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -37,7 +37,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCONNECT);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index f3a348d..d9089af 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -13,8 +13,8 @@ SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
test publication
(1 row)
-CREATE PUBLICATION testpib_ins_trunct WITH (nopublish delete, nopublish update);
-ALTER PUBLICATION testpub_default WITH (nopublish insert, nopublish delete);
+CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
+ALTER PUBLICATION testpub_default WITH (publish = update);
\dRp
List of publications
Name | Owner | Inserts | Updates | Deletes
@@ -23,7 +23,7 @@ ALTER PUBLICATION testpub_default WITH (nopublish insert, nopublish delete);
testpub_default | regress_publication_user | f | t | f
(2 rows)
-ALTER PUBLICATION testpub_default WITH (publish insert, publish delete);
+ALTER PUBLICATION testpub_default WITH (publish = 'insert, update, delete');
\dRp
List of publications
Name | Owner | Inserts | Updates | Deletes
@@ -38,8 +38,8 @@ CREATE TABLE testpub_tbl1 (id serial primary key, data text);
CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
CREATE VIEW testpub_view AS SELECT 1;
CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
-CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (nopublish delete, nopublish update);
-ALTER PUBLICATION testpub_foralltables WITH (publish update);
+CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+ALTER PUBLICATION testpub_foralltables WITH (publish = 'insert,update');
CREATE TABLE testpub_tbl2 (id serial primary key, data text);
-- fail - can't add to for all tables publication
ALTER PUBLICATION testpub_foralltables ADD TABLE testpub_tbl2;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index b1686db..33d53de 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -17,18 +17,18 @@ LINE 1: CREATE SUBSCRIPTION testsub PUBLICATION foo;
^
-- fail - cannot do CREATE SUBSCRIPTION CREATE SLOT inside transaction block
BEGIN;
-CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (CREATE SLOT);
-ERROR: CREATE SUBSCRIPTION ... CREATE SLOT cannot run inside a transaction block
+CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (create_slot);
+ERROR: CREATE SUBSCRIPTION ... (create_slot true) cannot run inside a transaction block
COMMIT;
-- fail - invalid connection string
CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub;
ERROR: invalid connection string syntax: missing "=" after "testconn" in connection info string
-- fail - duplicate publications
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (connect = false);
ERROR: publication name "foo" used more than once
-- ok
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
COMMENT ON SUBSCRIPTION testsub IS 'test subscription';
SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
@@ -38,11 +38,11 @@ SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
(1 row)
-- fail - name already exists
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
ERROR: subscription "testsub" already exists
-- fail - must be superuser
SET SESSION AUTHORIZATION 'regress_subscription_user2';
-CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
ERROR: must be superuser to create subscriptions
SET SESSION AUTHORIZATION 'regress_subscription_user';
\dRs+
@@ -52,9 +52,9 @@ SET SESSION AUTHORIZATION 'regress_subscription_user';
testsub | regress_subscription_user | f | {testpub} | off | dbname=doesnotexist
(1 row)
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 NOREFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
-ALTER SUBSCRIPTION testsub WITH (SLOT NAME = 'newname');
+ALTER SUBSCRIPTION testsub WITH (slot_name = 'newname');
-- fail
ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
ERROR: subscription "doesnotexist" does not exist
@@ -89,8 +89,8 @@ ALTER SUBSCRIPTION testsub RENAME TO testsub_dummy;
ERROR: must be owner of subscription testsub
RESET ROLE;
ALTER SUBSCRIPTION testsub RENAME TO testsub_foo;
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = local);
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = foobar);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = local);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = foobar);
ERROR: invalid value for parameter "synchronous_commit": "foobar"
HINT: Available values: local, remote_write, remote_apply, on, off.
\dRs+
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index c9219e4..c698a63 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -40,7 +40,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCONNECT);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false);
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 7d1cba5..ed6bd57 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -11,13 +11,13 @@ CREATE PUBLICATION testpub_default;
COMMENT ON PUBLICATION testpub_default IS 'test publication';
SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
-CREATE PUBLICATION testpib_ins_trunct WITH (nopublish delete, nopublish update);
+CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
-ALTER PUBLICATION testpub_default WITH (nopublish insert, nopublish delete);
+ALTER PUBLICATION testpub_default WITH (publish = update);
\dRp
-ALTER PUBLICATION testpub_default WITH (publish insert, publish delete);
+ALTER PUBLICATION testpub_default WITH (publish = 'insert, update, delete');
\dRp
@@ -28,8 +28,8 @@ CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
CREATE VIEW testpub_view AS SELECT 1;
CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
-CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (nopublish delete, nopublish update);
-ALTER PUBLICATION testpub_foralltables WITH (publish update);
+CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+ALTER PUBLICATION testpub_foralltables WITH (publish = 'insert,update');
CREATE TABLE testpub_tbl2 (id serial primary key, data text);
-- fail - can't add to for all tables publication
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 1b30d15..5454420 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -15,34 +15,34 @@ CREATE SUBSCRIPTION testsub PUBLICATION foo;
-- fail - cannot do CREATE SUBSCRIPTION CREATE SLOT inside transaction block
BEGIN;
-CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (CREATE SLOT);
+CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (create_slot);
COMMIT;
-- fail - invalid connection string
CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub;
-- fail - duplicate publications
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (connect = false);
-- ok
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
COMMENT ON SUBSCRIPTION testsub IS 'test subscription';
SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
-- fail - name already exists
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
-- fail - must be superuser
SET SESSION AUTHORIZATION 'regress_subscription_user2';
-CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
SET SESSION AUTHORIZATION 'regress_subscription_user';
\dRs+
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 NOREFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
-ALTER SUBSCRIPTION testsub WITH (SLOT NAME = 'newname');
+ALTER SUBSCRIPTION testsub WITH (slot_name = 'newname');
-- fail
ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
@@ -66,8 +66,8 @@ ALTER SUBSCRIPTION testsub RENAME TO testsub_dummy;
RESET ROLE;
ALTER SUBSCRIPTION testsub RENAME TO testsub_foo;
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = local);
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = foobar);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = local);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = foobar);
\dRs+
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index d1817f5..b4c5539 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -40,7 +40,7 @@ my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION tap_pub");
$node_publisher->safe_psql('postgres',
- "CREATE PUBLICATION tap_pub_ins_only WITH (nopublish delete, nopublish update)");
+ "CREATE PUBLICATION tap_pub_ins_only WITH (publish = insert)");
$node_publisher->safe_psql('postgres',
"ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full");
$node_publisher->safe_psql('postgres',
@@ -136,7 +136,7 @@ $node_publisher->poll_query_until('postgres',
$oldpid = $node_publisher->safe_psql('postgres',
"SELECT pid FROM pg_stat_replication WHERE application_name = '$appname';");
$node_subscriber->safe_psql('postgres',
- "ALTER SUBSCRIPTION tap_sub SET PUBLICATION tap_pub_ins_only REFRESH WITH (NOCOPY DATA)");
+ "ALTER SUBSCRIPTION tap_sub SET PUBLICATION tap_pub_ins_only REFRESH WITH (copy_data = false)");
$node_publisher->poll_query_until('postgres',
"SELECT pid != $oldpid FROM pg_stat_replication WHERE application_name = '$appname';")
or die "Timed out while waiting for apply to restart";
@@ -159,13 +159,13 @@ is($result, qq(20|-20|-1), 'check changes skipped after subscription publication
# check alter publication (relcache invalidation etc)
$node_publisher->safe_psql('postgres',
- "ALTER PUBLICATION tap_pub_ins_only WITH (publish delete)");
+ "ALTER PUBLICATION tap_pub_ins_only WITH (publish = 'insert,delete')");
$node_publisher->safe_psql('postgres',
"ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_full");
$node_publisher->safe_psql('postgres',
"DELETE FROM tab_ins WHERE a > 0");
$node_subscriber->safe_psql('postgres',
- "ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (NOCOPY DATA)");
+ "ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (copy_data = false)");
$node_publisher->safe_psql('postgres',
"INSERT INTO tab_full VALUES(0)");
diff --git a/src/test/subscription/t/002_types.pl b/src/test/subscription/t/002_types.pl
index ad15e85..e9368ab 100644
--- a/src/test/subscription/t/002_types.pl
+++ b/src/test/subscription/t/002_types.pl
@@ -103,7 +103,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (SLOT NAME = tap_sub_slot)");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (slot_name = tap_sub_slot)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/003_constraints.pl b/src/test/subscription/t/003_constraints.pl
index 11b8254..e906ea1 100644
--- a/src/test/subscription/t/003_constraints.pl
+++ b/src/test/subscription/t/003_constraints.pl
@@ -34,7 +34,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA)");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (copy_data = false)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index fa0bf7f..8ece7dd 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -82,7 +82,7 @@ is($result, qq(20), 'initial data synced for second sub');
# now check another subscription for the same node pair
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub2 CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA)");
+ "CREATE SUBSCRIPTION tap_sub2 CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (copy_data = false)");
# wait for it to start
$node_subscriber->poll_query_until('postgres', "SELECT pid IS NOT NULL FROM pg_stat_subscription WHERE subname = 'tap_sub2' AND relid IS NULL")
--
2.7.4
On 02/05/17 15:31, Tom Lane wrote:
Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
Let me expand, if we don't drop the slot by default when dropping
subscription, we'll have a lot of users with dead slots laying around
holding back WAL/catalog_xmin, that's really bad. If we do drop by
default like now, we need option to not do that, neither RESTRICT, nor
CASCADE fit that.I'm not sure which part of "you can't have an option in DROP" isn't
clear to you. Whatever the default behavior is always has to work,
because that is what's going to happen during DROP OWNED BY, or
DROP DATABASE. If you want more than one behavior during DROP,
you need to drive that off something represented as a persistent
property of the object, not off an option in the DROP command.I agree that RESTRICT/CASCADE don't fit this, unless the model
is that the slot is always owned by the subscription, which might
be too restrictive.
What do you think of attached. I actually did add RESTRICT/CASCADE, in a
way that if there is slot RESTRICT will refuse to drop subscription and
CASCADE will try to drop it. Still all errors.
The new way to not drop slot is to set slot_name to NONE which is new
value that I invented (inspiration from OWNED BY sequences) which
basically disassociates the subscription from slot.
It's slightly less automatic this way but perhaps that's not a bad
thing, at least nobody will drop slots by mistake while we still make
sure that slots are not left over without anybody noticing.
Note that this would need catalog version bump as it removes NOT NULL
constraint from subslotname.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPTION.patchbinary/octet-stream; name=Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPTION.patchDownload
From d242be3dc95443f226a906f108c52d32b9ac3e1e Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Fri, 5 May 2017 18:14:00 +0200
Subject: [PATCH] Remove the NODROP SLOT option from DROP SUBSCRIPTION
---
doc/src/sgml/ref/create_subscription.sgml | 7 +
doc/src/sgml/ref/drop_subscription.sgml | 22 +--
src/backend/catalog/pg_subscription.c | 9 +-
src/backend/commands/subscriptioncmds.c | 195 ++++++++++++++++-----
src/backend/nodes/copyfuncs.c | 2 +-
src/backend/nodes/equalfuncs.c | 2 +-
src/backend/parser/gram.y | 43 ++---
src/backend/replication/logical/worker.c | 35 ++--
src/backend/tcop/utility.c | 3 +
src/include/catalog/pg_subscription.h | 2 +-
src/include/nodes/parsenodes.h | 2 +-
.../dummy_seclabel/expected/dummy_seclabel.out | 4 +-
.../modules/dummy_seclabel/sql/dummy_seclabel.sql | 4 +-
src/test/regress/expected/object_address.out | 4 +-
src/test/regress/expected/subscription.out | 13 +-
src/test/regress/sql/object_address.sql | 4 +-
src/test/regress/sql/subscription.sql | 12 +-
src/test/subscription/t/001_rep_changes.pl | 2 +-
src/test/subscription/t/004_sync.pl | 8 +-
19 files changed, 241 insertions(+), 132 deletions(-)
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index fcec254..c9256e7 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -129,6 +129,13 @@ CREATE SUBSCRIPTION <replaceable class="PARAMETER">subscription_name</replaceabl
Name of the replication slot to use. The default behavior is to
use <literal>subscription_name</> for slot name.
</para>
+ <para>
+ When <literal>slot_name</literal> is set to
+ <literal>NONE</literal> there will be no slot associated with the
+ subscription. Such subscriptions must also have both
+ <literal>enabled</literal> and <literal>create_slot</literal> set
+ to <literal>false</literal>.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml
index f1ac125..8b00c45 100644
--- a/doc/src/sgml/ref/drop_subscription.sgml
+++ b/doc/src/sgml/ref/drop_subscription.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ DROP SLOT | NODROP SLOT ]
+DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
@@ -57,20 +57,16 @@ DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable
</varlistentry>
<varlistentry>
- <term><literal>DROP SLOT</literal></term>
- <term><literal>NODROP SLOT</literal></term>
- <listitem>
- <para>
- Specifies whether to drop the replication slot on the publisher. The
- default is
- <literal>DROP SLOT</literal>.
- </para>
+ <term><literal>CASCADE</literal></term>
+ <term><literal>RESTRICT</literal></term>
+ <listitem>
<para>
- If the publisher is not reachable when the subscription is to be
- dropped, then it is useful to specify <literal>NODROP SLOT</literal>.
- But the replication slot on the publisher will then have to be removed
- manually.
+ These key words are used to determine what to do with when there is a
+ replication slot associated with the subscription. The
+ <literal>RESTRICT</literal> will refuse to drop the subscription in
+ such case, while <literal>CASCADE</literal> will drop the associated
+ slot. <literal>RESTRICT</literal> is the default.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 22587a4..7dc21f1 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -82,8 +82,10 @@ GetSubscription(Oid subid, bool missing_ok)
tup,
Anum_pg_subscription_subslotname,
&isnull);
- Assert(!isnull);
- sub->slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ if (!isnull)
+ sub->slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ else
+ sub->slotname = NULL;
/* Get synccommit */
datum = SysCacheGetAttr(SUBSCRIPTIONOID,
@@ -147,7 +149,8 @@ FreeSubscription(Subscription *sub)
{
pfree(sub->name);
pfree(sub->conninfo);
- pfree(sub->slotname);
+ if (sub->slotname)
+ pfree(sub->slotname);
list_free_deep(sub->publications);
pfree(sub);
}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index c00981e..afb116a 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -60,7 +60,8 @@ static List *fetch_table_list(WalReceiverConn *wrconn, List *publications);
*/
static void
parse_subscription_options(List *options, bool *connect, bool *enabled_given,
- bool *enabled, bool *create_slot, char **slot_name,
+ bool *enabled, bool *create_slot,
+ bool *slot_name_given, char **slot_name,
bool *copy_data, char **synchronous_commit)
{
ListCell *lc;
@@ -78,7 +79,10 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (create_slot)
*create_slot = true;
if (slot_name)
+ {
+ *slot_name_given = false;
*slot_name = NULL;
+ }
if (copy_data)
*copy_data = true;
if (synchronous_commit)
@@ -121,12 +125,17 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
}
else if (strcmp(defel->defname, "slot_name") == 0 && slot_name)
{
- if (*slot_name)
+ if (*slot_name_given)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
+ *slot_name_given = true;
*slot_name = defGetString(defel);
+
+ /* Setting slot_name = NONE is treated as no slot name. */
+ if (strcmp(*slot_name, "none") == 0)
+ *slot_name = NULL;
}
else if (strcmp(defel->defname, "copy_data") == 0 && copy_data)
{
@@ -164,26 +173,43 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (connect && !*connect)
{
/* Check for incompatible options from the user. */
- if (*enabled_given && *enabled)
+ if (enabled && *enabled_given && *enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("noconnect and enabled are mutually exclusive options")));
+ errmsg("connect = false and enabled are mutually exclusive options")));
- if (create_slot_given && *create_slot)
+ if (create_slot && create_slot_given && *create_slot)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("noconnect and create slot are mutually exclusive options")));
+ errmsg("connect = false and create_slot are mutually exclusive options")));
- if (copy_data_given && *copy_data)
+ if (copy_data && copy_data_given && *copy_data)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("noconnect and copy data are mutually exclusive options")));
+ errmsg("connect = false and copy_data are mutually exclusive options")));
/* Change the defaults of other options. */
*enabled = false;
*create_slot = false;
*copy_data = false;
}
+
+ /*
+ * Do additional checking for disallowed combination when
+ * slot_name = NONE was used.
+ */
+ if (slot_name && *slot_name_given && !*slot_name)
+ {
+ if (enabled && *enabled_given && *enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("slot_name = NONE and enabled are mutually exclusive options")));
+
+ if (create_slot && create_slot_given && *create_slot)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("slot_name = NONE and create_slot are mutually exclusive options")));
+ }
}
/*
@@ -260,6 +286,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
char *synchronous_commit;
char *conninfo;
char *slotname;
+ bool slotname_given;
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
@@ -269,8 +296,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
* Connection and publication should not be specified here.
*/
parse_subscription_options(stmt->options, &connect, &enabled_given,
- &enabled, &create_slot, &slotname, ©_data,
- &synchronous_commit);
+ &enabled, &create_slot, &slotname_given,
+ &slotname, ©_data, &synchronous_commit);
/*
* Since creating a replication slot is not transactional, rolling back
@@ -299,8 +326,9 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
stmt->subname)));
}
- if (slotname == NULL)
+ if (!slotname_given && slotname == NULL)
slotname = stmt->subname;
+
/* The default for synchronous_commit of subscriptions is off. */
if (synchronous_commit == NULL)
synchronous_commit = "off";
@@ -325,8 +353,11 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
values[Anum_pg_subscription_subenabled - 1] = BoolGetDatum(enabled);
values[Anum_pg_subscription_subconninfo - 1] =
CStringGetTextDatum(conninfo);
- values[Anum_pg_subscription_subslotname - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ if (slotname)
+ values[Anum_pg_subscription_subslotname - 1] =
+ DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ else
+ nulls[Anum_pg_subscription_subslotname - 1] = true;
values[Anum_pg_subscription_subsynccommit - 1] =
CStringGetTextDatum(synchronous_commit);
values[Anum_pg_subscription_subpublications - 1] =
@@ -396,6 +427,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
*/
if (create_slot)
{
+ Assert(slotname);
+
walrcv_create_slot(wrconn, slotname, false,
CRS_NOEXPORT_SNAPSHOT, &lsn);
ereport(NOTICE,
@@ -548,6 +581,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
HeapTuple tup;
Oid subid;
bool update_tuple = false;
+ Subscription *sub;
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -567,6 +601,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
stmt->subname);
subid = HeapTupleGetOid(tup);
+ sub = GetSubscription(subid, false);
/* Form a new tuple. */
memset(values, 0, sizeof(values));
@@ -577,19 +612,29 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
{
case ALTER_SUBSCRIPTION_OPTIONS:
{
- char *slot_name;
- char *synchronous_commit;
+ char *slotname;
+ bool slotname_given;
+ char *synchronous_commit;
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, &slot_name, NULL,
- &synchronous_commit);
+ NULL, &slotname_given, &slotname,
+ NULL, &synchronous_commit);
- if (slot_name)
+ if (slotname_given)
{
- values[Anum_pg_subscription_subslotname - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(slot_name));
+ if (sub->enabled && !slotname)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot set slot_name = NONE for enabled subscription")));
+
+ if (slotname)
+ values[Anum_pg_subscription_subslotname - 1] =
+ DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ else
+ nulls[Anum_pg_subscription_subslotname - 1] = true;
replaces[Anum_pg_subscription_subslotname - 1] = true;
}
+
if (synchronous_commit)
{
values[Anum_pg_subscription_subsynccommit - 1] =
@@ -608,9 +653,14 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
parse_subscription_options(stmt->options, NULL,
&enabled_given, &enabled, NULL,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
Assert(enabled_given);
+ if (!sub->slotname && enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot enable subscription which does not have a slot_name")));
+
values[Anum_pg_subscription_subenabled - 1] =
BoolGetDatum(enabled);
replaces[Anum_pg_subscription_subenabled - 1] = true;
@@ -633,10 +683,10 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
case ALTER_SUBSCRIPTION_PUBLICATION_REFRESH:
{
bool copy_data;
- Subscription *sub = GetSubscription(subid, false);
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, NULL, ©_data, NULL);
+ NULL, NULL, NULL, ©_data,
+ NULL);
values[Anum_pg_subscription_subpublications - 1] =
publicationListToArray(stmt->publication);
@@ -647,6 +697,11 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
/* Refresh if user asked us to. */
if (stmt->kind == ALTER_SUBSCRIPTION_PUBLICATION_REFRESH)
{
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
+
/* Make sure refresh sees the new list of publications. */
sub->publications = stmt->publication;
@@ -659,10 +714,15 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
case ALTER_SUBSCRIPTION_REFRESH:
{
bool copy_data;
- Subscription *sub = GetSubscription(subid, false);
+
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, NULL, ©_data, NULL);
+ NULL, NULL, NULL, ©_data,
+ NULL);
AlterSubscription_refresh(sub, copy_data);
@@ -721,8 +781,8 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
* run DROP SUBSCRIPTION inside a transaction block if dropping the
* replication slot.
*/
- if (stmt->drop_slot)
- PreventTransactionChain(isTopLevel, "DROP SUBSCRIPTION ... DROP SLOT");
+ if (stmt->behavior == DROP_CASCADE)
+ PreventTransactionChain(isTopLevel, "DROP SUBSCRIPTION ... CASCADE");
/*
* Lock pg_subscription with AccessExclusiveLock to ensure
@@ -782,8 +842,10 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
/* Get slotname */
datum = SysCacheGetAttr(SUBSCRIPTIONOID, tup,
Anum_pg_subscription_subslotname, &isnull);
- Assert(!isnull);
- slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ if (!isnull)
+ slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ else
+ slotname = NULL;
ObjectAddressSet(myself, SubscriptionRelationId, subid);
EventTriggerSQLDropAddObject(&myself, true, true);
@@ -808,43 +870,84 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
if (originid != InvalidRepOriginId)
replorigin_drop(originid);
- /* If the user asked to not drop the slot, we are done mow.*/
- if (!stmt->drop_slot)
+ /* If there is no slot associated with subscription we can finish here. */
+ if (!slotname)
{
heap_close(rel, NoLock);
return;
}
/*
- * Otherwise drop the replication slot at the publisher node using
+ * Otherwise check for the replication slot at the publisher node using
* the replication connection.
*/
load_file("libpqwalreceiver", false);
- initStringInfo(&cmd);
- appendStringInfo(&cmd, "DROP_REPLICATION_SLOT \"%s\"", slotname);
-
wrconn = walrcv_connect(conninfo, true, subname, &err);
if (wrconn == NULL)
ereport(ERROR,
- (errmsg("could not connect to publisher when attempting to "
- "drop the replication slot \"%s\"", slotname),
- errdetail("The error was: %s", err)));
+ (errmsg("could not connect to publisher when attempting to fetch "
+ "information about replication slot \"%s\"", slotname),
+ errdetail("The error was: %s", err),
+ errhint("Use ALTER SUBSCRIPTION ... WITH (slot_name = NONE) "
+ "to disassociate the subscription from slot.")));
PG_TRY();
{
WalRcvExecResult *res;
- res = walrcv_exec(wrconn, cmd.data, 0, NULL);
-
- if (res->status != WALRCV_OK_COMMAND)
+ TupleTableSlot *tupslot;
+ Oid slot_row_desc[1] = {BOOLOID};
+ bool found;
+
+ initStringInfo(&cmd);
+ appendStringInfo(&cmd,
+ "SELECT true FROM pg_catalog.pg_replication_slots WHERE slot_name = %s",
+ quote_literal_cstr(slotname));
+ res = walrcv_exec(wrconn, cmd.data, 1, slot_row_desc);
+ if (res->status != WALRCV_OK_TUPLES)
ereport(ERROR,
- (errmsg("could not drop the replication slot \"%s\" on publisher",
- slotname),
+ (errmsg("could not fetch the replication slot info from publisher"),
errdetail("The error was: %s", res->err)));
+
+ tupslot = MakeSingleTupleTableSlot(res->tupledesc);
+ found = tuplestore_gettupleslot(res->tuplestore, true, false,
+ tupslot);
+ ExecDropSingleTupleTableSlot(tupslot);
+ walrcv_clear_result(res);
+
+ /* If slot was not found on publisher, we are done. */
+ if (!found)
+ {
+ walrcv_disconnect(wrconn);
+ pfree(cmd.data);
+ heap_close(rel, NoLock);
+ return;
+ }
+
+ /* Otherwise the next action depends on the drop_behavior. */
+ if (stmt->behavior == DROP_CASCADE)
+ {
+ resetStringInfo(&cmd);
+ appendStringInfo(&cmd, "DROP_REPLICATION_SLOT %s",
+ quote_identifier(slotname));
+ res = walrcv_exec(wrconn, cmd.data, 0, NULL);
+
+ if (res->status != WALRCV_OK_COMMAND)
+ ereport(ERROR,
+ (errmsg("could not drop the replication slot \"%s\" on publisher",
+ slotname),
+ errdetail("The error was: %s", res->err)));
+ else
+ ereport(NOTICE,
+ (errmsg("dropped replication slot \"%s\" on publisher",
+ slotname)));
+ }
else
- ereport(NOTICE,
- (errmsg("dropped replication slot \"%s\" on publisher",
- slotname)));
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot drop subscription \"%s\" because there is still replication slot associated with it",
+ subname),
+ errhint("Use DROP ... CASCADE to drop the slot too.")));
walrcv_clear_result(res);
}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 35a237a..2d2a9d0 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4537,8 +4537,8 @@ _copyDropSubscriptionStmt(const DropSubscriptionStmt *from)
DropSubscriptionStmt *newnode = makeNode(DropSubscriptionStmt);
COPY_STRING_FIELD(subname);
- COPY_SCALAR_FIELD(drop_slot);
COPY_SCALAR_FIELD(missing_ok);
+ COPY_SCALAR_FIELD(behavior);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 21dfbb0..b5459cd 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2246,8 +2246,8 @@ _equalDropSubscriptionStmt(const DropSubscriptionStmt *a,
const DropSubscriptionStmt *b)
{
COMPARE_STRING_FIELD(subname);
- COMPARE_SCALAR_FIELD(drop_slot);
COMPARE_SCALAR_FIELD(missing_ok);
+ COMPARE_SCALAR_FIELD(behavior);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 732647b..bdd2f6d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -415,7 +415,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <fun_param_mode> arg_class
%type <typnam> func_return func_type
-%type <boolean> opt_trusted opt_restart_seqs opt_drop_slot
+%type <boolean> opt_trusted opt_restart_seqs
%type <ival> OptTemp
%type <ival> OptNoLog
%type <oncommit> OnCommitOption
@@ -467,7 +467,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <node> def_arg columnElem where_clause where_or_current_clause
a_expr b_expr c_expr AexprConst indirection_el opt_slice_bound
columnref in_expr having_clause func_table xmltable array_expr
- ExclusionWhereClause
+ ExclusionWhereClause operator_def_arg
%type <list> rowsfrom_item rowsfrom_list opt_col_def_list
%type <boolean> opt_ordinality
%type <list> ExclusionConstraintList ExclusionConstraintElem
@@ -5684,6 +5684,7 @@ def_arg: func_type { $$ = (Node *)$1; }
| qual_all_Op { $$ = (Node *)$1; }
| NumericOnly { $$ = (Node *)$1; }
| Sconst { $$ = (Node *)makeString($1); }
+ | NONE { $$ = (Node *)makeString(pstrdup($1)); }
;
old_aggr_definition: '(' old_aggr_list ')' { $$ = $2; }
@@ -8923,8 +8924,16 @@ operator_def_list: operator_def_elem { $$ = list_make1($1); }
operator_def_elem: ColLabel '=' NONE
{ $$ = makeDefElem($1, NULL, @1); }
- | ColLabel '=' def_arg
- { $$ = makeDefElem($1, (Node *) $3, @1); }
+ | ColLabel '='operator_def_arg
+ { $$ = makeDefElem($1, (Node *) $3, @1); }
+ ;
+
+operator_def_arg:
+ func_type { $$ = (Node *)$1; }
+ | reserved_keyword { $$ = (Node *)makeString(pstrdup($1)); }
+ | qual_all_Op { $$ = (Node *)$1; }
+ | NumericOnly { $$ = (Node *)$1; }
+ | Sconst { $$ = (Node *)makeString($1); }
;
/*****************************************************************************
@@ -9315,42 +9324,24 @@ AlterSubscriptionStmt:
*
*****************************************************************************/
-DropSubscriptionStmt: DROP SUBSCRIPTION name opt_drop_slot
+DropSubscriptionStmt: DROP SUBSCRIPTION name opt_drop_behavior
{
DropSubscriptionStmt *n = makeNode(DropSubscriptionStmt);
n->subname = $3;
- n->drop_slot = $4;
n->missing_ok = false;
+ n->behavior = $4;
$$ = (Node *) n;
}
- | DROP SUBSCRIPTION IF_P EXISTS name opt_drop_slot
+ | DROP SUBSCRIPTION IF_P EXISTS name opt_drop_behavior
{
DropSubscriptionStmt *n = makeNode(DropSubscriptionStmt);
n->subname = $5;
- n->drop_slot = $6;
n->missing_ok = true;
+ n->behavior = $6;
$$ = (Node *) n;
}
;
-opt_drop_slot:
- DROP SLOT
- {
- $$ = TRUE;
- }
- | IDENT SLOT
- {
- if (strcmp($1, "nodrop") == 0)
- $$ = FALSE;
- else
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("unrecognized option \"%s\"", $1),
- parser_errposition(@1)));
- }
- | /*EMPTY*/ { $$ = TRUE; }
- ;
-
/*****************************************************************************
*
* QUERY: Define Rewrite Rule
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 2d7770d..a4b7dcb 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1329,6 +1329,22 @@ reread_subscription(void)
}
/*
+ * Exit if the subscription was disabled.
+ * This normally should not happen as the worker gets killed
+ * during ALTER SUBSCRIPTION ... DISABLE.
+ */
+ if (!newsub->enabled)
+ {
+ ereport(LOG,
+ (errmsg("logical replication worker for subscription \"%s\" will "
+ "stop because the subscription was disabled",
+ MySubscription->name)));
+
+ walrcv_disconnect(wrconn);
+ proc_exit(0);
+ }
+
+ /*
* Exit if connection string was changed. The launcher will start
* new worker.
*/
@@ -1358,6 +1374,9 @@ reread_subscription(void)
proc_exit(0);
}
+ /* !slotname should never happen when enabled is true. */
+ Assert(newsub->slotname);
+
/*
* We need to make new connection to new slot if slot name has changed
* so exit here as well if that's the case.
@@ -1388,22 +1407,6 @@ reread_subscription(void)
proc_exit(0);
}
- /*
- * Exit if the subscription was disabled.
- * This normally should not happen as the worker gets killed
- * during ALTER SUBSCRIPTION ... DISABLE.
- */
- if (!newsub->enabled)
- {
- ereport(LOG,
- (errmsg("logical replication worker for subscription \"%s\" will "
- "stop because the subscription was disabled",
- MySubscription->name)));
-
- walrcv_disconnect(wrconn);
- proc_exit(0);
- }
-
/* Check for other changes that should never happen too. */
if (newsub->dbid != MySubscription->dbid)
{
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 24e5c42..d4fa5a7 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -2303,6 +2303,9 @@ CreateCommandTag(Node *parsetree)
case OBJECT_PUBLICATION:
tag = "DROP PUBLICATION";
break;
+ case OBJECT_SUBSCRIPTION:
+ tag = "DROP SUBSCRIPTION";
+ break;
case OBJECT_STATISTIC_EXT:
tag = "DROP STATISTICS";
break;
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 5550f19..d4f3979 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -45,7 +45,7 @@ CATALOG(pg_subscription,6100) BKI_SHARED_RELATION BKI_ROWTYPE_OID(6101) BKI_SCHE
text subconninfo BKI_FORCE_NOT_NULL;
/* Slot name on publisher */
- NameData subslotname BKI_FORCE_NOT_NULL;
+ NameData subslotname;
/* Synchronous commit setting for worker */
text subsynccommit BKI_FORCE_NOT_NULL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e1d454a..46c23c2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3393,8 +3393,8 @@ typedef struct DropSubscriptionStmt
{
NodeTag type;
char *subname; /* Name of of the subscription */
- bool drop_slot; /* Should we drop the slot on remote side? */
bool missing_ok; /* Skip error if missing? */
+ DropBehavior behavior; /* RESTRICT or CASCADE behavior */
} DropSubscriptionStmt;
#endif /* PARSENODES_H */
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index f6fc29a..77bdc93 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -69,7 +69,7 @@ CREATE SCHEMA dummy_seclabel_test;
SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false, slot_name = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
@@ -111,7 +111,7 @@ NOTICE: event ddl_command_end: SECURITY LABEL
DROP EVENT TRIGGER always_start, always_end, always_drop, always_rewrite;
DROP VIEW dummy_seclabel_view1;
DROP TABLE dummy_seclabel_tbl1, dummy_seclabel_tbl2;
-DROP SUBSCRIPTION dummy_sub NODROP SLOT;
+DROP SUBSCRIPTION dummy_sub;
DROP PUBLICATION dummy_pub;
DROP ROLE regress_dummy_seclabel_user1;
DROP ROLE regress_dummy_seclabel_user2;
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
index d7795bd..8c347b6 100644
--- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
+++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
@@ -73,7 +73,7 @@ SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false, slot_name = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
@@ -108,7 +108,7 @@ DROP EVENT TRIGGER always_start, always_end, always_drop, always_rewrite;
DROP VIEW dummy_seclabel_view1;
DROP TABLE dummy_seclabel_tbl1, dummy_seclabel_tbl2;
-DROP SUBSCRIPTION dummy_sub NODROP SLOT;
+DROP SUBSCRIPTION dummy_sub;
DROP PUBLICATION dummy_pub;
DROP ROLE regress_dummy_seclabel_user1;
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 0849715..700f261 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -37,7 +37,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
@@ -477,7 +477,7 @@ SELECT (pg_identify_object(addr1.classid, addr1.objid, addr1.objsubid)).*,
SET client_min_messages TO 'warning';
DROP FOREIGN DATA WRAPPER addr_fdw CASCADE;
DROP PUBLICATION addr_pub;
-DROP SUBSCRIPTION addr_sub NODROP SLOT;
+DROP SUBSCRIPTION addr_sub;
DROP SCHEMA addr_nsp CASCADE;
DROP OWNED BY regress_addr_user;
DROP USER regress_addr_user;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 33d53de..fcf5646 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -109,17 +109,18 @@ HINT: The owner of a subscription must be a superuser.
ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
--- fail - cannot do DROP SUBSCRIPTION DROP SLOT inside transaction block
+ALTER SUBSCRIPTION testsub WITH (slot_name = NONE);
+-- fail - cannot do DROP SUBSCRIPTION CASCADE inside transaction block
BEGIN;
-DROP SUBSCRIPTION testsub DROP SLOT;
-ERROR: DROP SUBSCRIPTION ... DROP SLOT cannot run inside a transaction block
+DROP SUBSCRIPTION testsub CASCADE;
+ERROR: DROP SUBSCRIPTION ... CASCADE cannot run inside a transaction block
COMMIT;
BEGIN;
-DROP SUBSCRIPTION testsub NODROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
-DROP SUBSCRIPTION IF EXISTS testsub NODROP SLOT;
+DROP SUBSCRIPTION IF EXISTS testsub;
NOTICE: subscription "testsub" does not exist, skipping
-DROP SUBSCRIPTION testsub NODROP SLOT; -- fail
+DROP SUBSCRIPTION testsub; -- fail
ERROR: subscription "testsub" does not exist
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index c698a63..8a738e2 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -40,7 +40,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
@@ -205,7 +205,7 @@ SET client_min_messages TO 'warning';
DROP FOREIGN DATA WRAPPER addr_fdw CASCADE;
DROP PUBLICATION addr_pub;
-DROP SUBSCRIPTION addr_sub NODROP SLOT;
+DROP SUBSCRIPTION addr_sub;
DROP SCHEMA addr_nsp CASCADE;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 5454420..c27fee5 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -80,17 +80,19 @@ ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
--- fail - cannot do DROP SUBSCRIPTION DROP SLOT inside transaction block
+ALTER SUBSCRIPTION testsub WITH (slot_name = NONE);
+
+-- fail - cannot do DROP SUBSCRIPTION CASCADE inside transaction block
BEGIN;
-DROP SUBSCRIPTION testsub DROP SLOT;
+DROP SUBSCRIPTION testsub CASCADE;
COMMIT;
BEGIN;
-DROP SUBSCRIPTION testsub NODROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
-DROP SUBSCRIPTION IF EXISTS testsub NODROP SLOT;
-DROP SUBSCRIPTION testsub NODROP SLOT; -- fail
+DROP SUBSCRIPTION IF EXISTS testsub;
+DROP SUBSCRIPTION testsub; -- fail
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index b4c5539..be55279 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -191,7 +191,7 @@ $node_publisher->poll_query_until('postgres',
or die "Timed out while waiting for apply to restart";
# check all the cleanup
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_renamed DROP SLOT");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_renamed CASCADE");
$result =
$node_subscriber->safe_psql('postgres', "SELECT count(*) FROM pg_subscription");
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index 8ece7dd..779c668 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -52,7 +52,7 @@ my $result =
is($result, qq(10), 'initial data synced for first sub');
# drop subscription so that there is unreplicated data
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub CASCADE");
$node_publisher->safe_psql('postgres',
"INSERT INTO tab_rep SELECT generate_series(11,20)");
@@ -89,8 +89,8 @@ $node_subscriber->poll_query_until('postgres', "SELECT pid IS NOT NULL FROM pg_s
or die "Timed out while waiting for subscriber to start";
# and drop both subscriptions
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub2");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub CASCADE");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub2 CASCADE");
# check subscriptions are removed
$result =
@@ -154,7 +154,7 @@ $result =
$node_subscriber->safe_psql('postgres', "SELECT count(*) FROM tab_rep_next");
is($result, qq(20), 'changes for table added after subscription initialized replicated');
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub CASCADE");
$node_subscriber->stop('fast');
$node_publisher->stop('fast');
--
2.7.4
On Tue, May 02, 2017 at 09:10:52AM -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:DROP SUBSCRIPTION mysub NODROP SLOT;
I'm pretty uninspired by this choice of syntax.
Actually, this command has got much worse problems than whether you like
the spelling of its option: it shouldn't have an option in the first
place. I put it to you as an inviolable rule that no object DROP command
should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
If it does, then you don't know what to do when the object is recursed
to by a cascaded drop.It's possible that we could allow exceptions to this rule for object types that can never have any dependencies. But a subscription doesn't qualify --- it's got an owner, so DROP OWNED BY already poses this problem. Looks to me like it's got a dependency on a database, too. (BTW, if subscriptions are per-database, why is pg_subscription a shared catalog? There were some pretty schizophrenic decisions here.)So ISTM we need to get rid of the above-depicted syntax. We could instead
have what-to-do-with-the-slot as a property of the subscription,
established at CREATE SUBSCRIPTION and possibly changed later by ALTER
SUBSCRIPTION. Not quite sure what to call the option, maybe something
based on the concept of the subscription "owning" the slot.I think this is a must-fix issue, and will put it on the open items
list.
[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 Tue, May 02, 2017 at 01:42:37PM -0400, Robert Haas wrote:
On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:I am happy to implement something different, it's quite trivial to
change. But I am not going to propose anything different as I can't
think of better syntax (if I could I would have done it). I don't like
the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
it also seems to not map very well to action (as opposed to output
option as it is in EXPLAIN). It might not be very close to SQL way but
that's because SQL way would be do not do any of those default actions
unless they are actually asked for (ie NODROP SLOT would be default and
DROP SLOT would be the option) but that's IMHO less user friendly.So the cases where this "NO" prefixing comes up are:
1. CREATE SUBSCRIPTION
...
2. ALTER SUBSCRIPTION
...
3. DROP SUBSCRIPTION
...
4. CREATE PUBLICATION
...
So it doesn't actually look hard to get rid of all of the NO prefixes.
[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/5/17 13:01, Petr Jelinek wrote:
What do you think of attached. I actually did add RESTRICT/CASCADE, in a
way that if there is slot RESTRICT will refuse to drop subscription and
CASCADE will try to drop it. Still all errors.The new way to not drop slot is to set slot_name to NONE which is new
value that I invented (inspiration from OWNED BY sequences) which
basically disassociates the subscription from slot.It's slightly less automatic this way but perhaps that's not a bad
thing, at least nobody will drop slots by mistake while we still make
sure that slots are not left over without anybody noticing.
I think this is OK. Could you send a version of this patch based on
master please?
--
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 5/7/17 04:21, Noah Misch wrote:
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 think we have a workable patch, but it needs some rebasing. I hope to
have this sorted by Wednesday.
--
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 08/05/17 22:55, Peter Eisentraut wrote:
On 5/5/17 13:01, Petr Jelinek wrote:
What do you think of attached. I actually did add RESTRICT/CASCADE, in a
way that if there is slot RESTRICT will refuse to drop subscription and
CASCADE will try to drop it. Still all errors.The new way to not drop slot is to set slot_name to NONE which is new
value that I invented (inspiration from OWNED BY sequences) which
basically disassociates the subscription from slot.It's slightly less automatic this way but perhaps that's not a bad
thing, at least nobody will drop slots by mistake while we still make
sure that slots are not left over without anybody noticing.I think this is OK. Could you send a version of this patch based on
master please?
Here it is.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPTION-0508.patchbinary/octet-stream; name=Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPTION-0508.patchDownload
From 0b2b9a774fc636807f9515eecc2bf01476415d37 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Mon, 8 May 2017 23:53:02 +0200
Subject: [PATCH] Remove the NODROP SLOT option from DROP SUBSCRIPTION
---
doc/src/sgml/ref/create_subscription.sgml | 8 +
doc/src/sgml/ref/drop_subscription.sgml | 22 +--
src/backend/catalog/pg_subscription.c | 9 +-
src/backend/commands/subscriptioncmds.c | 195 ++++++++++++++++-----
src/backend/nodes/copyfuncs.c | 2 +-
src/backend/nodes/equalfuncs.c | 2 +-
src/backend/parser/gram.y | 43 ++---
src/backend/replication/logical/worker.c | 35 ++--
src/backend/tcop/utility.c | 3 +
src/include/catalog/pg_subscription.h | 2 +-
src/include/nodes/parsenodes.h | 2 +-
.../dummy_seclabel/expected/dummy_seclabel.out | 4 +-
.../modules/dummy_seclabel/sql/dummy_seclabel.sql | 4 +-
src/test/regress/expected/object_address.out | 4 +-
src/test/regress/expected/subscription.out | 13 +-
src/test/regress/sql/object_address.sql | 4 +-
src/test/regress/sql/subscription.sql | 12 +-
src/test/subscription/t/001_rep_changes.pl | 2 +-
src/test/subscription/t/004_sync.pl | 8 +-
19 files changed, 242 insertions(+), 132 deletions(-)
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 3c51012..c22bb20 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -133,6 +133,14 @@ CREATE SUBSCRIPTION <replaceable class="PARAMETER">subscription_name</replaceabl
Name of the replication slot to use. The default behavior is to use
<literal>subscription_name</> for slot name.
</para>
+
+ <para>
+ When <literal>SLOT NAME</literal> is set to
+ <literal>NONE</literal> there will be no slot associated with the
+ subscription. Such subscriptions must also have both
+ <literal>ENABLED</literal> and <literal>CREATE SLOT</literal> set
+ to <literal>false</literal>.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml
index f1ac125..8b00c45 100644
--- a/doc/src/sgml/ref/drop_subscription.sgml
+++ b/doc/src/sgml/ref/drop_subscription.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ DROP SLOT | NODROP SLOT ]
+DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
@@ -57,20 +57,16 @@ DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable
</varlistentry>
<varlistentry>
- <term><literal>DROP SLOT</literal></term>
- <term><literal>NODROP SLOT</literal></term>
- <listitem>
- <para>
- Specifies whether to drop the replication slot on the publisher. The
- default is
- <literal>DROP SLOT</literal>.
- </para>
+ <term><literal>CASCADE</literal></term>
+ <term><literal>RESTRICT</literal></term>
+ <listitem>
<para>
- If the publisher is not reachable when the subscription is to be
- dropped, then it is useful to specify <literal>NODROP SLOT</literal>.
- But the replication slot on the publisher will then have to be removed
- manually.
+ These key words are used to determine what to do with when there is a
+ replication slot associated with the subscription. The
+ <literal>RESTRICT</literal> will refuse to drop the subscription in
+ such case, while <literal>CASCADE</literal> will drop the associated
+ slot. <literal>RESTRICT</literal> is the default.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 22587a4..7dc21f1 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -82,8 +82,10 @@ GetSubscription(Oid subid, bool missing_ok)
tup,
Anum_pg_subscription_subslotname,
&isnull);
- Assert(!isnull);
- sub->slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ if (!isnull)
+ sub->slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ else
+ sub->slotname = NULL;
/* Get synccommit */
datum = SysCacheGetAttr(SUBSCRIPTIONOID,
@@ -147,7 +149,8 @@ FreeSubscription(Subscription *sub)
{
pfree(sub->name);
pfree(sub->conninfo);
- pfree(sub->slotname);
+ if (sub->slotname)
+ pfree(sub->slotname);
list_free_deep(sub->publications);
pfree(sub);
}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index fde9e6e..ba0d4b8 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -60,7 +60,8 @@ static List *fetch_table_list(WalReceiverConn *wrconn, List *publications);
*/
static void
parse_subscription_options(List *options, bool *connect, bool *enabled_given,
- bool *enabled, bool *create_slot, char **slot_name,
+ bool *enabled, bool *create_slot,
+ bool *slot_name_given, char **slot_name,
bool *copy_data, char **synchronous_commit)
{
ListCell *lc;
@@ -78,7 +79,10 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (create_slot)
*create_slot = true;
if (slot_name)
+ {
+ *slot_name_given = false;
*slot_name = NULL;
+ }
if (copy_data)
*copy_data = true;
if (synchronous_commit)
@@ -141,12 +145,17 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
}
else if (strcmp(defel->defname, "slot name") == 0 && slot_name)
{
- if (*slot_name)
+ if (*slot_name_given)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
+ *slot_name_given = true;
*slot_name = defGetString(defel);
+
+ /* Setting slot_name = NONE is treated as no slot name. */
+ if (strcmp(*slot_name, "none") == 0)
+ *slot_name = NULL;
}
else if (strcmp(defel->defname, "copy data") == 0 && copy_data)
{
@@ -194,26 +203,43 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (connect && !*connect)
{
/* Check for incompatible options from the user. */
- if (*enabled_given && *enabled)
+ if (enabled && *enabled_given && *enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("noconnect and enabled are mutually exclusive options")));
+ errmsg("connect = false and enabled are mutually exclusive options")));
- if (create_slot_given && *create_slot)
+ if (create_slot && create_slot_given && *create_slot)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("noconnect and create slot are mutually exclusive options")));
+ errmsg("connect = false and create_slot are mutually exclusive options")));
- if (copy_data_given && *copy_data)
+ if (copy_data && copy_data_given && *copy_data)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("noconnect and copy data are mutually exclusive options")));
+ errmsg("connect = false and copy_data are mutually exclusive options")));
/* Change the defaults of other options. */
*enabled = false;
*create_slot = false;
*copy_data = false;
}
+
+ /*
+ * Do additional checking for disallowed combination when
+ * slot_name = NONE was used.
+ */
+ if (slot_name && *slot_name_given && !*slot_name)
+ {
+ if (enabled && *enabled_given && *enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("slot_name = NONE and enabled are mutually exclusive options")));
+
+ if (create_slot && create_slot_given && *create_slot)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("slot_name = NONE and create_slot are mutually exclusive options")));
+ }
}
/*
@@ -290,6 +316,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
char *synchronous_commit;
char *conninfo;
char *slotname;
+ bool slotname_given;
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
@@ -299,8 +326,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
* Connection and publication should not be specified here.
*/
parse_subscription_options(stmt->options, &connect, &enabled_given,
- &enabled, &create_slot, &slotname, ©_data,
- &synchronous_commit);
+ &enabled, &create_slot, &slotname_given,
+ &slotname, ©_data, &synchronous_commit);
/*
* Since creating a replication slot is not transactional, rolling back
@@ -329,8 +356,9 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
stmt->subname)));
}
- if (slotname == NULL)
+ if (!slotname_given && slotname == NULL)
slotname = stmt->subname;
+
/* The default for synchronous_commit of subscriptions is off. */
if (synchronous_commit == NULL)
synchronous_commit = "off";
@@ -355,8 +383,11 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
values[Anum_pg_subscription_subenabled - 1] = BoolGetDatum(enabled);
values[Anum_pg_subscription_subconninfo - 1] =
CStringGetTextDatum(conninfo);
- values[Anum_pg_subscription_subslotname - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ if (slotname)
+ values[Anum_pg_subscription_subslotname - 1] =
+ DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ else
+ nulls[Anum_pg_subscription_subslotname - 1] = true;
values[Anum_pg_subscription_subsynccommit - 1] =
CStringGetTextDatum(synchronous_commit);
values[Anum_pg_subscription_subpublications - 1] =
@@ -426,6 +457,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
*/
if (create_slot)
{
+ Assert(slotname);
+
walrcv_create_slot(wrconn, slotname, false,
CRS_NOEXPORT_SNAPSHOT, &lsn);
ereport(NOTICE,
@@ -578,6 +611,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
HeapTuple tup;
Oid subid;
bool update_tuple = false;
+ Subscription *sub;
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -597,6 +631,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
stmt->subname);
subid = HeapTupleGetOid(tup);
+ sub = GetSubscription(subid, false);
/* Form a new tuple. */
memset(values, 0, sizeof(values));
@@ -607,19 +642,29 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
{
case ALTER_SUBSCRIPTION_OPTIONS:
{
- char *slot_name;
- char *synchronous_commit;
+ char *slotname;
+ bool slotname_given;
+ char *synchronous_commit;
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, &slot_name, NULL,
- &synchronous_commit);
+ NULL, &slotname_given, &slotname,
+ NULL, &synchronous_commit);
- if (slot_name)
+ if (slotname_given)
{
- values[Anum_pg_subscription_subslotname - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(slot_name));
+ if (sub->enabled && !slotname)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot set slot_name = NONE for enabled subscription")));
+
+ if (slotname)
+ values[Anum_pg_subscription_subslotname - 1] =
+ DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ else
+ nulls[Anum_pg_subscription_subslotname - 1] = true;
replaces[Anum_pg_subscription_subslotname - 1] = true;
}
+
if (synchronous_commit)
{
values[Anum_pg_subscription_subsynccommit - 1] =
@@ -638,9 +683,14 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
parse_subscription_options(stmt->options, NULL,
&enabled_given, &enabled, NULL,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
Assert(enabled_given);
+ if (!sub->slotname && enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot enable subscription which does not have a slot_name")));
+
values[Anum_pg_subscription_subenabled - 1] =
BoolGetDatum(enabled);
replaces[Anum_pg_subscription_subenabled - 1] = true;
@@ -668,10 +718,10 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
case ALTER_SUBSCRIPTION_PUBLICATION_REFRESH:
{
bool copy_data;
- Subscription *sub = GetSubscription(subid, false);
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, NULL, ©_data, NULL);
+ NULL, NULL, NULL, ©_data,
+ NULL);
values[Anum_pg_subscription_subpublications - 1] =
publicationListToArray(stmt->publication);
@@ -682,6 +732,11 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
/* Refresh if user asked us to. */
if (stmt->kind == ALTER_SUBSCRIPTION_PUBLICATION_REFRESH)
{
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
+
/* Make sure refresh sees the new list of publications. */
sub->publications = stmt->publication;
@@ -694,10 +749,15 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
case ALTER_SUBSCRIPTION_REFRESH:
{
bool copy_data;
- Subscription *sub = GetSubscription(subid, false);
+
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, NULL, ©_data, NULL);
+ NULL, NULL, NULL, ©_data,
+ NULL);
AlterSubscription_refresh(sub, copy_data);
@@ -756,8 +816,8 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
* run DROP SUBSCRIPTION inside a transaction block if dropping the
* replication slot.
*/
- if (stmt->drop_slot)
- PreventTransactionChain(isTopLevel, "DROP SUBSCRIPTION ... DROP SLOT");
+ if (stmt->behavior == DROP_CASCADE)
+ PreventTransactionChain(isTopLevel, "DROP SUBSCRIPTION ... CASCADE");
/*
* Lock pg_subscription with AccessExclusiveLock to ensure
@@ -817,8 +877,10 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
/* Get slotname */
datum = SysCacheGetAttr(SUBSCRIPTIONOID, tup,
Anum_pg_subscription_subslotname, &isnull);
- Assert(!isnull);
- slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ if (!isnull)
+ slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ else
+ slotname = NULL;
ObjectAddressSet(myself, SubscriptionRelationId, subid);
EventTriggerSQLDropAddObject(&myself, true, true);
@@ -843,43 +905,84 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
if (originid != InvalidRepOriginId)
replorigin_drop(originid);
- /* If the user asked to not drop the slot, we are done mow.*/
- if (!stmt->drop_slot)
+ /* If there is no slot associated with subscription we can finish here. */
+ if (!slotname)
{
heap_close(rel, NoLock);
return;
}
/*
- * Otherwise drop the replication slot at the publisher node using
+ * Otherwise check for the replication slot at the publisher node using
* the replication connection.
*/
load_file("libpqwalreceiver", false);
- initStringInfo(&cmd);
- appendStringInfo(&cmd, "DROP_REPLICATION_SLOT \"%s\"", slotname);
-
wrconn = walrcv_connect(conninfo, true, subname, &err);
if (wrconn == NULL)
ereport(ERROR,
- (errmsg("could not connect to publisher when attempting to "
- "drop the replication slot \"%s\"", slotname),
- errdetail("The error was: %s", err)));
+ (errmsg("could not connect to publisher when attempting to fetch "
+ "information about replication slot \"%s\"", slotname),
+ errdetail("The error was: %s", err),
+ errhint("Use ALTER SUBSCRIPTION ... WITH (slot_name = NONE) "
+ "to disassociate the subscription from slot.")));
PG_TRY();
{
WalRcvExecResult *res;
- res = walrcv_exec(wrconn, cmd.data, 0, NULL);
-
- if (res->status != WALRCV_OK_COMMAND)
+ TupleTableSlot *tupslot;
+ Oid slot_row_desc[1] = {BOOLOID};
+ bool found;
+
+ initStringInfo(&cmd);
+ appendStringInfo(&cmd,
+ "SELECT true FROM pg_catalog.pg_replication_slots WHERE slot_name = %s",
+ quote_literal_cstr(slotname));
+ res = walrcv_exec(wrconn, cmd.data, 1, slot_row_desc);
+ if (res->status != WALRCV_OK_TUPLES)
ereport(ERROR,
- (errmsg("could not drop the replication slot \"%s\" on publisher",
- slotname),
+ (errmsg("could not fetch the replication slot info from publisher"),
errdetail("The error was: %s", res->err)));
+
+ tupslot = MakeSingleTupleTableSlot(res->tupledesc);
+ found = tuplestore_gettupleslot(res->tuplestore, true, false,
+ tupslot);
+ ExecDropSingleTupleTableSlot(tupslot);
+ walrcv_clear_result(res);
+
+ /* If slot was not found on publisher, we are done. */
+ if (!found)
+ {
+ walrcv_disconnect(wrconn);
+ pfree(cmd.data);
+ heap_close(rel, NoLock);
+ return;
+ }
+
+ /* Otherwise the next action depends on the drop_behavior. */
+ if (stmt->behavior == DROP_CASCADE)
+ {
+ resetStringInfo(&cmd);
+ appendStringInfo(&cmd, "DROP_REPLICATION_SLOT %s",
+ quote_identifier(slotname));
+ res = walrcv_exec(wrconn, cmd.data, 0, NULL);
+
+ if (res->status != WALRCV_OK_COMMAND)
+ ereport(ERROR,
+ (errmsg("could not drop the replication slot \"%s\" on publisher",
+ slotname),
+ errdetail("The error was: %s", res->err)));
+ else
+ ereport(NOTICE,
+ (errmsg("dropped replication slot \"%s\" on publisher",
+ slotname)));
+ }
else
- ereport(NOTICE,
- (errmsg("dropped replication slot \"%s\" on publisher",
- slotname)));
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot drop subscription \"%s\" because there is still replication slot associated with it",
+ subname),
+ errhint("Use DROP ... CASCADE to drop the slot too.")));
walrcv_clear_result(res);
}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 35a237a..2d2a9d0 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4537,8 +4537,8 @@ _copyDropSubscriptionStmt(const DropSubscriptionStmt *from)
DropSubscriptionStmt *newnode = makeNode(DropSubscriptionStmt);
COPY_STRING_FIELD(subname);
- COPY_SCALAR_FIELD(drop_slot);
COPY_SCALAR_FIELD(missing_ok);
+ COPY_SCALAR_FIELD(behavior);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 21dfbb0..b5459cd 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2246,8 +2246,8 @@ _equalDropSubscriptionStmt(const DropSubscriptionStmt *a,
const DropSubscriptionStmt *b)
{
COMPARE_STRING_FIELD(subname);
- COMPARE_SCALAR_FIELD(drop_slot);
COMPARE_SCALAR_FIELD(missing_ok);
+ COMPARE_SCALAR_FIELD(behavior);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2cad8b2..2537f84 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -415,7 +415,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <fun_param_mode> arg_class
%type <typnam> func_return func_type
-%type <boolean> opt_trusted opt_restart_seqs opt_drop_slot
+%type <boolean> opt_trusted opt_restart_seqs
%type <ival> OptTemp
%type <ival> OptNoLog
%type <oncommit> OnCommitOption
@@ -467,7 +467,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <node> def_arg columnElem where_clause where_or_current_clause
a_expr b_expr c_expr AexprConst indirection_el opt_slice_bound
columnref in_expr having_clause func_table xmltable array_expr
- ExclusionWhereClause
+ ExclusionWhereClause operator_def_arg
%type <list> rowsfrom_item rowsfrom_list opt_col_def_list
%type <boolean> opt_ordinality
%type <list> ExclusionConstraintList ExclusionConstraintElem
@@ -5694,6 +5694,7 @@ def_arg: func_type { $$ = (Node *)$1; }
| qual_all_Op { $$ = (Node *)$1; }
| NumericOnly { $$ = (Node *)$1; }
| Sconst { $$ = (Node *)makeString($1); }
+ | NONE { $$ = (Node *)makeString(pstrdup($1)); }
;
old_aggr_definition: '(' old_aggr_list ')' { $$ = $2; }
@@ -8933,8 +8934,16 @@ operator_def_list: operator_def_elem { $$ = list_make1($1); }
operator_def_elem: ColLabel '=' NONE
{ $$ = makeDefElem($1, NULL, @1); }
- | ColLabel '=' def_arg
- { $$ = makeDefElem($1, (Node *) $3, @1); }
+ | ColLabel '='operator_def_arg
+ { $$ = makeDefElem($1, (Node *) $3, @1); }
+ ;
+
+operator_def_arg:
+ func_type { $$ = (Node *)$1; }
+ | reserved_keyword { $$ = (Node *)makeString(pstrdup($1)); }
+ | qual_all_Op { $$ = (Node *)$1; }
+ | NumericOnly { $$ = (Node *)$1; }
+ | Sconst { $$ = (Node *)makeString($1); }
;
/*****************************************************************************
@@ -9324,42 +9333,24 @@ AlterSubscriptionStmt:
*
*****************************************************************************/
-DropSubscriptionStmt: DROP SUBSCRIPTION name opt_drop_slot
+DropSubscriptionStmt: DROP SUBSCRIPTION name opt_drop_behavior
{
DropSubscriptionStmt *n = makeNode(DropSubscriptionStmt);
n->subname = $3;
- n->drop_slot = $4;
n->missing_ok = false;
+ n->behavior = $4;
$$ = (Node *) n;
}
- | DROP SUBSCRIPTION IF_P EXISTS name opt_drop_slot
+ | DROP SUBSCRIPTION IF_P EXISTS name opt_drop_behavior
{
DropSubscriptionStmt *n = makeNode(DropSubscriptionStmt);
n->subname = $5;
- n->drop_slot = $6;
n->missing_ok = true;
+ n->behavior = $6;
$$ = (Node *) n;
}
;
-opt_drop_slot:
- DROP SLOT
- {
- $$ = TRUE;
- }
- | IDENT SLOT
- {
- if (strcmp($1, "nodrop") == 0)
- $$ = FALSE;
- else
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("unrecognized option \"%s\"", $1),
- parser_errposition(@1)));
- }
- | /*EMPTY*/ { $$ = TRUE; }
- ;
-
/*****************************************************************************
*
* QUERY: Define Rewrite Rule
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index a61240c..362de12 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1329,6 +1329,22 @@ reread_subscription(void)
}
/*
+ * Exit if the subscription was disabled.
+ * This normally should not happen as the worker gets killed
+ * during ALTER SUBSCRIPTION ... DISABLE.
+ */
+ if (!newsub->enabled)
+ {
+ ereport(LOG,
+ (errmsg("logical replication worker for subscription \"%s\" will "
+ "stop because the subscription was disabled",
+ MySubscription->name)));
+
+ walrcv_disconnect(wrconn);
+ proc_exit(0);
+ }
+
+ /*
* Exit if connection string was changed. The launcher will start
* new worker.
*/
@@ -1358,6 +1374,9 @@ reread_subscription(void)
proc_exit(0);
}
+ /* !slotname should never happen when enabled is true. */
+ Assert(newsub->slotname);
+
/*
* We need to make new connection to new slot if slot name has changed
* so exit here as well if that's the case.
@@ -1388,22 +1407,6 @@ reread_subscription(void)
proc_exit(0);
}
- /*
- * Exit if the subscription was disabled.
- * This normally should not happen as the worker gets killed
- * during ALTER SUBSCRIPTION ... DISABLE.
- */
- if (!newsub->enabled)
- {
- ereport(LOG,
- (errmsg("logical replication worker for subscription \"%s\" will "
- "stop because the subscription was disabled",
- MySubscription->name)));
-
- walrcv_disconnect(wrconn);
- proc_exit(0);
- }
-
/* Check for other changes that should never happen too. */
if (newsub->dbid != MySubscription->dbid)
{
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 24e5c42..d4fa5a7 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -2303,6 +2303,9 @@ CreateCommandTag(Node *parsetree)
case OBJECT_PUBLICATION:
tag = "DROP PUBLICATION";
break;
+ case OBJECT_SUBSCRIPTION:
+ tag = "DROP SUBSCRIPTION";
+ break;
case OBJECT_STATISTIC_EXT:
tag = "DROP STATISTICS";
break;
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 5550f19..d4f3979 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -45,7 +45,7 @@ CATALOG(pg_subscription,6100) BKI_SHARED_RELATION BKI_ROWTYPE_OID(6101) BKI_SCHE
text subconninfo BKI_FORCE_NOT_NULL;
/* Slot name on publisher */
- NameData subslotname BKI_FORCE_NOT_NULL;
+ NameData subslotname;
/* Synchronous commit setting for worker */
text subsynccommit BKI_FORCE_NOT_NULL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e1d454a..46c23c2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3393,8 +3393,8 @@ typedef struct DropSubscriptionStmt
{
NodeTag type;
char *subname; /* Name of of the subscription */
- bool drop_slot; /* Should we drop the slot on remote side? */
bool missing_ok; /* Skip error if missing? */
+ DropBehavior behavior; /* RESTRICT or CASCADE behavior */
} DropSubscriptionStmt;
#endif /* PARSENODES_H */
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index 27c8ec5..5f37681 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -69,7 +69,7 @@ CREATE SCHEMA dummy_seclabel_test;
SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT, SLOT NAME = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
@@ -111,7 +111,7 @@ NOTICE: event ddl_command_end: SECURITY LABEL
DROP EVENT TRIGGER always_start, always_end, always_drop, always_rewrite;
DROP VIEW dummy_seclabel_view1;
DROP TABLE dummy_seclabel_tbl1, dummy_seclabel_tbl2;
-DROP SUBSCRIPTION dummy_sub NODROP SLOT;
+DROP SUBSCRIPTION dummy_sub;
DROP PUBLICATION dummy_pub;
DROP ROLE regress_dummy_seclabel_user1;
DROP ROLE regress_dummy_seclabel_user2;
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
index 8d43244..97311c7 100644
--- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
+++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
@@ -73,7 +73,7 @@ SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT, SLOT NAME = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
@@ -108,7 +108,7 @@ DROP EVENT TRIGGER always_start, always_end, always_drop, always_rewrite;
DROP VIEW dummy_seclabel_view1;
DROP TABLE dummy_seclabel_tbl1, dummy_seclabel_tbl2;
-DROP SUBSCRIPTION dummy_sub NODROP SLOT;
+DROP SUBSCRIPTION dummy_sub;
DROP PUBLICATION dummy_pub;
DROP ROLE regress_dummy_seclabel_user1;
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 814e05e..40eeeed 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -37,7 +37,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCONNECT);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (NOCONNECT, SLOT NAME = NONE);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
@@ -477,7 +477,7 @@ SELECT (pg_identify_object(addr1.classid, addr1.objid, addr1.objsubid)).*,
SET client_min_messages TO 'warning';
DROP FOREIGN DATA WRAPPER addr_fdw CASCADE;
DROP PUBLICATION addr_pub;
-DROP SUBSCRIPTION addr_sub NODROP SLOT;
+DROP SUBSCRIPTION addr_sub;
DROP SCHEMA addr_nsp CASCADE;
DROP OWNED BY regress_addr_user;
DROP USER regress_addr_user;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index fd09f54..154e913 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -113,17 +113,18 @@ HINT: The owner of a subscription must be a superuser.
ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
--- fail - cannot do DROP SUBSCRIPTION DROP SLOT inside transaction block
+ALTER SUBSCRIPTION testsub WITH (SLOT NAME = NONE);
+-- fail - cannot do DROP SUBSCRIPTION CASCADE inside transaction block
BEGIN;
-DROP SUBSCRIPTION testsub DROP SLOT;
-ERROR: DROP SUBSCRIPTION ... DROP SLOT cannot run inside a transaction block
+DROP SUBSCRIPTION testsub CASCADE;
+ERROR: DROP SUBSCRIPTION ... CASCADE cannot run inside a transaction block
COMMIT;
BEGIN;
-DROP SUBSCRIPTION testsub NODROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
-DROP SUBSCRIPTION IF EXISTS testsub NODROP SLOT;
+DROP SUBSCRIPTION IF EXISTS testsub;
NOTICE: subscription "testsub" does not exist, skipping
-DROP SUBSCRIPTION testsub NODROP SLOT; -- fail
+DROP SUBSCRIPTION testsub; -- fail
ERROR: subscription "testsub" does not exist
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index c9219e4..6940392 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -40,7 +40,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCONNECT);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (NOCONNECT, SLOT NAME = NONE);
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
@@ -205,7 +205,7 @@ SET client_min_messages TO 'warning';
DROP FOREIGN DATA WRAPPER addr_fdw CASCADE;
DROP PUBLICATION addr_pub;
-DROP SUBSCRIPTION addr_sub NODROP SLOT;
+DROP SUBSCRIPTION addr_sub;
DROP SCHEMA addr_nsp CASCADE;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index db05f52..e38a33a 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -83,17 +83,19 @@ ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
--- fail - cannot do DROP SUBSCRIPTION DROP SLOT inside transaction block
+ALTER SUBSCRIPTION testsub WITH (SLOT NAME = NONE);
+
+-- fail - cannot do DROP SUBSCRIPTION CASCADE inside transaction block
BEGIN;
-DROP SUBSCRIPTION testsub DROP SLOT;
+DROP SUBSCRIPTION testsub CASCADE;
COMMIT;
BEGIN;
-DROP SUBSCRIPTION testsub NODROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
-DROP SUBSCRIPTION IF EXISTS testsub NODROP SLOT;
-DROP SUBSCRIPTION testsub NODROP SLOT; -- fail
+DROP SUBSCRIPTION IF EXISTS testsub;
+DROP SUBSCRIPTION testsub; -- fail
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index d1817f5..aa150a9 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -191,7 +191,7 @@ $node_publisher->poll_query_until('postgres',
or die "Timed out while waiting for apply to restart";
# check all the cleanup
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_renamed DROP SLOT");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_renamed CASCADE");
$result =
$node_subscriber->safe_psql('postgres', "SELECT count(*) FROM pg_subscription");
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index fa0bf7f..7f309ae 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -52,7 +52,7 @@ my $result =
is($result, qq(10), 'initial data synced for first sub');
# drop subscription so that there is unreplicated data
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub CASCADE");
$node_publisher->safe_psql('postgres',
"INSERT INTO tab_rep SELECT generate_series(11,20)");
@@ -89,8 +89,8 @@ $node_subscriber->poll_query_until('postgres', "SELECT pid IS NOT NULL FROM pg_s
or die "Timed out while waiting for subscriber to start";
# and drop both subscriptions
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub2");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub CASCADE");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub2 CASCADE");
# check subscriptions are removed
$result =
@@ -154,7 +154,7 @@ $result =
$node_subscriber->safe_psql('postgres', "SELECT count(*) FROM tab_rep_next");
is($result, qq(20), 'changes for table added after subscription initialized replicated');
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub CASCADE");
$node_subscriber->stop('fast');
$node_publisher->stop('fast');
--
2.7.4
On 5/8/17 17:55, Petr Jelinek wrote:
On 08/05/17 22:55, Peter Eisentraut wrote:
On 5/5/17 13:01, Petr Jelinek wrote:
What do you think of attached. I actually did add RESTRICT/CASCADE, in a
way that if there is slot RESTRICT will refuse to drop subscription and
CASCADE will try to drop it. Still all errors.The new way to not drop slot is to set slot_name to NONE which is new
value that I invented (inspiration from OWNED BY sequences) which
basically disassociates the subscription from slot.It's slightly less automatic this way but perhaps that's not a bad
thing, at least nobody will drop slots by mistake while we still make
sure that slots are not left over without anybody noticing.I think this is OK. Could you send a version of this patch based on
master please?Here it is.
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.
What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.
I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.
Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.
--
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 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.
Here is your patch amended for that.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPT.patchinvalid/octet-stream; name=v2-0001-Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPT.patchDownload
From 84b7305e58e03a4e0bb1f5c1d26ac0e2f51490ed Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Mon, 8 May 2017 23:53:02 +0200
Subject: [PATCH v2] Remove the NODROP SLOT option from DROP SUBSCRIPTION
---
doc/src/sgml/ref/create_subscription.sgml | 8 ++
doc/src/sgml/ref/drop_subscription.sgml | 19 +--
src/backend/catalog/pg_subscription.c | 9 +-
src/backend/commands/subscriptioncmds.c | 139 +++++++++++++++------
src/backend/nodes/copyfuncs.c | 2 +-
src/backend/nodes/equalfuncs.c | 2 +-
src/backend/parser/gram.y | 46 +++----
src/backend/replication/logical/worker.c | 35 +++---
src/include/catalog/pg_subscription.h | 2 +-
src/include/nodes/parsenodes.h | 2 +-
src/include/parser/kwlist.h | 1 -
.../dummy_seclabel/expected/dummy_seclabel.out | 4 +-
.../modules/dummy_seclabel/sql/dummy_seclabel.sql | 4 +-
src/test/regress/expected/object_address.out | 4 +-
src/test/regress/expected/subscription.out | 14 ++-
src/test/regress/sql/object_address.sql | 4 +-
src/test/regress/sql/subscription.sql | 13 +-
src/test/subscription/t/001_rep_changes.pl | 2 +-
18 files changed, 188 insertions(+), 122 deletions(-)
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 3c51012df8..c22bb20eee 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -133,6 +133,14 @@ <title>Parameters</title>
Name of the replication slot to use. The default behavior is to use
<literal>subscription_name</> for slot name.
</para>
+
+ <para>
+ When <literal>SLOT NAME</literal> is set to
+ <literal>NONE</literal> there will be no slot associated with the
+ subscription. Such subscriptions must also have both
+ <literal>ENABLED</literal> and <literal>CREATE SLOT</literal> set
+ to <literal>false</literal>.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml
index f1ac125057..ea2cc874d9 100644
--- a/doc/src/sgml/ref/drop_subscription.sgml
+++ b/doc/src/sgml/ref/drop_subscription.sgml
@@ -21,7 +21,7 @@
<refsynopsisdiv>
<synopsis>
-DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ DROP SLOT | NODROP SLOT ]
+DROP SUBSCRIPTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
@@ -57,20 +57,13 @@ <title>Parameters</title>
</varlistentry>
<varlistentry>
- <term><literal>DROP SLOT</literal></term>
- <term><literal>NODROP SLOT</literal></term>
- <listitem>
- <para>
- Specifies whether to drop the replication slot on the publisher. The
- default is
- <literal>DROP SLOT</literal>.
- </para>
+ <term><literal>CASCADE</literal></term>
+ <term><literal>RESTRICT</literal></term>
+ <listitem>
<para>
- If the publisher is not reachable when the subscription is to be
- dropped, then it is useful to specify <literal>NODROP SLOT</literal>.
- But the replication slot on the publisher will then have to be removed
- manually.
+ These key words do not have any effect, since there are no dependencies
+ on subscriptions.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 22587a43b0..7dc21f1052 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -82,8 +82,10 @@ GetSubscription(Oid subid, bool missing_ok)
tup,
Anum_pg_subscription_subslotname,
&isnull);
- Assert(!isnull);
- sub->slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ if (!isnull)
+ sub->slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ else
+ sub->slotname = NULL;
/* Get synccommit */
datum = SysCacheGetAttr(SUBSCRIPTIONOID,
@@ -147,7 +149,8 @@ FreeSubscription(Subscription *sub)
{
pfree(sub->name);
pfree(sub->conninfo);
- pfree(sub->slotname);
+ if (sub->slotname)
+ pfree(sub->slotname);
list_free_deep(sub->publications);
pfree(sub);
}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index fde9e6e20c..1aa9eb689e 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -60,7 +60,8 @@ static List *fetch_table_list(WalReceiverConn *wrconn, List *publications);
*/
static void
parse_subscription_options(List *options, bool *connect, bool *enabled_given,
- bool *enabled, bool *create_slot, char **slot_name,
+ bool *enabled, bool *create_slot,
+ bool *slot_name_given, char **slot_name,
bool *copy_data, char **synchronous_commit)
{
ListCell *lc;
@@ -78,7 +79,10 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (create_slot)
*create_slot = true;
if (slot_name)
+ {
+ *slot_name_given = false;
*slot_name = NULL;
+ }
if (copy_data)
*copy_data = true;
if (synchronous_commit)
@@ -141,12 +145,17 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
}
else if (strcmp(defel->defname, "slot name") == 0 && slot_name)
{
- if (*slot_name)
+ if (*slot_name_given)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
+ *slot_name_given = true;
*slot_name = defGetString(defel);
+
+ /* Setting slot_name = NONE is treated as no slot name. */
+ if (strcmp(*slot_name, "none") == 0)
+ *slot_name = NULL;
}
else if (strcmp(defel->defname, "copy data") == 0 && copy_data)
{
@@ -194,17 +203,17 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (connect && !*connect)
{
/* Check for incompatible options from the user. */
- if (*enabled_given && *enabled)
+ if (enabled && *enabled_given && *enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("noconnect and enabled are mutually exclusive options")));
- if (create_slot_given && *create_slot)
+ if (create_slot && create_slot_given && *create_slot)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("noconnect and create slot are mutually exclusive options")));
- if (copy_data_given && *copy_data)
+ if (copy_data && copy_data_given && *copy_data)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("noconnect and copy data are mutually exclusive options")));
@@ -214,6 +223,23 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
*create_slot = false;
*copy_data = false;
}
+
+ /*
+ * Do additional checking for disallowed combination when
+ * slot_name = NONE was used.
+ */
+ if (slot_name && *slot_name_given && !*slot_name)
+ {
+ if (enabled && *enabled_given && *enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("slot_name = NONE and enabled are mutually exclusive options")));
+
+ if (create_slot && create_slot_given && *create_slot)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("slot_name = NONE and create slot are mutually exclusive options")));
+ }
}
/*
@@ -290,6 +316,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
char *synchronous_commit;
char *conninfo;
char *slotname;
+ bool slotname_given;
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
@@ -299,8 +326,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
* Connection and publication should not be specified here.
*/
parse_subscription_options(stmt->options, &connect, &enabled_given,
- &enabled, &create_slot, &slotname, ©_data,
- &synchronous_commit);
+ &enabled, &create_slot, &slotname_given,
+ &slotname, ©_data, &synchronous_commit);
/*
* Since creating a replication slot is not transactional, rolling back
@@ -329,8 +356,9 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
stmt->subname)));
}
- if (slotname == NULL)
+ if (!slotname_given && slotname == NULL)
slotname = stmt->subname;
+
/* The default for synchronous_commit of subscriptions is off. */
if (synchronous_commit == NULL)
synchronous_commit = "off";
@@ -355,8 +383,11 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
values[Anum_pg_subscription_subenabled - 1] = BoolGetDatum(enabled);
values[Anum_pg_subscription_subconninfo - 1] =
CStringGetTextDatum(conninfo);
- values[Anum_pg_subscription_subslotname - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ if (slotname)
+ values[Anum_pg_subscription_subslotname - 1] =
+ DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ else
+ nulls[Anum_pg_subscription_subslotname - 1] = true;
values[Anum_pg_subscription_subsynccommit - 1] =
CStringGetTextDatum(synchronous_commit);
values[Anum_pg_subscription_subpublications - 1] =
@@ -426,6 +457,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
*/
if (create_slot)
{
+ Assert(slotname);
+
walrcv_create_slot(wrconn, slotname, false,
CRS_NOEXPORT_SNAPSHOT, &lsn);
ereport(NOTICE,
@@ -578,6 +611,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
HeapTuple tup;
Oid subid;
bool update_tuple = false;
+ Subscription *sub;
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -597,6 +631,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
stmt->subname);
subid = HeapTupleGetOid(tup);
+ sub = GetSubscription(subid, false);
/* Form a new tuple. */
memset(values, 0, sizeof(values));
@@ -607,19 +642,29 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
{
case ALTER_SUBSCRIPTION_OPTIONS:
{
- char *slot_name;
- char *synchronous_commit;
+ char *slotname;
+ bool slotname_given;
+ char *synchronous_commit;
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, &slot_name, NULL,
- &synchronous_commit);
+ NULL, &slotname_given, &slotname,
+ NULL, &synchronous_commit);
- if (slot_name)
+ if (slotname_given)
{
- values[Anum_pg_subscription_subslotname - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(slot_name));
+ if (sub->enabled && !slotname)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot set slot_name = NONE for enabled subscription")));
+
+ if (slotname)
+ values[Anum_pg_subscription_subslotname - 1] =
+ DirectFunctionCall1(namein, CStringGetDatum(slotname));
+ else
+ nulls[Anum_pg_subscription_subslotname - 1] = true;
replaces[Anum_pg_subscription_subslotname - 1] = true;
}
+
if (synchronous_commit)
{
values[Anum_pg_subscription_subsynccommit - 1] =
@@ -638,9 +683,14 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
parse_subscription_options(stmt->options, NULL,
&enabled_given, &enabled, NULL,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
Assert(enabled_given);
+ if (!sub->slotname && enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot enable subscription which does not have a slot_name")));
+
values[Anum_pg_subscription_subenabled - 1] =
BoolGetDatum(enabled);
replaces[Anum_pg_subscription_subenabled - 1] = true;
@@ -668,10 +718,10 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
case ALTER_SUBSCRIPTION_PUBLICATION_REFRESH:
{
bool copy_data;
- Subscription *sub = GetSubscription(subid, false);
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, NULL, ©_data, NULL);
+ NULL, NULL, NULL, ©_data,
+ NULL);
values[Anum_pg_subscription_subpublications - 1] =
publicationListToArray(stmt->publication);
@@ -682,6 +732,11 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
/* Refresh if user asked us to. */
if (stmt->kind == ALTER_SUBSCRIPTION_PUBLICATION_REFRESH)
{
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
+
/* Make sure refresh sees the new list of publications. */
sub->publications = stmt->publication;
@@ -694,10 +749,15 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
case ALTER_SUBSCRIPTION_REFRESH:
{
bool copy_data;
- Subscription *sub = GetSubscription(subid, false);
+
+ if (!sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
parse_subscription_options(stmt->options, NULL, NULL, NULL,
- NULL, NULL, ©_data, NULL);
+ NULL, NULL, NULL, ©_data,
+ NULL);
AlterSubscription_refresh(sub, copy_data);
@@ -751,15 +811,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
StringInfoData cmd;
/*
- * Since dropping a replication slot is not transactional, the replication
- * slot stays dropped even if the transaction rolls back. So we cannot
- * run DROP SUBSCRIPTION inside a transaction block if dropping the
- * replication slot.
- */
- if (stmt->drop_slot)
- PreventTransactionChain(isTopLevel, "DROP SUBSCRIPTION ... DROP SLOT");
-
- /*
* Lock pg_subscription with AccessExclusiveLock to ensure
* that the launcher doesn't restart new worker during dropping
* the subscription
@@ -817,8 +868,20 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
/* Get slotname */
datum = SysCacheGetAttr(SUBSCRIPTIONOID, tup,
Anum_pg_subscription_subslotname, &isnull);
- Assert(!isnull);
- slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ if (!isnull)
+ slotname = pstrdup(NameStr(*DatumGetName(datum)));
+ else
+ slotname = NULL;
+
+ /*
+ * Since dropping a replication slot is not transactional, the replication
+ * slot stays dropped even if the transaction rolls back. So we cannot
+ * run DROP SUBSCRIPTION inside a transaction block if dropping the
+ * replication slot.
+ */
+ if (slotname)
+ PreventTransactionChain(isTopLevel, "DROP SUBSCRIPTION");
+
ObjectAddressSet(myself, SubscriptionRelationId, subid);
EventTriggerSQLDropAddObject(&myself, true, true);
@@ -843,8 +906,8 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
if (originid != InvalidRepOriginId)
replorigin_drop(originid);
- /* If the user asked to not drop the slot, we are done mow.*/
- if (!stmt->drop_slot)
+ /* If there is no slot associated with subscription we can finish here. */
+ if (!slotname)
{
heap_close(rel, NoLock);
return;
@@ -857,14 +920,16 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
load_file("libpqwalreceiver", false);
initStringInfo(&cmd);
- appendStringInfo(&cmd, "DROP_REPLICATION_SLOT \"%s\"", slotname);
+ appendStringInfo(&cmd, "DROP_REPLICATION_SLOT %s", quote_identifier(slotname));
wrconn = walrcv_connect(conninfo, true, subname, &err);
if (wrconn == NULL)
ereport(ERROR,
(errmsg("could not connect to publisher when attempting to "
"drop the replication slot \"%s\"", slotname),
- errdetail("The error was: %s", err)));
+ errdetail("The error was: %s", err),
+ errhint("Use ALTER SUBSCRIPTION ... WITH (slot_name = NONE) "
+ "to disassociate the subscription from the slot.")));
PG_TRY();
{
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 35a237a000..2d2a9d00b7 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4537,8 +4537,8 @@ _copyDropSubscriptionStmt(const DropSubscriptionStmt *from)
DropSubscriptionStmt *newnode = makeNode(DropSubscriptionStmt);
COPY_STRING_FIELD(subname);
- COPY_SCALAR_FIELD(drop_slot);
COPY_SCALAR_FIELD(missing_ok);
+ COPY_SCALAR_FIELD(behavior);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 21dfbb0d75..b5459cd726 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2246,8 +2246,8 @@ _equalDropSubscriptionStmt(const DropSubscriptionStmt *a,
const DropSubscriptionStmt *b)
{
COMPARE_STRING_FIELD(subname);
- COMPARE_SCALAR_FIELD(drop_slot);
COMPARE_SCALAR_FIELD(missing_ok);
+ COMPARE_SCALAR_FIELD(behavior);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2cad8b25b8..c3c10a27ec 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -415,7 +415,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <fun_param_mode> arg_class
%type <typnam> func_return func_type
-%type <boolean> opt_trusted opt_restart_seqs opt_drop_slot
+%type <boolean> opt_trusted opt_restart_seqs
%type <ival> OptTemp
%type <ival> OptNoLog
%type <oncommit> OnCommitOption
@@ -467,7 +467,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <node> def_arg columnElem where_clause where_or_current_clause
a_expr b_expr c_expr AexprConst indirection_el opt_slice_bound
columnref in_expr having_clause func_table xmltable array_expr
- ExclusionWhereClause
+ ExclusionWhereClause operator_def_arg
%type <list> rowsfrom_item rowsfrom_list opt_col_def_list
%type <boolean> opt_ordinality
%type <list> ExclusionConstraintList ExclusionConstraintElem
@@ -671,7 +671,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
SAVEPOINT SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES
SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW
- SIMILAR SIMPLE SKIP SLOT SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P
+ SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P
START STATEMENT STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P
SUBSCRIPTION SUBSTRING SYMMETRIC SYSID SYSTEM_P
@@ -5694,6 +5694,7 @@ def_arg: func_type { $$ = (Node *)$1; }
| qual_all_Op { $$ = (Node *)$1; }
| NumericOnly { $$ = (Node *)$1; }
| Sconst { $$ = (Node *)makeString($1); }
+ | NONE { $$ = (Node *)makeString(pstrdup($1)); }
;
old_aggr_definition: '(' old_aggr_list ')' { $$ = $2; }
@@ -8933,8 +8934,16 @@ operator_def_list: operator_def_elem { $$ = list_make1($1); }
operator_def_elem: ColLabel '=' NONE
{ $$ = makeDefElem($1, NULL, @1); }
- | ColLabel '=' def_arg
- { $$ = makeDefElem($1, (Node *) $3, @1); }
+ | ColLabel '='operator_def_arg
+ { $$ = makeDefElem($1, (Node *) $3, @1); }
+ ;
+
+operator_def_arg:
+ func_type { $$ = (Node *)$1; }
+ | reserved_keyword { $$ = (Node *)makeString(pstrdup($1)); }
+ | qual_all_Op { $$ = (Node *)$1; }
+ | NumericOnly { $$ = (Node *)$1; }
+ | Sconst { $$ = (Node *)makeString($1); }
;
/*****************************************************************************
@@ -9324,42 +9333,24 @@ AlterSubscriptionStmt:
*
*****************************************************************************/
-DropSubscriptionStmt: DROP SUBSCRIPTION name opt_drop_slot
+DropSubscriptionStmt: DROP SUBSCRIPTION name opt_drop_behavior
{
DropSubscriptionStmt *n = makeNode(DropSubscriptionStmt);
n->subname = $3;
- n->drop_slot = $4;
n->missing_ok = false;
+ n->behavior = $4;
$$ = (Node *) n;
}
- | DROP SUBSCRIPTION IF_P EXISTS name opt_drop_slot
+ | DROP SUBSCRIPTION IF_P EXISTS name opt_drop_behavior
{
DropSubscriptionStmt *n = makeNode(DropSubscriptionStmt);
n->subname = $5;
- n->drop_slot = $6;
n->missing_ok = true;
+ n->behavior = $6;
$$ = (Node *) n;
}
;
-opt_drop_slot:
- DROP SLOT
- {
- $$ = TRUE;
- }
- | IDENT SLOT
- {
- if (strcmp($1, "nodrop") == 0)
- $$ = FALSE;
- else
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("unrecognized option \"%s\"", $1),
- parser_errposition(@1)));
- }
- | /*EMPTY*/ { $$ = TRUE; }
- ;
-
/*****************************************************************************
*
* QUERY: Define Rewrite Rule
@@ -14846,7 +14837,6 @@ unreserved_keyword:
| SHOW
| SIMPLE
| SKIP
- | SLOT
| SNAPSHOT
| SQL_P
| STABLE
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index a61240ceee..362de12457 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1329,6 +1329,22 @@ reread_subscription(void)
}
/*
+ * Exit if the subscription was disabled.
+ * This normally should not happen as the worker gets killed
+ * during ALTER SUBSCRIPTION ... DISABLE.
+ */
+ if (!newsub->enabled)
+ {
+ ereport(LOG,
+ (errmsg("logical replication worker for subscription \"%s\" will "
+ "stop because the subscription was disabled",
+ MySubscription->name)));
+
+ walrcv_disconnect(wrconn);
+ proc_exit(0);
+ }
+
+ /*
* Exit if connection string was changed. The launcher will start
* new worker.
*/
@@ -1358,6 +1374,9 @@ reread_subscription(void)
proc_exit(0);
}
+ /* !slotname should never happen when enabled is true. */
+ Assert(newsub->slotname);
+
/*
* We need to make new connection to new slot if slot name has changed
* so exit here as well if that's the case.
@@ -1388,22 +1407,6 @@ reread_subscription(void)
proc_exit(0);
}
- /*
- * Exit if the subscription was disabled.
- * This normally should not happen as the worker gets killed
- * during ALTER SUBSCRIPTION ... DISABLE.
- */
- if (!newsub->enabled)
- {
- ereport(LOG,
- (errmsg("logical replication worker for subscription \"%s\" will "
- "stop because the subscription was disabled",
- MySubscription->name)));
-
- walrcv_disconnect(wrconn);
- proc_exit(0);
- }
-
/* Check for other changes that should never happen too. */
if (newsub->dbid != MySubscription->dbid)
{
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 5550f19926..d4f3979e7b 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -45,7 +45,7 @@ CATALOG(pg_subscription,6100) BKI_SHARED_RELATION BKI_ROWTYPE_OID(6101) BKI_SCHE
text subconninfo BKI_FORCE_NOT_NULL;
/* Slot name on publisher */
- NameData subslotname BKI_FORCE_NOT_NULL;
+ NameData subslotname;
/* Synchronous commit setting for worker */
text subsynccommit BKI_FORCE_NOT_NULL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e1d454a07d..46c23c2530 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3393,8 +3393,8 @@ typedef struct DropSubscriptionStmt
{
NodeTag type;
char *subname; /* Name of of the subscription */
- bool drop_slot; /* Should we drop the slot on remote side? */
bool missing_ok; /* Skip error if missing? */
+ DropBehavior behavior; /* RESTRICT or CASCADE behavior */
} DropSubscriptionStmt;
#endif /* PARSENODES_H */
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 37542aaee4..1ef03cfe52 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -366,7 +366,6 @@ PG_KEYWORD("show", SHOW, UNRESERVED_KEYWORD)
PG_KEYWORD("similar", SIMILAR, TYPE_FUNC_NAME_KEYWORD)
PG_KEYWORD("simple", SIMPLE, UNRESERVED_KEYWORD)
PG_KEYWORD("skip", SKIP, UNRESERVED_KEYWORD)
-PG_KEYWORD("slot", SLOT, UNRESERVED_KEYWORD)
PG_KEYWORD("smallint", SMALLINT, COL_NAME_KEYWORD)
PG_KEYWORD("snapshot", SNAPSHOT, UNRESERVED_KEYWORD)
PG_KEYWORD("some", SOME, RESERVED_KEYWORD)
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index 27c8ec5321..5f3768137c 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -69,7 +69,7 @@ CREATE SCHEMA dummy_seclabel_test;
SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT, SLOT NAME = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
@@ -111,7 +111,7 @@ NOTICE: event ddl_command_end: SECURITY LABEL
DROP EVENT TRIGGER always_start, always_end, always_drop, always_rewrite;
DROP VIEW dummy_seclabel_view1;
DROP TABLE dummy_seclabel_tbl1, dummy_seclabel_tbl2;
-DROP SUBSCRIPTION dummy_sub NODROP SLOT;
+DROP SUBSCRIPTION dummy_sub;
DROP PUBLICATION dummy_pub;
DROP ROLE regress_dummy_seclabel_user1;
DROP ROLE regress_dummy_seclabel_user2;
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
index 8d432441af..97311c7971 100644
--- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
+++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
@@ -73,7 +73,7 @@ CREATE SCHEMA dummy_seclabel_test;
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT, SLOT NAME = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
@@ -108,7 +108,7 @@ CREATE EVENT TRIGGER always_rewrite ON table_rewrite
DROP VIEW dummy_seclabel_view1;
DROP TABLE dummy_seclabel_tbl1, dummy_seclabel_tbl2;
-DROP SUBSCRIPTION dummy_sub NODROP SLOT;
+DROP SUBSCRIPTION dummy_sub;
DROP PUBLICATION dummy_pub;
DROP ROLE regress_dummy_seclabel_user1;
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 814e05e4ef..40eeeed3d2 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -37,7 +37,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCONNECT);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (NOCONNECT, SLOT NAME = NONE);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
@@ -477,7 +477,7 @@ SELECT (pg_identify_object(addr1.classid, addr1.objid, addr1.objsubid)).*,
SET client_min_messages TO 'warning';
DROP FOREIGN DATA WRAPPER addr_fdw CASCADE;
DROP PUBLICATION addr_pub;
-DROP SUBSCRIPTION addr_sub NODROP SLOT;
+DROP SUBSCRIPTION addr_sub;
DROP SCHEMA addr_nsp CASCADE;
DROP OWNED BY regress_addr_user;
DROP USER regress_addr_user;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index fd09f54548..56f826ba5c 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -113,17 +113,19 @@ HINT: The owner of a subscription must be a superuser.
ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
--- fail - cannot do DROP SUBSCRIPTION DROP SLOT inside transaction block
+-- fail - cannot do DROP SUBSCRIPTION inside transaction block with slot name
BEGIN;
-DROP SUBSCRIPTION testsub DROP SLOT;
-ERROR: DROP SUBSCRIPTION ... DROP SLOT cannot run inside a transaction block
+DROP SUBSCRIPTION testsub;
+ERROR: DROP SUBSCRIPTION cannot run inside a transaction block
COMMIT;
+ALTER SUBSCRIPTION testsub WITH (SLOT NAME = NONE);
+-- now it works
BEGIN;
-DROP SUBSCRIPTION testsub NODROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
-DROP SUBSCRIPTION IF EXISTS testsub NODROP SLOT;
+DROP SUBSCRIPTION IF EXISTS testsub;
NOTICE: subscription "testsub" does not exist, skipping
-DROP SUBSCRIPTION testsub NODROP SLOT; -- fail
+DROP SUBSCRIPTION testsub; -- fail
ERROR: subscription "testsub" does not exist
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index c9219e47c4..6940392c01 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -40,7 +40,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCONNECT);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (NOCONNECT, SLOT NAME = NONE);
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
@@ -205,7 +205,7 @@ CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
DROP FOREIGN DATA WRAPPER addr_fdw CASCADE;
DROP PUBLICATION addr_pub;
-DROP SUBSCRIPTION addr_sub NODROP SLOT;
+DROP SUBSCRIPTION addr_sub;
DROP SCHEMA addr_nsp CASCADE;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index db05f523a2..b9204460a4 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -83,17 +83,20 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WI
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
--- fail - cannot do DROP SUBSCRIPTION DROP SLOT inside transaction block
+-- fail - cannot do DROP SUBSCRIPTION inside transaction block with slot name
BEGIN;
-DROP SUBSCRIPTION testsub DROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
+ALTER SUBSCRIPTION testsub WITH (SLOT NAME = NONE);
+
+-- now it works
BEGIN;
-DROP SUBSCRIPTION testsub NODROP SLOT;
+DROP SUBSCRIPTION testsub;
COMMIT;
-DROP SUBSCRIPTION IF EXISTS testsub NODROP SLOT;
-DROP SUBSCRIPTION testsub NODROP SLOT; -- fail
+DROP SUBSCRIPTION IF EXISTS testsub;
+DROP SUBSCRIPTION testsub; -- fail
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index d1817f57da..8e79fa3c44 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -191,7 +191,7 @@
or die "Timed out while waiting for apply to restart";
# check all the cleanup
-$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_renamed DROP SLOT");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_renamed");
$result =
$node_subscriber->safe_psql('postgres', "SELECT count(*) FROM pg_subscription");
--
2.12.2
On Tue, May 9, 2017 at 2:07 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.Here is your patch amended for that.
I think we should change tab-completion support for that as well.
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 183fc37..046fdd5 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2684,7 +2684,7 @@ psql_completion(const char *text, int start, int end) /* DROP SUBSCRIPTION */
else if (Matches3("DROP", "SUBSCRIPTION", MatchAny))
- COMPLETE_WITH_LIST2("DROP SLOT", "NODROP SLOT");
+ COMPLETE_WITH_LIST2("CASCADE", "RESTRICT"); /* EXECUTE */
else if (Matches1("EXECUTE"))
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 09/05/17 07:07, Peter Eisentraut wrote:
On 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.Here is your patch amended for that.
I am fine with this mechanism as well.
--
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 Tue, May 9, 2017 at 5:39 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
On 09/05/17 07:07, Peter Eisentraut wrote:
On 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.
IIUC in this design, for example if we mistakenly create a
subscription without creating replication slot and corresponding
replication slot doesn't exist on publisher, we cannot drop
subscription until we create corresponding replication slot manually.
Isn't it become a problem for user?
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 09/05/17 10:51, Masahiko Sawada wrote:
On Tue, May 9, 2017 at 5:39 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:On 09/05/17 07:07, Peter Eisentraut wrote:
On 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.IIUC in this design, for example if we mistakenly create a
subscription without creating replication slot and corresponding
replication slot doesn't exist on publisher, we cannot drop
subscription until we create corresponding replication slot manually.
Isn't it become a problem for user?
We can, that's why the NONE value for slot name was added by the patch
so that subscription can be made "slot-less". The change of
RESTRICT/CASCADE behavior that Peter made is just about whether we
refuse to drop subscription by default when there is slot associated
with or if we just go straight to dropping the slot.
--
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 Tue, May 9, 2017 at 5:57 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
On 09/05/17 10:51, Masahiko Sawada wrote:
On Tue, May 9, 2017 at 5:39 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:On 09/05/17 07:07, Peter Eisentraut wrote:
On 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.IIUC in this design, for example if we mistakenly create a
subscription without creating replication slot and corresponding
replication slot doesn't exist on publisher, we cannot drop
subscription until we create corresponding replication slot manually.
Isn't it become a problem for user?We can, that's why the NONE value for slot name was added by the patch
so that subscription can be made "slot-less".
Yeah, but since we can still create subscription with only NOCREATE
SLOT option (option name will be changed but still exists), if we do
that then non-NULL value is stored into subslotname and the
subscription is enable. We can drop such subscription after disabled
it and after set its slot name to NONE. But I think it's still not
good for user..
The change of
RESTRICT/CASCADE behavior that Peter made is just about whether we
refuse to drop subscription by default when there is slot associated
with or if we just go straight to dropping the slot.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 09/05/17 11:44, Masahiko Sawada wrote:
On Tue, May 9, 2017 at 5:57 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:On 09/05/17 10:51, Masahiko Sawada wrote:
On Tue, May 9, 2017 at 5:39 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:On 09/05/17 07:07, Peter Eisentraut wrote:
On 5/8/17 23:23, Peter Eisentraut wrote:
The way this uses RESTRICT and CASCADE appears to be backwards from its
usual meaning. Normally, CASCADE when dropping an object that is still
used by others will cause those other objects to be dropped. The
equivalent here would be DROP REPLICATION SLOT + CASCADE would drop the
subscription.What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.IIUC in this design, for example if we mistakenly create a
subscription without creating replication slot and corresponding
replication slot doesn't exist on publisher, we cannot drop
subscription until we create corresponding replication slot manually.
Isn't it become a problem for user?We can, that's why the NONE value for slot name was added by the patch
so that subscription can be made "slot-less".Yeah, but since we can still create subscription with only NOCREATE
SLOT option (option name will be changed but still exists), if we do
that then non-NULL value is stored into subslotname and the
subscription is enable. We can drop such subscription after disabled
it and after set its slot name to NONE. But I think it's still not
good for user..
Well that's why I originally had the options directly as part of DROP
SUBSCRIPTION. But if you read the discussion in this thread, that's not
something we should be doing. There is no sensible way of mapping the
nodrop behavior to the only allowed options of RESTRICT and CASCADE so
there is not much we can do other than the ALTER.
--
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:39, Petr Jelinek wrote:
What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.Here is your patch amended for that.
I am fine with this mechanism as well.
Committed.
I also wrote a bit of documentation about slot handling for
subscriptions, covering some of what was discussed in this thread.
--
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 5/9/17 03:27, Masahiko Sawada wrote:
I think we should change tab-completion support for that as well.
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 183fc37..046fdd5 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2684,7 +2684,7 @@ psql_completion(const char *text, int start, int end)/* DROP SUBSCRIPTION */ else if (Matches3("DROP", "SUBSCRIPTION", MatchAny)) - COMPLETE_WITH_LIST2("DROP SLOT", "NODROP SLOT"); + COMPLETE_WITH_LIST2("CASCADE", "RESTRICT");/* EXECUTE */
else if (Matches1("EXECUTE"))
Thanks for pointing that out. I just moved it under the generic DROP
support section.
--
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 09/05/17 16:28, Peter Eisentraut wrote:
On 5/9/17 04:39, Petr Jelinek wrote:
What we want to simulate instead is an "auto" dependency of the slot on
the subscription. So you can drop the slot separately (subject to other
restrictions), and it is dropped automatically when the subscription is
dropped. To avoid that, you can disassociate the slot from the
subscription, which you have implemented.I think we can therefore do without RESTRICT/CASCADE here. If a slot is
associated with the subscription, it should be there when we drop the
subscription. Otherwise, the user has to disassociate the slot and take
care of it manually. So just keep the "cascade" behavior.Similarly, I wouldn't check first whether the slot exists. If the
subscription is associated with the slot, it should be there.Here is your patch amended for that.
I am fine with this mechanism as well.
Committed.
I also wrote a bit of documentation about slot handling for
subscriptions, covering some of what was discussed in this thread.
Great, thanks.
Here is rebased version of the other patch (the interface rework). I
also fixed the tab completion there.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
Rework-the-options-for-logical-replication-v2.patchbinary/octet-stream; name=Rework-the-options-for-logical-replication-v2.patchDownload
From 30dc7544919c97610e1c22ef711ccbfd1873b921 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Wed, 3 May 2017 11:39:22 +0200
Subject: [PATCH] Rework the options for logical replication
Use similar option style as other statements which use WITH clause for
options.
---
doc/src/sgml/ref/alter_publication.sgml | 25 +--
doc/src/sgml/ref/alter_subscription.sgml | 72 +++++--
doc/src/sgml/ref/create_publication.sgml | 57 ++---
doc/src/sgml/ref/create_subscription.sgml | 229 +++++++++++----------
src/backend/commands/publicationcmds.c | 133 +++++-------
src/backend/commands/subscriptioncmds.c | 42 +---
src/backend/parser/gram.y | 8 +-
src/bin/pg_dump/pg_dump.c | 36 ++--
src/bin/pg_dump/t/002_pg_dump.pl | 12 +-
src/bin/psql/tab-complete.c | 16 +-
src/include/parser/kwlist.h | 1 -
.../dummy_seclabel/expected/dummy_seclabel.out | 2 +-
.../modules/dummy_seclabel/sql/dummy_seclabel.sql | 2 +-
src/test/regress/expected/object_address.out | 2 +-
src/test/regress/expected/publication.out | 10 +-
src/test/regress/expected/subscription.out | 22 +-
src/test/regress/sql/object_address.sql | 2 +-
src/test/regress/sql/publication.sql | 10 +-
src/test/regress/sql/subscription.sql | 20 +-
src/test/subscription/t/001_rep_changes.pl | 8 +-
src/test/subscription/t/002_types.pl | 2 +-
src/test/subscription/t/003_constraints.pl | 2 +-
src/test/subscription/t/004_sync.pl | 2 +-
23 files changed, 328 insertions(+), 387 deletions(-)
diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml
index 05bd57d..00e5cfe 100644
--- a/doc/src/sgml/ref/alter_publication.sgml
+++ b/doc/src/sgml/ref/alter_publication.sgml
@@ -21,14 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">option</replaceable> [, ... ] )
-
-<phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
-
- PUBLISH INSERT | NOPUBLISH INSERT
- | PUBLISH UPDATE | NOPUBLISH UPDATE
- | PUBLISH DELETE | NOPUBLISH DELETE
-
+ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> ADD TABLE [ ONLY ] <replaceable class="PARAMETER">table_name</replaceable> [ * ] [, ...]
ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> SET TABLE [ ONLY ] <replaceable class="PARAMETER">table_name</replaceable> [ * ] [, ...]
ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> DROP TABLE [ ONLY ] <replaceable class="PARAMETER">table_name</replaceable> [ * ] [, ...]
@@ -44,8 +37,7 @@ ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> RENAME TO <r
The first variant of this command listed in the synopsis can change
all of the publication properties specified in
<xref linkend="sql-createpublication">. Properties not mentioned in the
- command retain their previous settings. Database superusers can change any
- of these settings for any role.
+ command retain their previous settings.
</para>
<para>
@@ -80,15 +72,10 @@ ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> RENAME TO <r
</varlistentry>
<varlistentry>
- <term><literal>PUBLISH INSERT</literal></term>
- <term><literal>NOPUBLISH INSERT</literal></term>
- <term><literal>PUBLISH UPDATE</literal></term>
- <term><literal>NOPUBLISH UPDATE</literal></term>
- <term><literal>PUBLISH DELETE</literal></term>
- <term><literal>NOPUBLISH DELETE</literal></term>
+ <term><literal>publish</literal></term>
<listitem>
<para>
- These clauses alter properties originally set by
+ This clause alters property originally set by
<xref linkend="SQL-CREATEPUBLICATION">. See there for more information.
</para>
</listitem>
@@ -131,9 +118,9 @@ ALTER PUBLICATION <replaceable class="PARAMETER">name</replaceable> RENAME TO <r
<title>Examples</title>
<para>
- Change the publication to not publish inserts:
+ Change the publication to publish only deletes and updates:
<programlisting>
-ALTER PUBLICATION noinsert WITH (NOPUBLISH INSERT);
+ALTER PUBLICATION noinsert OPTIONS (publish 'update, delete');
</programlisting>
</para>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 5dae4ae..57652d1 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -21,20 +21,9 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">suboption</replaceable> [, ... ] )
-
-<phrase>where <replaceable class="PARAMETER">suboption</replaceable> can be:</phrase>
-
- SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
- | SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable>
-
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] { REFRESH WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] ) | NOREFRESH }
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> REFRESH PUBLICATION [ WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] ) ]
-
-<phrase>where <replaceable class="PARAMETER">puboption</replaceable> can be:</phrase>
-
- COPY DATA | NOCOPY DATA
-
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] { REFRESH WITH ( <replaceable class="PARAMETER">refresh_option</replaceable> <replaceable class="PARAMETER">value</replaceable> [, ... ] ) | SKIP REFRESH }
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> REFRESH PUBLICATION [ WITH ( <replaceable class="PARAMETER">refresh_option</replaceable> <replaceable class="PARAMETER">value</replaceable> [, ... ] ) ]
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> CONNECTION '<replaceable>conninfo</replaceable>'
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> ENABLE
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> DISABLE
@@ -73,11 +62,9 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
<varlistentry>
<term><literal>CONNECTION '<replaceable class="parameter">conninfo</replaceable>'</literal></term>
- <term><literal>SLOT NAME = <replaceable class="parameter">slot_name</replaceable></literal></term>
- <term><literal>SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable></literal></term>
<listitem>
<para>
- These clauses alter properties originally set by
+ This clause alter property originally set by
<xref linkend="SQL-CREATESUBSCRIPTION">. See there for more
information.
</para>
@@ -85,6 +72,18 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
</varlistentry>
<varlistentry>
+ <term><literal>WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )</literal></term>
+ <listitem>
+ <para>
+ This clause alter property originally set by
+ <xref linkend="SQL-CREATESUBSCRIPTION">. See there for more
+ information. The allowed options are <literal>slot_name</literal> and
+ <literal>synchronous_commit</literal>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>SET PUBLICATION <replaceable class="parameter">publication_name</replaceable></literal></term>
<listitem>
<para>
@@ -97,7 +96,12 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
<literal>NOREFRESH</literal> is specified, the comamnd will not try to
refresh table information.
</para>
- </listitem>
+ <para>
+ The <literal>refresh_option</literal> is an additional option for the
+ refresh operation. See
+ <xref linkend="sql-altersubscription-refresh-options"> for details.
+ </para>
+ </listitem>
</varlistentry>
<varlistentry>
@@ -110,10 +114,9 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
since <command>CREATE SUBSCRIPTION</command>.
</para>
<para>
- The <literal>COPY DATA</literal> and <literal>NOCOPY DATA</literal>
- options specify if the existing data in the publications that are being
- subscribed to should be copied. <literal>COPY DATA</literal> is the
- default.
+ The <literal>refresh_option</literal> is an additional option for the
+ refresh operation. See
+ <xref linkend="sql-altersubscription-refresh-options"> for details.
</para>
</listitem>
</varlistentry>
@@ -156,6 +159,31 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
</listitem>
</varlistentry>
</variablelist>
+
+ <refsect2 id="sql-altersubscription-refresh-options">
+ <title id="sql-altersubscription-refresh-options-title">Refresh options</title>
+
+ <para>
+ The optional <literal>WITH</> clause for
+ <literal>SET PUBLICATION</literal> and
+ <literal>REFRESH PUBLICATION</literal> clauses specifies behavior of the
+ refresh operation. The supported options are:
+ </para>
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>copy_data</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies if the existing data in the publications that are being
+ subscribed to should be copied once the replication starts.
+ The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </refsect2>
+
</refsect1>
<refsect1>
diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml
index 521376e..76f4250 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -24,13 +24,8 @@ PostgreSQL documentation
CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
[ FOR TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [, ...]
| FOR ALL TABLES ]
- [ WITH ( <replaceable class="parameter">option</replaceable> [, ... ] ) ]
+ [ WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
-<phrase>where <replaceable class="parameter">option</replaceable> can be:</phrase>
-
- PUBLISH INSERT | NOPUBLISH INSERT
- | PUBLISH UPDATE | NOPUBLISH UPDATE
- | PUBLISH DELETE | NOPUBLISH DELETE
</synopsis>
</refsynopsisdiv>
@@ -97,37 +92,29 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
</varlistentry>
<varlistentry>
- <term><literal>PUBLISH INSERT</literal></term>
- <term><literal>NOPUBLISH INSERT</literal></term>
- <listitem>
- <para>
- These clauses determine whether the new publication will send
- the <command>INSERT</command> operations to the subscribers.
- <literal>PUBLISH INSERT</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>PUBLISH UPDATE</literal></term>
- <term><literal>NOPUBLISH UPDATE</literal></term>
+ <term><literal>WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )</literal></term>
<listitem>
<para>
- These clauses determine whether the new publication will send
- the <command>UPDATE</command> operations to the subscribers.
- <literal>PUBLISH UPDATE</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
+ This clause specifies optional parameters for a pubication; the
+ following parameters are supported:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>publish</literal> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ This clause determine which DML operations will be published by
+ the new publication to the subscribers. The value is comma
+ separated list of actions. The allowed operations are
+ <literal>insert</literal>, <literal>update</literal>, and
+ <literal>delete</literal>. The default is to publish all actions so
+ the default value for this option is
+ <literal>'insert,update,delete'</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
- <varlistentry>
- <term><literal>PUBLISH DELETE</literal></term>
- <term><literal>NOPUBLISH DELETE</literal></term>
- <listitem>
- <para>
- These clauses determine whether the new publication will send
- the <command>DELETE</command> operations to the subscribers.
- <literal>PUBLISH DELETE</literal> is the default.
</para>
</listitem>
</varlistentry>
@@ -203,7 +190,7 @@ CREATE PUBLICATION alltables FOR ALL TABLES;
operations in one table:
<programlisting>
CREATE PUBLICATION insert_only FOR TABLE mydata
- WITH (NOPUBLISH UPDATE, NOPUBLISH DELETE);
+ WITH (publish = 'insert');
</programlisting>
</para>
</refsect1>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 6382468..9fbc804 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -24,16 +24,7 @@ PostgreSQL documentation
CREATE SUBSCRIPTION <replaceable class="PARAMETER">subscription_name</replaceable>
CONNECTION '<replaceable class="PARAMETER">conninfo</replaceable>'
PUBLICATION { <replaceable class="PARAMETER">publication_name</replaceable> [, ...] }
- [ WITH ( <replaceable class="PARAMETER">option</replaceable> [, ... ] ) ]
-
-<phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
-
- | ENABLED | DISABLED
- | CREATE SLOT | NOCREATE SLOT
- | SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
- | COPY DATA | NOCOPY DATA
- | SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable>
- | NOCONNECT
+ [ WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
</synopsis>
</refsynopsisdiv>
@@ -103,110 +94,122 @@ CREATE SUBSCRIPTION <replaceable class="PARAMETER">subscription_name</replaceabl
</varlistentry>
<varlistentry>
- <term><literal>ENABLED</literal></term>
- <term><literal>DISABLED</literal></term>
- <listitem>
- <para>
- Specifies whether the subscription should be actively replicating or
- if it should be just setup but not started yet. Note that the
- replication slot as described above is created in either case.
- <literal>ENABLED</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>CREATE SLOT</literal></term>
- <term><literal>NOCREATE SLOT</literal></term>
- <listitem>
- <para>
- Specifies whether the command should create the replication slot on the
- publisher. <literal>CREATE SLOT</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>SLOT NAME = <replaceable class="parameter">slot_name</replaceable></literal></term>
- <listitem>
- <para>
- Name of the replication slot to use. The default behavior is to use
- <literal>subscription_name</> for slot name.
- </para>
-
- <para>
- When <literal>SLOT NAME</literal> is set to
- <literal>NONE</literal>, there will be no replication slot associated
- with the subscription. This can be used if the replication slot will be
- created later manually. Such subscriptions must also have both
- <literal>ENABLED</literal> and <literal>CREATE SLOT</literal> set
- to <literal>false</literal>.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>COPY DATA</literal></term>
- <term><literal>NOCOPY DATA</literal></term>
+ <term><literal>WITH ( <replaceable class="parameter">subscription_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )</literal></term>
<listitem>
<para>
- Specifies if the existing data in the publications that are being
- subscribed to should be copied once the replication starts.
- <literal>COPY DATA</literal> is the default.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable></literal></term>
- <listitem>
- <para>
- The value of this parameter overrides the
- <xref linkend="guc-synchronous-commit"> setting. The default value is
- <literal>off</literal>.
- </para>
-
- <para>
- It is safe to use <literal>off</literal> for logical replication: If the
- subscriber loses transactions because of missing synchronization, the
- data will be resent from the publisher.
- </para>
-
- <para>
- A different setting might be appropriate when doing synchronous logical
- replication. The logical replication workers report the positions of
- writes and flushes to the publisher, and when using synchronous
- replication, the publisher will wait for the actual flush. This means
- that setting <literal>SYNCHRONOUS_COMMIT</literal> for the subscriber
- to <literal>off</literal> when the subscription is used for synchronous
- replication might increase the latency for <command>COMMIT</command> on
- the publisher. In this scenario, it can be advantageous to set
- <literal>SYNCHRONOUS_COMMIT</literal> to <literal>local</literal> or
- higher.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
- <term><literal>NOCONNECT</literal></term>
- <listitem>
- <para>
- Instructs <command>CREATE SUBSCRIPTION</command> to skip the initial
- connection to the provider. This will change default values of other
- options to <literal>DISABLED</literal>,
- <literal>NOCREATE SLOT</literal>, and <literal>NOCOPY DATA</literal>.
- </para>
- <para>
- It's not allowed to combine <literal>NOCONNECT</literal> and
- <literal>ENABLED</literal>, <literal>CREATE SLOT</literal>, or
- <literal>COPY DATA</literal>.
- </para>
- <para>
- Since no connection is made when this option is specified, the tables
- are not subscribed, so after you enable the subscription nothing will
- be replicated. It is required to run
- <literal>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</> in order for
- tables to be subscribed.
+ This clause specifies optional parameters for a subscription; the
+ following parameters are supported:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>enabled</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies whether the subscription should be actively replicating
+ or if it should be just setup but not started yet. The default is
+ <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>create_slot</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies whether the command should create the replication slot
+ on the publisher. The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>slot_name</literal> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ Name of the replication slot to use. The default behavior is to
+ use <literal>subscription_name</> for slot name.
+ </para>
+
+ <para>
+ When <literal>slot_name</literal> is set to
+ <literal>NONE</literal>, there will be no replication slot
+ associated with the subscription. This can be used if the
+ replication slot will be created later manually. Such
+ subscriptions must also have both <literal>enabled</literal> and
+ <literal>create_slot</literal> set to <literal>false</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>copy_data</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies if the existing data in the publications that are being
+ subscribed to should be copied once the replication starts.
+ The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>synchronous_commit</literal> (<type>enum</type>)</term>
+ <listitem>
+ <para>
+ The value of this parameter overrides the
+ <xref linkend="guc-synchronous-commit"> setting. The default
+ value is <literal>off</literal>.
+ </para>
+
+ <para>
+ It is safe to use <literal>off</literal> for logical replication:
+ If the subscriber loses transactions because of missing
+ synchronization, the data will be resent from the publisher.
+ </para>
+
+ <para>
+ A different setting might be appropriate when doing synchronous
+ logical replication. The logical replication workers report the
+ positions of writes and flushes to the publisher, and when using
+ synchronous replication, the publisher will wait for the actual
+ flush. This means that setting
+ <literal>synchronous_commit</literal> for the subscriber to
+ <literal>off</literal> when the subscription is used for
+ synchronous replication might increase the latency for
+ <command>COMMIT</command> on the publisher. In this scenario, it
+ can be advantageous to set <literal>synchronous_commit</literal>
+ to <literal>local</literal> or higher.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>connect</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Boolean specifying if the <command>CREATE SUBSCRIPTION</command>
+ should connect to the provider. Setting this to
+ <literal>false</literal> will change default values of
+ <literal>enabled</literal>, <literal>create_slot</literal> and
+ <literal>copy_data</literal> to <literal>false</literal>.
+ </para>
+ <para>
+ It's not allowed to combine <literal>connect</literal> set to
+ <literal>false</literal> and <literal>enabled</literal>,
+ <literal>create_slot</literal>, or <literal>copy_data</literal>
+ to <literal>true</literal>.
+ </para>
+ <para>
+ Since no connection is made when this option is specified, the
+ tables are not subscribed, so after you enable the subscription
+ nothing will be replicated. It is required to run
+ <literal>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</> in order
+ for tables to be subscribed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
</para>
</listitem>
</varlistentry>
@@ -246,7 +249,7 @@ CREATE SUBSCRIPTION mysub
CREATE SUBSCRIPTION mysub
CONNECTION 'host=192.168.1.50 port=5432 user=foo dbname=foodb'
PUBLICATION insert_only
- WITH (DISABLED);
+ WITH (enabled = false);
</programlisting>
</para>
</refsect1>
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 541da7e..0ead996 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -46,6 +46,7 @@
#include "utils/lsyscache.h"
#include "utils/rel.h"
#include "utils/syscache.h"
+#include "utils/varlena.h"
/* Same as MAXNUMMESSAGES in sinvaladt.c */
#define MAX_RELCACHE_INVAL_MSGS 4096
@@ -58,18 +59,14 @@ static void PublicationDropTables(Oid pubid, List *rels, bool missing_ok);
static void
parse_publication_options(List *options,
- bool *publish_insert_given,
+ bool *publish_given,
bool *publish_insert,
- bool *publish_update_given,
bool *publish_update,
- bool *publish_delete_given,
bool *publish_delete)
{
ListCell *lc;
- *publish_insert_given = false;
- *publish_update_given = false;
- *publish_delete_given = false;
+ *publish_given = false;
/* Defaults are true */
*publish_insert = true;
@@ -81,65 +78,47 @@ parse_publication_options(List *options,
{
DefElem *defel = (DefElem *) lfirst(lc);
- if (strcmp(defel->defname, "publish insert") == 0)
+ if (strcmp(defel->defname, "publish") == 0)
{
- if (*publish_insert_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ char *publish;
+ List *publish_list;
+ ListCell *lc;
- *publish_insert_given = true;
- *publish_insert = defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "nopublish insert") == 0)
- {
- if (*publish_insert_given)
+ if (*publish_given)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
- *publish_insert_given = true;
- *publish_insert = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "publish update") == 0)
- {
- if (*publish_update_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ /*
+ * If publish option was given only the explicitly listed actions
+ * should be published.
+ */
+ *publish_insert = false;
+ *publish_update = false;
+ *publish_delete = false;
- *publish_update_given = true;
- *publish_update = defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "nopublish update") == 0)
- {
- if (*publish_update_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ *publish_given = true;
+ publish = defGetString(defel);
- *publish_update_given = true;
- *publish_update = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "publish delete") == 0)
- {
- if (*publish_delete_given)
+ if (!SplitIdentifierString(publish, ',', &publish_list))
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("invalid publish list")));
- *publish_delete_given = true;
- *publish_delete = defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "nopublish delete") == 0)
- {
- if (*publish_delete_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- *publish_delete_given = true;
- *publish_delete = !defGetBoolean(defel);
+ /* Process the option list. */
+ foreach (lc, publish_list)
+ {
+ char *publish_opt = (char *)lfirst(lc);
+
+ if (strcmp(publish_opt, "insert") == 0)
+ *publish_insert = true;
+ else if (strcmp(publish_opt, "update") == 0)
+ *publish_update = true;
+ else if (strcmp(publish_opt, "delete") == 0)
+ *publish_delete = true;
+ else
+ elog(ERROR, "unrecognized publish option: %s", publish_opt);
+ }
}
else
elog(ERROR, "unrecognized option: %s", defel->defname);
@@ -158,9 +137,7 @@ CreatePublication(CreatePublicationStmt *stmt)
bool nulls[Natts_pg_publication];
Datum values[Natts_pg_publication];
HeapTuple tup;
- bool publish_insert_given;
- bool publish_update_given;
- bool publish_delete_given;
+ bool publish_given;
bool publish_insert;
bool publish_update;
bool publish_delete;
@@ -199,9 +176,8 @@ CreatePublication(CreatePublicationStmt *stmt)
values[Anum_pg_publication_pubowner - 1] = ObjectIdGetDatum(GetUserId());
parse_publication_options(stmt->options,
- &publish_insert_given, &publish_insert,
- &publish_update_given, &publish_update,
- &publish_delete_given, &publish_delete);
+ &publish_given, &publish_insert,
+ &publish_update, &publish_delete);
values[Anum_pg_publication_puballtables - 1] =
BoolGetDatum(stmt->for_all_tables);
@@ -253,42 +229,31 @@ AlterPublicationOptions(AlterPublicationStmt *stmt, Relation rel,
bool nulls[Natts_pg_publication];
bool replaces[Natts_pg_publication];
Datum values[Natts_pg_publication];
- bool publish_insert_given;
- bool publish_update_given;
- bool publish_delete_given;
+ bool publish_given;
bool publish_insert;
bool publish_update;
bool publish_delete;
ObjectAddress obj;
parse_publication_options(stmt->options,
- &publish_insert_given, &publish_insert,
- &publish_update_given, &publish_update,
- &publish_delete_given, &publish_delete);
+ &publish_given, &publish_insert,
+ &publish_update, &publish_delete);
+
+ Assert(publish_given);
/* Everything ok, form a new tuple. */
memset(values, 0, sizeof(values));
memset(nulls, false, sizeof(nulls));
memset(replaces, false, sizeof(replaces));
- if (publish_insert_given)
- {
- values[Anum_pg_publication_pubinsert - 1] =
- BoolGetDatum(publish_insert);
- replaces[Anum_pg_publication_pubinsert - 1] = true;
- }
- if (publish_update_given)
- {
- values[Anum_pg_publication_pubupdate - 1] =
- BoolGetDatum(publish_update);
- replaces[Anum_pg_publication_pubupdate - 1] = true;
- }
- if (publish_delete_given)
- {
- values[Anum_pg_publication_pubdelete - 1] =
- BoolGetDatum(publish_delete);
- replaces[Anum_pg_publication_pubdelete - 1] = true;
- }
+ values[Anum_pg_publication_pubinsert - 1] = BoolGetDatum(publish_insert);
+ replaces[Anum_pg_publication_pubinsert - 1] = true;
+
+ values[Anum_pg_publication_pubupdate - 1] = BoolGetDatum(publish_update);
+ replaces[Anum_pg_publication_pubupdate - 1] = true;
+
+ values[Anum_pg_publication_pubdelete - 1] = BoolGetDatum(publish_delete);
+ replaces[Anum_pg_publication_pubdelete - 1] = true;
tup = heap_modify_tuple(tup, RelationGetDescr(rel), values, nulls,
replaces);
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b76cdc5..b3a41a2 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -93,7 +93,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
{
DefElem *defel = (DefElem *) lfirst(lc);
- if (strcmp(defel->defname, "noconnect") == 0 && connect)
+ if (strcmp(defel->defname, "connect") == 0 && connect)
{
if (connect_given)
ereport(ERROR,
@@ -101,7 +101,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
errmsg("conflicting or redundant options")));
connect_given = true;
- *connect = !defGetBoolean(defel);
+ *connect = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "enabled") == 0 && enabled)
{
@@ -113,17 +113,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
*enabled_given = true;
*enabled = defGetBoolean(defel);
}
- else if (strcmp(defel->defname, "disabled") == 0 && enabled)
- {
- if (*enabled_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- *enabled_given = true;
- *enabled = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "create slot") == 0 && create_slot)
+ else if (strcmp(defel->defname, "create_slot") == 0 && create_slot)
{
if (create_slot_given)
ereport(ERROR,
@@ -133,17 +123,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
create_slot_given = true;
*create_slot = defGetBoolean(defel);
}
- else if (strcmp(defel->defname, "nocreate slot") == 0 && create_slot)
- {
- if (create_slot_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- create_slot_given = true;
- *create_slot = !defGetBoolean(defel);
- }
- else if (strcmp(defel->defname, "slot name") == 0 && slot_name)
+ else if (strcmp(defel->defname, "slot_name") == 0 && slot_name)
{
if (*slot_name_given)
ereport(ERROR,
@@ -157,7 +137,7 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
if (strcmp(*slot_name, "none") == 0)
*slot_name = NULL;
}
- else if (strcmp(defel->defname, "copy data") == 0 && copy_data)
+ else if (strcmp(defel->defname, "copy_data") == 0 && copy_data)
{
if (copy_data_given)
ereport(ERROR,
@@ -167,16 +147,6 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
copy_data_given = true;
*copy_data = defGetBoolean(defel);
}
- else if (strcmp(defel->defname, "nocopy data") == 0 && copy_data)
- {
- if (copy_data_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
-
- copy_data_given = true;
- *copy_data = !defGetBoolean(defel);
- }
else if (strcmp(defel->defname, "synchronous_commit") == 0 &&
synchronous_commit)
{
@@ -336,7 +306,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
* replication slot.
*/
if (create_slot)
- PreventTransactionChain(isTopLevel, "CREATE SUBSCRIPTION ... CREATE SLOT");
+ PreventTransactionChain(isTopLevel, "CREATE SUBSCRIPTION ... (create_slot true)");
if (!superuser())
ereport(ERROR,
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 65c004c..68f5efd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -652,8 +652,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
MAPPING MATCH MATERIALIZED MAXVALUE METHOD MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NONE
- NOREFRESH NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
- NULLS_P NUMERIC
+ NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF NULLS_P NUMERIC
OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OPTIONS OR
ORDER ORDINALITY OUT_P OUTER_P OVER OVERLAPS OVERLAY OVERRIDING OWNED OWNER
@@ -5685,7 +5684,6 @@ def_elem: def_key '=' def_arg
def_key:
ColLabel { $$ = $1; }
- | ColLabel ColLabel { $$ = psprintf("%s %s", $1, $2); }
;
/* Note: any simple identifier will be returned as a type name! */
@@ -9173,6 +9171,7 @@ publication_for_tables:
}
;
+
/*****************************************************************************
*
* ALTER PUBLICATION name [ WITH ] options
@@ -9296,7 +9295,7 @@ AlterSubscriptionStmt:
n->options = $8;
$$ = (Node *)n;
}
- | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list NOREFRESH
+ | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
{
AlterSubscriptionStmt *n =
makeNode(AlterSubscriptionStmt);
@@ -14758,7 +14757,6 @@ unreserved_keyword:
| NEW
| NEXT
| NO
- | NOREFRESH
| NOTHING
| NOTIFY
| NOWAIT
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d724b11..8aa420f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3460,6 +3460,7 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
PQExpBuffer delq;
PQExpBuffer query;
PQExpBuffer labelq;
+ bool first = true;
if (!(pubinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
return;
@@ -3479,23 +3480,32 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
if (pubinfo->puballtables)
appendPQExpBufferStr(query, " FOR ALL TABLES");
- appendPQExpBufferStr(query, " WITH (");
+ appendPQExpBufferStr(query, " WITH (publish = '");
if (pubinfo->pubinsert)
- appendPQExpBufferStr(query, "PUBLISH INSERT");
- else
- appendPQExpBufferStr(query, "NOPUBLISH INSERT");
+ {
+ appendPQExpBufferStr(query, "insert");
+ first = false;
+ }
+
+ if (!first)
+ appendPQExpBufferChar(query, ',');
if (pubinfo->pubupdate)
- appendPQExpBufferStr(query, ", PUBLISH UPDATE");
- else
- appendPQExpBufferStr(query, ", NOPUBLISH UPDATE");
+ {
+ appendPQExpBufferStr(query, "update");
+ first = false;
+ }
+
+ if (!first)
+ appendPQExpBufferChar(query, ',');
if (pubinfo->pubdelete)
- appendPQExpBufferStr(query, ", PUBLISH DELETE");
- else
- appendPQExpBufferStr(query, ", NOPUBLISH DELETE");
+ {
+ appendPQExpBufferStr(query, "delete");
+ first = false;
+ }
- appendPQExpBufferStr(query, ");\n");
+ appendPQExpBufferStr(query, "');\n");
ArchiveEntry(fout, pubinfo->dobj.catId, pubinfo->dobj.dumpId,
pubinfo->dobj.name,
@@ -3817,11 +3827,11 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
appendPQExpBufferStr(publications, fmtId(pubnames[i]));
}
- appendPQExpBuffer(query, " PUBLICATION %s WITH (NOCONNECT, SLOT NAME = ", publications->data);
+ appendPQExpBuffer(query, " PUBLICATION %s WITH (connect = false, slot_name = ", publications->data);
appendStringLiteralAH(query, subinfo->subslotname, fout);
if (strcmp(subinfo->subsynccommit, "off") != 0)
- appendPQExpBuffer(query, ", SYNCHRONOUS_COMMIT = %s", fmtId(subinfo->subsynccommit));
+ appendPQExpBuffer(query, ", synchronous_commit = %s", fmtId(subinfo->subsynccommit));
appendPQExpBufferStr(query, ");\n");
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ce0c9ef..938c332 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4351,7 +4351,7 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
create_order => 50,
create_sql => 'CREATE PUBLICATION pub1;',
regexp => qr/^
- \QCREATE PUBLICATION pub1 WITH (PUBLISH INSERT, PUBLISH UPDATE, PUBLISH DELETE);\E
+ \QCREATE PUBLICATION pub1 WITH (publish = 'insert,update,delete');\E
/xm,
like => {
binary_upgrade => 1,
@@ -4384,11 +4384,9 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
create_order => 50,
create_sql => 'CREATE PUBLICATION pub2
FOR ALL TABLES
- WITH (NOPUBLISH INSERT,
- NOPUBLISH UPDATE,
- NOPUBLISH DELETE);',
+ WITH (publish = \'\');',
regexp => qr/^
- \QCREATE PUBLICATION pub2 FOR ALL TABLES WITH (NOPUBLISH INSERT, NOPUBLISH UPDATE, NOPUBLISH DELETE);\E
+ \QCREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish = '');\E
/xm,
like => {
binary_upgrade => 1,
@@ -4421,9 +4419,9 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
create_order => 50,
create_sql => 'CREATE SUBSCRIPTION sub1
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
- WITH (NOCONNECT);',
+ WITH (connect = false);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (NOCONNECT, SLOT NAME = 'sub1');\E
+ \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E
/xm,
like => {
binary_upgrade => 1,
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3bd5277..16c93fa 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1507,8 +1507,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER PUBLICATION <name> .. WITH ( ... */
else if (HeadMatches3("ALTER", "PUBLICATION",MatchAny) && TailMatches2("WITH", "("))
{
- COMPLETE_WITH_LIST6("PUBLISH INSERT", "NOPUBLISH INSERT", "PUBLISH UPDATE",
- "NOPUBLISH UPDATE", "PUBLISH DELETE", "NOPUBLISH DELETE");
+ COMPLETE_WITH_CONST("publish");
}
/* ALTER SUBSCRIPTION <name> ... */
else if (Matches3("ALTER","SUBSCRIPTION",MatchAny))
@@ -1520,12 +1519,12 @@ psql_completion(const char *text, int start, int end)
else if (HeadMatches3("ALTER", "SUBSCRIPTION", MatchAny) &&
TailMatches4("REFRESH", "PUBLICATION", "WITH", "("))
{
- COMPLETE_WITH_LIST2("COPY DATA", "NOCOPY DATA");
+ COMPLETE_WITH_CONST("copy_data");
}
/* ALTER SUBSCRIPTION <name> .. WITH ( ... */
else if (HeadMatches3("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches2("WITH", "("))
{
- COMPLETE_WITH_CONST("SLOT NAME");
+ COMPLETE_WITH_CONST("slot_name");
}
/* ALTER SCHEMA <name> */
else if (Matches3("ALTER", "SCHEMA", MatchAny))
@@ -2349,9 +2348,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
/* Complete "CREATE PUBLICATION <name> [...] WITH" */
else if (HeadMatches2("CREATE", "PUBLICATION") && TailMatches2("WITH", "("))
- COMPLETE_WITH_LIST2("PUBLISH", "NOPUBLISH");
- else if (HeadMatches2("CREATE", "PUBLICATION") && TailMatches3("WITH", "(", MatchAny))
- COMPLETE_WITH_LIST3("INSERT", "UPDATE", "DELETE");
+ COMPLETE_WITH_CONST("publish");
/* CREATE RULE */
/* Complete "CREATE RULE <sth>" with "AS ON" */
@@ -2427,9 +2424,8 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_CONST("WITH (");
/* Complete "CREATE SUBSCRIPTION <name> ... WITH ( <opt>" */
else if (HeadMatches2("CREATE", "SUBSCRIPTION") && TailMatches2("WITH", "("))
- COMPLETE_WITH_LIST8("ENABLED", "DISABLED", "CREATE SLOT",
- "NOCREATE SLOT", "SLOT NAME", "COPY DATA", "NOCOPY DATA",
- "NOCONNECT");
+ COMPLETE_WITH_LIST5("enabled", "create_slot", "slot_name",
+ "copy_data", "connect");
/* CREATE TRIGGER --- is allowed inside CREATE SCHEMA, so use TailMatches */
/* complete CREATE TRIGGER <name> with BEFORE,AFTER,INSTEAD OF */
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 1ef03cf..f50e45e 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -259,7 +259,6 @@ PG_KEYWORD("new", NEW, UNRESERVED_KEYWORD)
PG_KEYWORD("next", NEXT, UNRESERVED_KEYWORD)
PG_KEYWORD("no", NO, UNRESERVED_KEYWORD)
PG_KEYWORD("none", NONE, COL_NAME_KEYWORD)
-PG_KEYWORD("norefresh", NOREFRESH, UNRESERVED_KEYWORD)
PG_KEYWORD("not", NOT, RESERVED_KEYWORD)
PG_KEYWORD("nothing", NOTHING, UNRESERVED_KEYWORD)
PG_KEYWORD("notify", NOTIFY, UNRESERVED_KEYWORD)
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index 5f37681..77bdc93 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -69,7 +69,7 @@ CREATE SCHEMA dummy_seclabel_test;
SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT, SLOT NAME = NONE);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false, slot_name = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
diff --git a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
index 97311c7..8c347b6 100644
--- a/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
+++ b/src/test/modules/dummy_seclabel/sql/dummy_seclabel.sql
@@ -73,7 +73,7 @@ SECURITY LABEL ON SCHEMA dummy_seclabel_test IS 'unclassified'; -- OK
SET client_min_messages = error;
CREATE PUBLICATION dummy_pub;
-CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (NOCONNECT, SLOT NAME = NONE);
+CREATE SUBSCRIPTION dummy_sub CONNECTION '' PUBLICATION foo WITH (connect = false, slot_name = NONE);
RESET client_min_messages;
SECURITY LABEL ON PUBLICATION dummy_pub IS 'classified';
SECURITY LABEL ON SUBSCRIPTION dummy_sub IS 'classified';
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 40eeeed..700f261 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -37,7 +37,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (NOCONNECT, SLOT NAME = NONE);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index f3a348d..d9089af 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -13,8 +13,8 @@ SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
test publication
(1 row)
-CREATE PUBLICATION testpib_ins_trunct WITH (nopublish delete, nopublish update);
-ALTER PUBLICATION testpub_default WITH (nopublish insert, nopublish delete);
+CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
+ALTER PUBLICATION testpub_default WITH (publish = update);
\dRp
List of publications
Name | Owner | Inserts | Updates | Deletes
@@ -23,7 +23,7 @@ ALTER PUBLICATION testpub_default WITH (nopublish insert, nopublish delete);
testpub_default | regress_publication_user | f | t | f
(2 rows)
-ALTER PUBLICATION testpub_default WITH (publish insert, publish delete);
+ALTER PUBLICATION testpub_default WITH (publish = 'insert, update, delete');
\dRp
List of publications
Name | Owner | Inserts | Updates | Deletes
@@ -38,8 +38,8 @@ CREATE TABLE testpub_tbl1 (id serial primary key, data text);
CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
CREATE VIEW testpub_view AS SELECT 1;
CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
-CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (nopublish delete, nopublish update);
-ALTER PUBLICATION testpub_foralltables WITH (publish update);
+CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+ALTER PUBLICATION testpub_foralltables WITH (publish = 'insert,update');
CREATE TABLE testpub_tbl2 (id serial primary key, data text);
-- fail - can't add to for all tables publication
ALTER PUBLICATION testpub_foralltables ADD TABLE testpub_tbl2;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 56f826b..2caff71 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -17,18 +17,18 @@ LINE 1: CREATE SUBSCRIPTION testsub PUBLICATION foo;
^
-- fail - cannot do CREATE SUBSCRIPTION CREATE SLOT inside transaction block
BEGIN;
-CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (CREATE SLOT);
-ERROR: CREATE SUBSCRIPTION ... CREATE SLOT cannot run inside a transaction block
+CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (create_slot);
+ERROR: CREATE SUBSCRIPTION ... (create_slot true) cannot run inside a transaction block
COMMIT;
-- fail - invalid connection string
CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub;
ERROR: invalid connection string syntax: missing "=" after "testconn" in connection info string
-- fail - duplicate publications
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (connect = false);
ERROR: publication name "foo" used more than once
-- ok
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
COMMENT ON SUBSCRIPTION testsub IS 'test subscription';
SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
@@ -38,11 +38,11 @@ SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
(1 row)
-- fail - name already exists
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
ERROR: subscription "testsub" already exists
-- fail - must be superuser
SET SESSION AUTHORIZATION 'regress_subscription_user2';
-CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
ERROR: must be superuser to create subscriptions
SET SESSION AUTHORIZATION 'regress_subscription_user';
-- fail - invalid connection string
@@ -56,9 +56,9 @@ ERROR: invalid connection string syntax: missing "=" after "foobar" in connecti
testsub | regress_subscription_user | f | {testpub} | off | dbname=doesnotexist
(1 row)
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 NOREFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
-ALTER SUBSCRIPTION testsub WITH (SLOT NAME = 'newname');
+ALTER SUBSCRIPTION testsub WITH (slot_name = 'newname');
-- fail
ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
ERROR: subscription "doesnotexist" does not exist
@@ -93,8 +93,8 @@ ALTER SUBSCRIPTION testsub RENAME TO testsub_dummy;
ERROR: must be owner of subscription testsub
RESET ROLE;
ALTER SUBSCRIPTION testsub RENAME TO testsub_foo;
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = local);
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = foobar);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = local);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = foobar);
ERROR: invalid value for parameter "synchronous_commit": "foobar"
HINT: Available values: local, remote_write, remote_apply, on, off.
\dRs+
@@ -118,7 +118,7 @@ BEGIN;
DROP SUBSCRIPTION testsub;
ERROR: DROP SUBSCRIPTION cannot run inside a transaction block
COMMIT;
-ALTER SUBSCRIPTION testsub WITH (SLOT NAME = NONE);
+ALTER SUBSCRIPTION testsub WITH (slot_name = NONE);
-- now it works
BEGIN;
DROP SUBSCRIPTION testsub;
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index 6940392..8a738e2 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -40,7 +40,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
-CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (NOCONNECT, SLOT NAME = NONE);
+CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
CREATE STATISTICS addr_nsp.gentable_stat ON (a,b) FROM addr_nsp.gentable;
-- test some error cases
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 7d1cba5..ed6bd57 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -11,13 +11,13 @@ CREATE PUBLICATION testpub_default;
COMMENT ON PUBLICATION testpub_default IS 'test publication';
SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
-CREATE PUBLICATION testpib_ins_trunct WITH (nopublish delete, nopublish update);
+CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
-ALTER PUBLICATION testpub_default WITH (nopublish insert, nopublish delete);
+ALTER PUBLICATION testpub_default WITH (publish = update);
\dRp
-ALTER PUBLICATION testpub_default WITH (publish insert, publish delete);
+ALTER PUBLICATION testpub_default WITH (publish = 'insert, update, delete');
\dRp
@@ -28,8 +28,8 @@ CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
CREATE VIEW testpub_view AS SELECT 1;
CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
-CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (nopublish delete, nopublish update);
-ALTER PUBLICATION testpub_foralltables WITH (publish update);
+CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+ALTER PUBLICATION testpub_foralltables WITH (publish = 'insert,update');
CREATE TABLE testpub_tbl2 (id serial primary key, data text);
-- fail - can't add to for all tables publication
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index b920446..0e0f042 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -15,27 +15,27 @@ CREATE SUBSCRIPTION testsub PUBLICATION foo;
-- fail - cannot do CREATE SUBSCRIPTION CREATE SLOT inside transaction block
BEGIN;
-CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (CREATE SLOT);
+CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub WITH (create_slot);
COMMIT;
-- fail - invalid connection string
CREATE SUBSCRIPTION testsub CONNECTION 'testconn' PUBLICATION testpub;
-- fail - duplicate publications
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION foo, testpub, foo WITH (connect = false);
-- ok
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
COMMENT ON SUBSCRIPTION testsub IS 'test subscription';
SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
-- fail - name already exists
-CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
-- fail - must be superuser
SET SESSION AUTHORIZATION 'regress_subscription_user2';
-CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (NOCONNECT);
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
SET SESSION AUTHORIZATION 'regress_subscription_user';
-- fail - invalid connection string
@@ -43,9 +43,9 @@ ALTER SUBSCRIPTION testsub CONNECTION 'foobar';
\dRs+
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 NOREFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
-ALTER SUBSCRIPTION testsub WITH (SLOT NAME = 'newname');
+ALTER SUBSCRIPTION testsub WITH (slot_name = 'newname');
-- fail
ALTER SUBSCRIPTION doesnotexist CONNECTION 'dbname=doesnotexist2';
@@ -69,8 +69,8 @@ ALTER SUBSCRIPTION testsub RENAME TO testsub_dummy;
RESET ROLE;
ALTER SUBSCRIPTION testsub RENAME TO testsub_foo;
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = local);
-ALTER SUBSCRIPTION testsub_foo WITH (SYNCHRONOUS_COMMIT = foobar);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = local);
+ALTER SUBSCRIPTION testsub_foo WITH (synchronous_commit = foobar);
\dRs+
@@ -88,7 +88,7 @@ BEGIN;
DROP SUBSCRIPTION testsub;
COMMIT;
-ALTER SUBSCRIPTION testsub WITH (SLOT NAME = NONE);
+ALTER SUBSCRIPTION testsub WITH (slot_name = NONE);
-- now it works
BEGIN;
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index 8e79fa3..50c4f4d 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -40,7 +40,7 @@ my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION tap_pub");
$node_publisher->safe_psql('postgres',
- "CREATE PUBLICATION tap_pub_ins_only WITH (nopublish delete, nopublish update)");
+ "CREATE PUBLICATION tap_pub_ins_only WITH (publish = insert)");
$node_publisher->safe_psql('postgres',
"ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full");
$node_publisher->safe_psql('postgres',
@@ -136,7 +136,7 @@ $node_publisher->poll_query_until('postgres',
$oldpid = $node_publisher->safe_psql('postgres',
"SELECT pid FROM pg_stat_replication WHERE application_name = '$appname';");
$node_subscriber->safe_psql('postgres',
- "ALTER SUBSCRIPTION tap_sub SET PUBLICATION tap_pub_ins_only REFRESH WITH (NOCOPY DATA)");
+ "ALTER SUBSCRIPTION tap_sub SET PUBLICATION tap_pub_ins_only REFRESH WITH (copy_data = false)");
$node_publisher->poll_query_until('postgres',
"SELECT pid != $oldpid FROM pg_stat_replication WHERE application_name = '$appname';")
or die "Timed out while waiting for apply to restart";
@@ -159,13 +159,13 @@ is($result, qq(20|-20|-1), 'check changes skipped after subscription publication
# check alter publication (relcache invalidation etc)
$node_publisher->safe_psql('postgres',
- "ALTER PUBLICATION tap_pub_ins_only WITH (publish delete)");
+ "ALTER PUBLICATION tap_pub_ins_only WITH (publish = 'insert,delete')");
$node_publisher->safe_psql('postgres',
"ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_full");
$node_publisher->safe_psql('postgres',
"DELETE FROM tab_ins WHERE a > 0");
$node_subscriber->safe_psql('postgres',
- "ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (NOCOPY DATA)");
+ "ALTER SUBSCRIPTION tap_sub REFRESH PUBLICATION WITH (copy_data = false)");
$node_publisher->safe_psql('postgres',
"INSERT INTO tab_full VALUES(0)");
diff --git a/src/test/subscription/t/002_types.pl b/src/test/subscription/t/002_types.pl
index ad15e85..e9368ab 100644
--- a/src/test/subscription/t/002_types.pl
+++ b/src/test/subscription/t/002_types.pl
@@ -103,7 +103,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (SLOT NAME = tap_sub_slot)");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (slot_name = tap_sub_slot)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/003_constraints.pl b/src/test/subscription/t/003_constraints.pl
index 11b8254..e906ea1 100644
--- a/src/test/subscription/t/003_constraints.pl
+++ b/src/test/subscription/t/003_constraints.pl
@@ -34,7 +34,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA)");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (copy_data = false)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index fa0bf7f..8ece7dd 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -82,7 +82,7 @@ is($result, qq(20), 'initial data synced for second sub');
# now check another subscription for the same node pair
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub2 CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA)");
+ "CREATE SUBSCRIPTION tap_sub2 CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (copy_data = false)");
# wait for it to start
$node_subscriber->poll_query_until('postgres', "SELECT pid IS NOT NULL FROM pg_stat_subscription WHERE subname = 'tap_sub2' AND relid IS NULL")
--
2.7.4
On Mon, May 08, 2017 at 05:01:00PM -0400, Peter Eisentraut wrote:
On 5/7/17 04:21, Noah Misch wrote:
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 think we have a workable patch, but it needs some rebasing. I hope to
have this sorted by Wednesday.
This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. 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 11/05/17 09:27, Noah Misch wrote:
On Mon, May 08, 2017 at 05:01:00PM -0400, Peter Eisentraut wrote:
On 5/7/17 04:21, Noah Misch wrote:
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 think we have a workable patch, but it needs some rebasing. I hope to
have this sorted by Wednesday.This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
The patch Peter mentioned was committed.
There is however another open item associated with this thread for which
there is another patch as well.
--
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 11:43, Petr Jelinek wrote:
Here is rebased version of the other patch (the interface rework). I
also fixed the tab completion there.
Committed.
One small thing I changed, to make it look more like CREATE/ALTER TABLE,
is that the CREATE commands use WITH (...) but the ALTER commands use
SET (...).
--
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 12/05/17 15:02, Peter Eisentraut wrote:
On 5/9/17 11:43, Petr Jelinek wrote:
Here is rebased version of the other patch (the interface rework). I
also fixed the tab completion there.Committed.
One small thing I changed, to make it look more like CREATE/ALTER TABLE,
is that the CREATE commands use WITH (...) but the ALTER commands use
SET (...).
Makes sense, thanks!
--
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