From: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> |
---|---|
To: | Euler Taveira <euler(at)timbira(dot)com(dot)br>, tushar <tushar(dot)ahuja(at)enterprisedb(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Subject: | Re: ALTER SUBSCRIPTION ..SET PUBLICATION <no name> refresh is not throwing error. |
Date: | 2017-05-24 19:28:39 |
Message-ID: | 04df4b4b-947d-5cd0-eb00-a2af2aaedf61@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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(dot)ahuja(at)enterprisedb(dot)com
>> <mailto:tushar(dot)ahuja(at)enterprisedb(dot)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_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.
>
> 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
Attachment | Content-Type | Size |
---|---|---|
0001-Remove-the-SKIP-REFRESH-syntax-suggar-in-ALTER-SUBSC.patch | binary/octet-stream | 2.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2017-05-24 19:34:07 | Re: Tightening isspace() tests where behavior should match SQL parser |
Previous Message | Petr Jelinek | 2017-05-24 19:21:20 | Re: ALTER SUBSCRIPTION ..SET PUBLICATION <no name> refresh is not throwing error. |