Re: [HACKERS] ALTER SUBSCRIPTION ..SET PUBLICATION refreshis not throwing error.

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_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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers