pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Petr Jelinek <petr.jelinek@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
Date: Tue, 9 May 2017 01:07:53 -0400
Message-ID: <35f3592b-8582-e348-1823-f8edfcf03ba3@2ndquadrant.com> (raw )
In-Reply-To: <52a1f160-db1f-49c6-9832-498dbea58ae8@2ndquadrant.com >
References: <CA+TgmoZmkbpAWRzVKDVcHnTBkYjJEFS8=09RL-G3zgdozCLFHQ@mail.gmail.com >
<27478.1493727233@sss.pgh.pa.us >
<29431.1493730652@sss.pgh.pa.us >
<360a1932-7af4-609b-94f1-2ad419430869@2ndquadrant.com >
<9174dc31-787a-a2f6-818f-cb4a5c1dfddb@2ndquadrant.com >
<30166.1493731904@sss.pgh.pa.us >
<daf9909a-7033-9cfe-a8cc-a606025820fc@2ndquadrant.com >
<2b931e0e-d9f2-c79e-b74b-91df2f915e66@2ndquadrant.com >
<96e95255-94d9-0881-16ba-bdb6d05bb307@2ndquadrant.com >
<52a1f160-db1f-49c6-9832-498dbea58ae8@2ndquadrant.com >
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
v2-0001-Remove-the-NODROP-SLOT-option-from-DROP-SUBSCRIPT.patch (invalid/octet-stream, 31.0K) download
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
view thread (47+ messages) latest in thread
Message-ID: <35f3592b-8582-e348-1823-f8edfcf03ba3@2ndquadrant.com>
Permalink: ../35f3592b-8582-e348-1823-f8edfcf03ba3@2ndquadrant.com/
Also on: postgresql.org/message-id/35f3592b-8582-e348-1823-f8edfcf03ba3@2ndquadrant.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com, petr.jelinek@2ndquadrant.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, michael.paquier@gmail.com
Subject: Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
In-Reply-To: <35f3592b-8582-e348-1823-f8edfcf03ba3@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox