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

On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
<petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
> I am happy to implement something different, it's quite trivial to
> change. But I am not going to propose anything different as I can't
> think of better syntax (if I could I would have done it). I don't like
> the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
> it also seems to not map very well to action (as opposed to output
> option as it is in EXPLAIN). It might not be very close to SQL way but
> that's because SQL way would be do not do any of those default actions
> unless they are actually asked for (ie NODROP SLOT would be default and
> DROP SLOT would be the option) but that's IMHO less user friendly.

So the cases where this "NO" prefixing comes up are:

1. CREATE SUBSCRIPTION

<phrase>where <replaceable class="PARAMETER">option</replaceable> can
be:</phrase>

| ENABLED | DISABLED
| CREATE SLOT | NOCREATE SLOT
| SLOT NAME = <replaceable class="PARAMETER">slot_name</replaceable>
| COPY DATA | NOCOPY DATA
| SYNCHRONOUS_COMMIT = <replaceable
class="PARAMETER">synchronous_commit</replaceable>
| NOCONNECT

I think it would have been a lot better to use the extensible options
syntax for this instead of inventing something new that's not even
consistent with itself. You've got SYNCHRONOUS_COMMIT with a hyphen
and CREATE SLOT with no hyphen and NOCOPY DATA with no hyphen and a
space left out. With the extensible options syntax, this would be
(enabled true/false, create_slot true/false, slot_name whatever,
synchronous_commit true/false, connect true/false). If we're going to
keep the present monstrosity, we can I think still change NOCONNECT to
NO CONNECT, but there's no fixing NOCOPY DATA in this syntax model.

2. ALTER SUBSCRIPTION

ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] {
REFRESH WITH ( puboption [, ... ] ) | NOREFRESH }

There is no obvious reason why this could not have been spelled NO
REFRESH instead of adding a new keyword.

3. DROP SUBSCRIPTION

DROP SUBSCRIPTION [ IF EXISTS ] name [ DROP SLOT | NODROP SLOT ]

This is where we started, and I have nothing to add to what I (and
Tom) have already said.

4. CREATE PUBLICATION

CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
[ FOR TABLE [ ONLY ] <replaceable
class="parameter">table_name</replaceable> [ * ] [, ...]
| FOR ALL TABLES ]
[ WITH ( <replaceable class="parameter">option</replaceable> [, ... ] ) ]

<phrase>where <replaceable class="parameter">option</replaceable> can
be:</phrase>

PUBLISH INSERT | NOPUBLISH INSERT
| PUBLISH UPDATE | NOPUBLISH UPDATE
| PUBLISH DELETE | NOPUBLISH DELETE

Again, the extensible options syntax like we use for EXPLAIN would
have been better here. You could have said (publish_insert
true/false, publish_update true/false, publish_delete true/false), for
instance, or combined them into a single option like (publish
'insert,update') to omit deletes.

So it doesn't actually look hard to get rid of all of the NO prefixes.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company