On 02/05/17 19:42, Robert Haas wrote:
> On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
> <petr.jeli...@2ndquadrant.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.
I am not sure what you mean by this, we always have to invent option
names if they are new options, even if we use generic options (which I
guess is what you mean by "extensible options syntax"). I used the
definitions instead of generic options, this means that the supported
syntax also includes COPY DATA = true/false, CREATE SLOT = true/false
etc, the NO* are just shorthands, it's quite simple to remove those.
> 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.
See above.
>
> 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.
>
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.
--
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
