ALTER SUBSCRIPTION ..SET PUBLICATION <no name> refresh is not throwing error.
Hi,
ALTER SUBSCRIPTION ..SET PUBLICATION <no name> refresh is removing all
the attached subscription('s).
X Machine -
s=# create table t(n int);
CREATE TABLE
s=# create table t1(n int);
CREATE TABLE
s=# create publication pub for table t,t1;
CREATE PUBLICATION
s=#
Y Machine -
s=# create table t(n int);
CREATE TABLE
s=# create table t1(n int);
CREATE TABLE
s=# create subscription s1 connection 'dbname=s port=5432 user=centos
host=localhost' publication pub;
NOTICE: synchronized table states
NOTICE: created replication slot "s1" on publisher
CREATE SUBSCRIPTION
s=# alter subscription s1 set publication skip refresh ;
NOTICE: removed subscription for table public.t
NOTICE: removed subscription for table public.t1
ALTER SUBSCRIPTION
s=#
I think - this is probably due to not given publication NAME in the sql
query .
we are doing a syntax check at the time of REFRESH but not with SKIP
REFRESH
s=# alter subscription s1 set publication refresh ;
ERROR: syntax error at or near ";"
LINE 1: alter subscription s1 set publication refresh ;
^
s=# alter subscription s1 set publication skip refresh ;
ALTER SUBSCRIPTION
s=#
--
regards,tushar
EnterpriseDB https://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
2017-05-23 6:00 GMT-03:00 tushar <tushar.ahuja@enterprisedb.com>:
s=# alter subscription s1 set publication skip refresh ;
NOTICE: removed subscription for table public.t
NOTICE: removed subscription for table public.t1
ALTER SUBSCRIPTION
s=#
That's a design flaw. Since SKIP is not a reserved word, parser consider it
as a publication name. Instead of causing an error, it executes another
command (REFRESH) that is the opposite someone expects. Also, as "skip" is
not a publication name, it removes all tables in the subscription.
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list REFRESH
opt_definition
I think the first command was a bad design. Why don't we transform SKIP
REFRESH into a REFRESH option?
ALTER SUBSCRIPTION sub1 SET PUBLICATION pub1 REFRESH WITH (skip = true);
skip (boolean): specifies that the command will not try to refresh table
information. The default is false.
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
On 24/05/17 21:07, Euler Taveira wrote:
2017-05-23 6:00 GMT-03:00 tushar <tushar.ahuja@enterprisedb.com
<mailto:tushar.ahuja@enterprisedb.com>>:s=# alter subscription s1 set publication skip refresh ;
NOTICE: removed subscription for table public.t
NOTICE: removed subscription for table public.t1
ALTER SUBSCRIPTION
s=#That's a design flaw. Since SKIP is not a reserved word, parser consider
it as a publication name. Instead of causing an error, it executes
another command (REFRESH) that is the opposite someone expects. Also, as
"skip" is not a publication name, it removes all tables in the subscription.
Ah that explains why I originally added the ugly NOREFRESH keyword.
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list REFRESH
opt_definitionI think the first command was a bad design. Why don't we transform SKIP
REFRESH into a REFRESH option?ALTER SUBSCRIPTION sub1 SET PUBLICATION pub1 REFRESH WITH (skip = true);
skip (boolean): specifies that the command will not try to refresh table
information. The default is false.
That's quite confusing IMHO, saying REFRESH but then adding option to
actually not refresh is not a good interface.
I wonder if we actually need the SKIP REFRESH syntax, there is the
"REFRESH [ WITH ... ]" when user wants to refresh, so if REFRESH is not
specified, we can just behave as if SKIP REFRESH was used, it's not like
there is 3rd possible behavior.
--
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 24/05/17 21:21, Petr Jelinek wrote:
On 24/05/17 21:07, Euler Taveira wrote:
2017-05-23 6:00 GMT-03:00 tushar <tushar.ahuja@enterprisedb.com
<mailto:tushar.ahuja@enterprisedb.com>>:s=# alter subscription s1 set publication skip refresh ;
NOTICE: removed subscription for table public.t
NOTICE: removed subscription for table public.t1
ALTER SUBSCRIPTION
s=#That's a design flaw. Since SKIP is not a reserved word, parser consider
it as a publication name. Instead of causing an error, it executes
another command (REFRESH) that is the opposite someone expects. Also, as
"skip" is not a publication name, it removes all tables in the subscription.Ah that explains why I originally added the ugly NOREFRESH keyword.
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list REFRESH
opt_definitionI think the first command was a bad design. Why don't we transform SKIP
REFRESH into a REFRESH option?ALTER SUBSCRIPTION sub1 SET PUBLICATION pub1 REFRESH WITH (skip = true);
skip (boolean): specifies that the command will not try to refresh table
information. The default is false.That's quite confusing IMHO, saying REFRESH but then adding option to
actually not refresh is not a good interface.I wonder if we actually need the SKIP REFRESH syntax, there is the
"REFRESH [ WITH ... ]" when user wants to refresh, so if REFRESH is not
specified, we can just behave as if SKIP REFRESH was used, it's not like
there is 3rd possible behavior.
Attached patch does exactly that.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Remove-the-SKIP-REFRESH-syntax-suggar-in-ALTER-SUBSC.patchbinary/octet-stream; name=0001-Remove-the-SKIP-REFRESH-syntax-suggar-in-ALTER-SUBSC.patchDownload
From 27a06589a6a6fb399f77b30cb5584d9b6e0ac110 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Wed, 24 May 2017 21:27:56 +0200
Subject: [PATCH] Remove the SKIP REFRESH syntax suggar in ALTER SUBSCRIPTION
---
src/backend/parser/gram.y | 2 +-
src/test/regress/expected/subscription.out | 2 +-
src/test/regress/sql/subscription.sql | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2822331..62f6b52 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9289,7 +9289,7 @@ AlterSubscriptionStmt:
n->options = $8;
$$ = (Node *)n;
}
- | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
+ | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list
{
AlterSubscriptionStmt *n =
makeNode(AlterSubscriptionStmt);
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 91ba8ab..0e8d809 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -82,7 +82,7 @@ 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 SKIP REFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
-- fail
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 4b694a3..04aa063 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -61,7 +61,7 @@ ALTER SUBSCRIPTION testsub CONNECTION 'foobar';
\dRs+
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
--
2.7.4
On 24/05/17 21:28, Petr Jelinek wrote:
On 24/05/17 21:21, Petr Jelinek wrote:
On 24/05/17 21:07, Euler Taveira wrote:
2017-05-23 6:00 GMT-03:00 tushar <tushar.ahuja@enterprisedb.com
<mailto:tushar.ahuja@enterprisedb.com>>:s=# alter subscription s1 set publication skip refresh ;
NOTICE: removed subscription for table public.t
NOTICE: removed subscription for table public.t1
ALTER SUBSCRIPTION
s=#That's a design flaw. Since SKIP is not a reserved word, parser consider
it as a publication name. Instead of causing an error, it executes
another command (REFRESH) that is the opposite someone expects. Also, as
"skip" is not a publication name, it removes all tables in the subscription.Ah that explains why I originally added the ugly NOREFRESH keyword.
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list REFRESH
opt_definitionI think the first command was a bad design. Why don't we transform SKIP
REFRESH into a REFRESH option?ALTER SUBSCRIPTION sub1 SET PUBLICATION pub1 REFRESH WITH (skip = true);
skip (boolean): specifies that the command will not try to refresh table
information. The default is false.That's quite confusing IMHO, saying REFRESH but then adding option to
actually not refresh is not a good interface.I wonder if we actually need the SKIP REFRESH syntax, there is the
"REFRESH [ WITH ... ]" when user wants to refresh, so if REFRESH is not
specified, we can just behave as if SKIP REFRESH was used, it's not like
there is 3rd possible behavior.Attached patch does exactly that.
And of course I forgot to update docs...
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
Remove-the-SKIP-REFRESH-syntax-suggar-in-ALTER-SUBSC-v2.patchbinary/octet-stream; name=Remove-the-SKIP-REFRESH-syntax-suggar-in-ALTER-SUBSC-v2.patchDownload
From 08f6419050c089e716f2f900c8dc3215a3b121ca Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Wed, 24 May 2017 21:27:56 +0200
Subject: [PATCH] Remove the SKIP REFRESH syntax suggar in ALTER SUBSCRIPTION
---
doc/src/sgml/ref/alter_subscription.sgml | 9 +++------
src/backend/parser/gram.y | 2 +-
src/test/regress/expected/subscription.out | 2 +-
src/test/regress/sql/subscription.sql | 2 +-
4 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 113e32b..5224052 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> CONNECTION '<replaceable>conninfo</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> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] [ REFRESH [ WITH ( <replaceable class="PARAMETER">refresh_option</replaceable> [= <replaceable class="PARAMETER">value</replaceable>] [, ... ] ) ] ]
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> ENABLE
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> DISABLE
@@ -84,11 +84,8 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> RENAME TO <
like <literal>REFRESH
PUBLICATION</literal>. <literal>refresh_option</literal> specifies
additional options for the refresh operation, as described
- under <literal>REFRESH PUBLICATION</literal>. When
- <literal>SKIP REFRESH</literal> is specified, the command will not try
- to refresh table information. Note that
- either <literal>REFRESH</literal> or <literal>SKIP REFRESH</literal>
- must be specified.
+ under <literal>REFRESH PUBLICATION</literal>. Otherwise, the table
+ information will not be updated.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2822331..62f6b52 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9289,7 +9289,7 @@ AlterSubscriptionStmt:
n->options = $8;
$$ = (Node *)n;
}
- | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
+ | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list
{
AlterSubscriptionStmt *n =
makeNode(AlterSubscriptionStmt);
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 91ba8ab..0e8d809 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -82,7 +82,7 @@ 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 SKIP REFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
-- fail
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 4b694a3..04aa063 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -61,7 +61,7 @@ ALTER SUBSCRIPTION testsub CONNECTION 'foobar';
\dRs+
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3;
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
--
2.7.4
On 5/24/17 15:38, Petr Jelinek wrote:
I wonder if we actually need the SKIP REFRESH syntax, there is the
"REFRESH [ WITH ... ]" when user wants to refresh, so if REFRESH is not
specified, we can just behave as if SKIP REFRESH was used, it's not like
there is 3rd possible behavior.Attached patch does exactly that.
And of course I forgot to update docs...
Do we want not-refreshing to be the default behavior?
--
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
2017-05-26 17:58 GMT-03:00 Peter Eisentraut <
peter.eisentraut@2ndquadrant.com>:
On 5/24/17 15:38, Petr Jelinek wrote:
I wonder if we actually need the SKIP REFRESH syntax, there is the
"REFRESH [ WITH ... ]" when user wants to refresh, so if REFRESH is not
specified, we can just behave as if SKIP REFRESH was used, it's notlike
there is 3rd possible behavior.
Attached patch does exactly that.
And of course I forgot to update docs...
Do we want not-refreshing to be the default behavior?
It is a different behavior from the initial proposal. However, we
fortunately have ALTER SUBSCRIPTION foo REFRESH PUBLICATION and can refresh
later. Also, if "refresh" is more popular than "skip", it is just a small
word in the command. That's the price we pay to avoid ambiguity that the
previous syntax had.At least I think Petr's proposal is less confusing than
mine (my proposal maintains current behavior but can cause some confusion).
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
On 27/05/17 02:13, Euler Taveira wrote:
2017-05-26 17:58 GMT-03:00 Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>>:On 5/24/17 15:38, Petr Jelinek wrote:
I wonder if we actually need the SKIP REFRESH syntax, there is the
"REFRESH [ WITH ... ]" when user wants to refresh, so if REFRESH is not
specified, we can just behave as if SKIP REFRESH was used, it's not like
there is 3rd possible behavior.Attached patch does exactly that.
And of course I forgot to update docs...
Do we want not-refreshing to be the default behavior?
It is a different behavior from the initial proposal. However, we
fortunately have ALTER SUBSCRIPTION foo REFRESH PUBLICATION and can
refresh later. Also, if "refresh" is more popular than "skip", it is
just a small word in the command. That's the price we pay to avoid
ambiguity that the previous syntax had.At least I think Petr's proposal
is less confusing than mine (my proposal maintains current behavior but
can cause some confusion).
Actually another possibility would be to remove the REFRESH keyword
completely and just have [ WITH (...) ] and have the refresh option
there, ie simplified version of what you have suggested (without the
ugliness of specifying refresh twice to disable).
--
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
2017-05-26 21:29 GMT-03:00 Petr Jelinek <petr.jelinek@2ndquadrant.com>:
Actually another possibility would be to remove the REFRESH keyword
completely and just have [ WITH (...) ] and have the refresh option
there, ie simplified version of what you have suggested (without the
ugliness of specifying refresh twice to disable).
It will cause confusion. It seems that WITH sets ALTER SUBSCRIPTION
properties. Indeed, they are REFRESH properties. I think we shouldn't
exclude REFRESH keyword. Syntax leaves no doubt that WITH are REFRESH
properties.
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
On 27/05/17 04:00, Euler Taveira wrote:
2017-05-26 21:29 GMT-03:00 Petr Jelinek <petr.jelinek@2ndquadrant.com
<mailto:petr.jelinek@2ndquadrant.com>>:Actually another possibility would be to remove the REFRESH keyword
completely and just have [ WITH (...) ] and have the refresh option
there, ie simplified version of what you have suggested (without the
ugliness of specifying refresh twice to disable).It will cause confusion. It seems that WITH sets ALTER SUBSCRIPTION
properties. Indeed, they are REFRESH properties. I think we shouldn't
exclude REFRESH keyword. Syntax leaves no doubt that WITH are REFRESH
properties.
Maybe, I don't know, it might not be that confusing when SET PUBLICATION
and REFRESH PUBLICATION have same set of WITH options.
--
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/27/17 06:54, Petr Jelinek wrote:
On 27/05/17 04:00, Euler Taveira wrote:
2017-05-26 21:29 GMT-03:00 Petr Jelinek <petr.jelinek@2ndquadrant.com
<mailto:petr.jelinek@2ndquadrant.com>>:Actually another possibility would be to remove the REFRESH keyword
completely and just have [ WITH (...) ] and have the refresh option
there, ie simplified version of what you have suggested (without the
ugliness of specifying refresh twice to disable).It will cause confusion. It seems that WITH sets ALTER SUBSCRIPTION
properties. Indeed, they are REFRESH properties. I think we shouldn't
exclude REFRESH keyword. Syntax leaves no doubt that WITH are REFRESH
properties.Maybe, I don't know, it might not be that confusing when SET PUBLICATION
and REFRESH PUBLICATION have same set of WITH options.
I'm not sure what the conclusion from the above discussion was supposed
to be, but here is a patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Fix-ALTER-SUBSCRIPTION-grammar-ambiguity.patchtext/plain; charset=UTF-8; name=0001-Fix-ALTER-SUBSCRIPTION-grammar-ambiguity.patch; x-mac-creator=0; x-mac-type=0Download
From 697c4cbdd386a4bd856614de6cedf5af8d1b63be Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Fri, 2 Jun 2017 21:59:07 -0400
Subject: [PATCH] Fix ALTER SUBSCRIPTION grammar ambiguity
There was a grammar ambiguity between SET PUBLICATION name REFRESH and
SET PUBLICATION SKIP REFRESH, because SKIP is not a reserved word. To
resolve that, fold the refresh choice into the WITH options. Refreshing
is the default now.
Author: tushar <tushar.ahuja@enterprisedb.com>
---
doc/src/sgml/catalogs.sgml | 2 +-
doc/src/sgml/ref/alter_subscription.sgml | 35 ++++++++++++++++++++----------
src/backend/commands/subscriptioncmds.c | 34 +++++++++++++++++++++--------
src/backend/parser/gram.y | 14 ++----------
src/include/nodes/parsenodes.h | 1 -
src/test/regress/expected/subscription.out | 2 +-
src/test/regress/sql/subscription.sql | 2 +-
src/test/subscription/t/001_rep_changes.pl | 2 +-
8 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index b2fae027f5..5723be744d 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6609,7 +6609,7 @@ <title><structname>pg_subscription_rel</structname></title>
<para>
This catalog only contains tables known to the subscription after running
either <command>CREATE SUBSCRIPTION</command> or
- <command>ALTER SUBSCRIPTION ... REFRESH</command>.
+ <command>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</command>.
</para>
<table>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index a3471a0442..bead99622e 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -22,7 +22,7 @@
<refsynopsisdiv>
<synopsis>
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> CONNECTION '<replaceable>conninfo</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> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="PARAMETER">set_publication_option</replaceable> [= <replaceable class="PARAMETER">value</replaceable>] [, ... ] ) ]
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> ENABLE
ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> DISABLE
@@ -80,18 +80,29 @@ <title>Parameters</title>
<para>
Changes list of subscribed publications. See
<xref linkend="SQL-CREATESUBSCRIPTION"> for more information.
+ By default this command will also act like <literal>REFRESH
+ PUBLICATION</literal>.
</para>
<para>
- When <literal>REFRESH</literal> is specified, this command will also act
- like <literal>REFRESH
- PUBLICATION</literal>. <literal>refresh_option</literal> specifies
- additional options for the refresh operation, as described
- under <literal>REFRESH PUBLICATION</literal>. When
- <literal>SKIP REFRESH</literal> is specified, the command will not try
- to refresh table information. Note that
- either <literal>REFRESH</literal> or <literal>SKIP REFRESH</literal>
- must be specified.
+ <replaceable>set_publication_option</replaceable> specifies additional
+ options for this operation. The supported options are:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>refresh</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ When false, the command will not try to refresh table information.
+ <literal>REFRESH PUBLICATION</literal> should then be executed separately.
+ The default is <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ Additionally, refresh options as described
+ under <literal>REFRESH PUBLICATION</literal> may be specified.
</para>
</listitem>
</varlistentry>
@@ -107,7 +118,7 @@ <title>Parameters</title>
</para>
<para>
- <literal>refresh_option</literal> specifies additional options for the
+ <replaceable>refresh_option</replaceable> specifies additional options for the
refresh operation. The supported options are:
<variablelist>
@@ -185,7 +196,7 @@ <title>Examples</title>
Change the publication subscribed by a subscription to
<literal>insert_only</literal>:
<programlisting>
-ALTER SUBSCRIPTION mysub SET PUBLICATION insert_only REFRESH;
+ALTER SUBSCRIPTION mysub SET PUBLICATION insert_only;
</programlisting>
</para>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 86eb31df93..ad98b38efe 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -64,12 +64,14 @@ static void
parse_subscription_options(List *options, bool *connect, bool *enabled_given,
bool *enabled, bool *create_slot,
bool *slot_name_given, char **slot_name,
- bool *copy_data, char **synchronous_commit)
+ bool *copy_data, char **synchronous_commit,
+ bool *refresh)
{
ListCell *lc;
bool connect_given = false;
bool create_slot_given = false;
bool copy_data_given = false;
+ bool refresh_given = false;
/* If connect is specified, the others also need to be. */
Assert(!connect || (enabled && create_slot && copy_data));
@@ -92,6 +94,8 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
*copy_data = true;
if (synchronous_commit)
*synchronous_commit = NULL;
+ if (refresh)
+ *refresh = true;
/* Parse options */
foreach(lc, options)
@@ -167,6 +171,16 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
PGC_BACKEND, PGC_S_TEST, GUC_ACTION_SET,
false, 0, false);
}
+ else if (strcmp(defel->defname, "refresh") == 0 && refresh)
+ {
+ if (refresh_given)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or redundant options")));
+
+ refresh_given = true;
+ *refresh = defGetBoolean(defel);
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -315,7 +329,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
*/
parse_subscription_options(stmt->options, &connect, &enabled_given,
&enabled, &create_slot, &slotname_given,
- &slotname, ©_data, &synchronous_commit);
+ &slotname, ©_data, &synchronous_commit,
+ NULL);
/*
* Since creating a replication slot is not transactional, rolling back
@@ -645,7 +660,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
parse_subscription_options(stmt->options, NULL, NULL, NULL,
NULL, &slotname_given, &slotname,
- NULL, &synchronous_commit);
+ NULL, &synchronous_commit, NULL);
if (slotname_given)
{
@@ -680,7 +695,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
parse_subscription_options(stmt->options, NULL,
&enabled_given, &enabled, NULL,
- NULL, NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL, NULL);
Assert(enabled_given);
if (!sub->slotname && enabled)
@@ -712,13 +727,13 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
break;
case ALTER_SUBSCRIPTION_PUBLICATION:
- case ALTER_SUBSCRIPTION_PUBLICATION_REFRESH:
{
bool copy_data;
+ bool refresh;
parse_subscription_options(stmt->options, NULL, NULL, NULL,
NULL, NULL, NULL, ©_data,
- NULL);
+ NULL, &refresh);
values[Anum_pg_subscription_subpublications - 1] =
publicationListToArray(stmt->publication);
@@ -727,12 +742,13 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
update_tuple = true;
/* Refresh if user asked us to. */
- if (stmt->kind == ALTER_SUBSCRIPTION_PUBLICATION_REFRESH)
+ if (refresh)
{
if (!sub->enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions")));
+ errmsg("ALTER SUBSCRIPTION with refresh is not allowed for disabled subscriptions"),
+ errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false).")));
/* Make sure refresh sees the new list of publications. */
sub->publications = stmt->publication;
@@ -754,7 +770,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
parse_subscription_options(stmt->options, NULL, NULL, NULL,
NULL, NULL, NULL, ©_data,
- NULL);
+ NULL, NULL);
AlterSubscription_refresh(sub, copy_data);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7e03624eb4..ada95e5bc3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9279,24 +9279,14 @@ AlterSubscriptionStmt:
n->options = $6;
$$ = (Node *)n;
}
- | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list REFRESH opt_definition
- {
- AlterSubscriptionStmt *n =
- makeNode(AlterSubscriptionStmt);
- n->kind = ALTER_SUBSCRIPTION_PUBLICATION_REFRESH;
- n->subname = $3;
- n->publication = $6;
- n->options = $8;
- $$ = (Node *)n;
- }
- | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list SKIP REFRESH
+ | ALTER SUBSCRIPTION name SET PUBLICATION publication_name_list opt_definition
{
AlterSubscriptionStmt *n =
makeNode(AlterSubscriptionStmt);
n->kind = ALTER_SUBSCRIPTION_PUBLICATION;
n->subname = $3;
n->publication = $6;
- n->options = NIL;
+ n->options = $7;
$$ = (Node *)n;
}
| ALTER SUBSCRIPTION name ENABLE_P
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 8720e713c4..2d2e2c0fbc 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3382,7 +3382,6 @@ typedef enum AlterSubscriptionType
ALTER_SUBSCRIPTION_OPTIONS,
ALTER_SUBSCRIPTION_CONNECTION,
ALTER_SUBSCRIPTION_PUBLICATION,
- ALTER_SUBSCRIPTION_PUBLICATION_REFRESH,
ALTER_SUBSCRIPTION_REFRESH,
ALTER_SUBSCRIPTION_ENABLED
} AlterSubscriptionType;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 91ba8ab95a..4fcbf7efe9 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -82,7 +82,7 @@ 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 SKIP REFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = false);
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
-- fail
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 4b694a357e..36fa1bbac8 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -61,7 +61,7 @@ CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
\dRs+
-ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
+ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = false);
ALTER SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist2';
ALTER SUBSCRIPTION testsub SET (slot_name = 'newname');
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index e5638d3322..f9cf5e4392 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -143,7 +143,7 @@
"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 (copy_data = false)"
+"ALTER SUBSCRIPTION tap_sub SET PUBLICATION tap_pub_ins_only WITH (copy_data = false)"
);
$node_publisher->poll_query_until('postgres',
"SELECT pid != $oldpid FROM pg_stat_replication WHERE application_name = '$appname';"
--
2.13.0
On 6/2/17 22:13, Peter Eisentraut wrote:
On 5/27/17 06:54, Petr Jelinek wrote:
On 27/05/17 04:00, Euler Taveira wrote:
2017-05-26 21:29 GMT-03:00 Petr Jelinek <petr.jelinek@2ndquadrant.com
<mailto:petr.jelinek@2ndquadrant.com>>:Actually another possibility would be to remove the REFRESH keyword
completely and just have [ WITH (...) ] and have the refresh option
there, ie simplified version of what you have suggested (without the
ugliness of specifying refresh twice to disable).It will cause confusion. It seems that WITH sets ALTER SUBSCRIPTION
properties. Indeed, they are REFRESH properties. I think we shouldn't
exclude REFRESH keyword. Syntax leaves no doubt that WITH are REFRESH
properties.Maybe, I don't know, it might not be that confusing when SET PUBLICATION
and REFRESH PUBLICATION have same set of WITH options.I'm not sure what the conclusion from the above discussion was supposed
to be, but here is a patch.
committed
--
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