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

On 02/05/17 22:40, Robert Haas wrote:
> On Tue, May 2, 2017 at 3:02 PM, Petr Jelinek
> <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>> That sounds okay. I know PeterE didn't like the lower case and
>> underscore separated words for options in the original patch, so I'd
>> like to hear his opinion on this. I am not sure how much advantage is
>> there in removing the '=' in between the key and value. That's the main
>> difference between generic options and definitions (well and definitions
>> can have 2 words for key, but that's something I have added anyway), I
>> don't really understand why we have both and use one for some commends
>> and the other for others btw.
>
> I don't care all *that* much about the equals sign, although I think
> it's marginally more elegant without it, and VACUUM, EXPLAIN, and COPY
> all do it that way. But I think allowing two-word names is just a
> pile of parser nastiness that we'd be far better off without. If you
> use the same syntax as VACUUM, EXPLAIN, and COPY, all options are a
> single identifier.

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.

To repeat the proposal:
- change the WITH (...) clauses in subscription/publication commands to:
(create_slot true/false, connect true/false, slot_name 'something',
copy_data true/false, etc)

- change the NOREFRESH to NO REFRESH in ALTER SUBSCRIPTION name SET
PUBLICATION (btw I originally had SKIP REFRESH there but changed it to
NOREFRESH for consistency with the other NO* stuff, wonder if SKIP would
sound more english).

- change the (publish insert/nopublish insert/publish update/nopublish
update), etc options to (publish 'update,insert').

And one question, if we are not using the definitions (key = value)
should we keep the WITH keyword in the syntax, would it be confusing?

> If it's got to be multiple words, they can be
> separated by underscores. But with what you've got right now, it's
> sometimes one identifier even when it's two words, and sometimes two
> identifiers.
>

The main inconsistency is the synchronous_commit which is that way to
make it clear it's same as the GUC it changes, but looks like that was a
mistake.

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