Re: ALTER SUBSCRIPTION ..SET PUBLICATION <no name> refresh is not throwing error.

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.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services