From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
Date: Tue, 2 May 2017 21:02:19 +0200
Message-ID: <0afdeda8-d8a1-56d8-f637-7f110c2ca443@2ndquadrant.com> (raw)
In-Reply-To: <CA+TgmoYe8_FZ9oNTxL-q42kUXGKCDrrG512qbyKAQw9ROBvQ9g@mail.gmail.com>
References: <CA+TgmoZmkbpAWRzVKDVcHnTBkYjJEFS8=09RL-G3zgdozCLFHQ@mail.gmail.com>
<27478.1493727233@sss.pgh.pa.us>
<cacf7538-5658-a93f-c03a-83d309ed1d9a@2ndquadrant.com>
<CA+TgmoYe8_FZ9oNTxL-q42kUXGKCDrrG512qbyKAQw9ROBvQ9g@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 02/05/17 19:42, Robert Haas wrote:
> On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
> <petr.jelinek@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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: petr.jelinek@2ndquadrant.com, robertmhaas@gmail.com, tgl@sss.pgh.pa.us, michael.paquier@gmail.com
Subject: Re: logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)
In-Reply-To: <0afdeda8-d8a1-56d8-f637-7f110c2ca443@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox