Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

On 04/05/17 23:29, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, May 3, 2017 at 12:38 AM, Petr Jelinek
>> <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>>> Ok, Let me be clear, I actually happen to agree with your proposal. The
>>> reason I am moaning is that I always seem to find myself doing tons of
>>> mechanical work to rewrite some cosmetic aspect of some patch based on
>>> which committer is paying attention in a specific week. So while I am
>>> for doing exactly what you proposed, I'd like to see couple of +1s first
>>> (Peter?) since I don't want to rewrite it to something different again
>>> and it's also long past freeze.
>
>> So, Tom Lane and Thom Brown and Josh Drake all seemed generally in
>> favor of cleaning this up. Perhaps they could opine on this
>> particular proposal.
>
> It seems like there's some remaining indecision between "make it look
> like the options in EXPLAIN, VACUUM, etc" and "make it look like the
> WITH options found in some other statements". I do not have a strong
> opinion which one to do, but I'd definitely say that you should use WITH
> in the latter case but not in the former. I think this mostly boils down
> to whether to use "=" or not; you've got "not" in the proposal, which
> means you are following the EXPLAIN precedent and should not use WITH.
>

Okay, here is my initial attempt on changing this. I opted for WITH and
"=" as I like it slightly better (also the generic_options expect values
to be quoted which I don't like and then I would have to again invent
something like generic_options but not quite the same).

Most of the changes go to doc and tests, not that much code has changed
as I already used the definiton (which is the parser's name for these
WITH things). Except that I removed the NO shorthands and changed
publish_insert,etc to publish = 'insert,...'. I also changed the
NOREFRESH to SKIP REFRESH.

I didn't touch the DROP SUBSCRIPTION slot handling so far, that needs to
be separate patch as there is behavior change there, while this is
purely cosmetic and IMHO it's better to not mix those. (I plan to send
patch for that which changes the behavior heavily soonish as well)

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