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+3-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+6-10
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+54-39
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