Thread

Commits

  1. Doc: improve documentation of CREATE/ALTER SUBSCRIPTION.

  2. Doc: Change optional parameters grouping in Create Subscription.

  1. PG Docs - CREATE SUBSCRIPTION option list order

    Peter Smith <smithpb2250@gmail.com> — 2021-04-18T23:59:44Z

    Hi,
    
    The CREATE SUBSCRIPTION documentation [1] includes a list of "WITH"
    options, which are currently in some kind of quasi alphabetical /
    random order which I found unnecessarily confusing.
    
    I can't think of any good reason for the current ordering, so PSA my
    patch which has identical content but just re-orders that option list
    to be alphabetical.
    
    ------
    [1] = https://www.postgresql.org/docs/devel/sql-createsubscription.html
    
    Kind Regards,
    Peter Smith.
    Fujitsu Australia
    
  2. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Euler Taveira <euler@eulerto.com> — 2021-04-19T01:01:36Z

    On Sun, Apr 18, 2021, at 8:59 PM, Peter Smith wrote:
    > The CREATE SUBSCRIPTION documentation [1] includes a list of "WITH"
    > options, which are currently in some kind of quasi alphabetical /
    > random order which I found unnecessarily confusing.
    > 
    > I can't think of any good reason for the current ordering, so PSA my
    > patch which has identical content but just re-orders that option list
    > to be alphabetical.
    AFAICS there is not reason to use a random order here. I think this parameter
    list is in frequency of use. Your patch looks good to me.
    
    
    --
    Euler Taveira
    EDB   https://www.enterprisedb.com/
    
  3. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Amit Kapila <amit.kapila16@gmail.com> — 2021-04-19T04:08:54Z

    On Mon, Apr 19, 2021 at 6:32 AM Euler Taveira <euler@eulerto.com> wrote:
    >
    > On Sun, Apr 18, 2021, at 8:59 PM, Peter Smith wrote:
    >
    > The CREATE SUBSCRIPTION documentation [1] includes a list of "WITH"
    > options, which are currently in some kind of quasi alphabetical /
    > random order which I found unnecessarily confusing.
    >
    > I can't think of any good reason for the current ordering, so PSA my
    > patch which has identical content but just re-orders that option list
    > to be alphabetical.
    >
    > AFAICS there is not reason to use a random order here. I think this parameter
    > list is in frequency of use. Your patch looks good to me.
    >
    
    I also agree that the current order is quite random. One idea is to
    keep them in alphabetical order as suggested by Peter and the other
    could be to arrange parameters based on properties, for example, there
    are few parameters like binary, streaming, copy_data which are in some
    way related to the data being replicated and others are more of slot
    properties (create_slot, slot_name). I see that few parameters among
    these have some dependencies on other parameters as well. I noticed
    that the other DDL commands like Create Table, Create Index doesn't
    have the WITH clause parameters in any alphabetical order so it might
    be better if the related parameters can be together here but if we
    think it is not a good idea in this case due to some reason then
    probably keeping them in alphabetical order makes sense.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  4. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Peter Smith <smithpb2250@gmail.com> — 2021-04-19T05:02:32Z

    On Mon, Apr 19, 2021 at 2:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Mon, Apr 19, 2021 at 6:32 AM Euler Taveira <euler@eulerto.com> wrote:
    > >
    > > On Sun, Apr 18, 2021, at 8:59 PM, Peter Smith wrote:
    > >
    > > The CREATE SUBSCRIPTION documentation [1] includes a list of "WITH"
    > > options, which are currently in some kind of quasi alphabetical /
    > > random order which I found unnecessarily confusing.
    > >
    > > I can't think of any good reason for the current ordering, so PSA my
    > > patch which has identical content but just re-orders that option list
    > > to be alphabetical.
    > >
    > > AFAICS there is not reason to use a random order here. I think this parameter
    > > list is in frequency of use. Your patch looks good to me.
    > >
    >
    > I also agree that the current order is quite random. One idea is to
    > keep them in alphabetical order as suggested by Peter and the other
    > could be to arrange parameters based on properties, for example, there
    > are few parameters like binary, streaming, copy_data which are in some
    > way related to the data being replicated and others are more of slot
    > properties (create_slot, slot_name). I see that few parameters among
    > these have some dependencies on other parameters as well. I noticed
    > that the other DDL commands like Create Table, Create Index doesn't
    > have the WITH clause parameters in any alphabetical order so it might
    > be better if the related parameters can be together here but if we
    > think it is not a good idea in this case due to some reason then
    > probably keeping them in alphabetical order makes sense.
    >
    
    Yes, if there were dozens of list items then I would agree that they
    should be grouped somehow. But there aren't.
    
    I think what may seem like a clever grouping to one reader may look
    more like an over-complicated muddle to somebody else.
    
    So I prefer just to apply the KISS Principle.
    
    ------
    Kind Regards,
    Peter Smith.
    Fujitsu Australia.
    
    
    
    
  5. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Amit Kapila <amit.kapila16@gmail.com> — 2021-04-19T05:16:31Z

    On Mon, Apr 19, 2021 at 10:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > On Mon, Apr 19, 2021 at 2:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > >
    > > On Mon, Apr 19, 2021 at 6:32 AM Euler Taveira <euler@eulerto.com> wrote:
    > > >
    > > > On Sun, Apr 18, 2021, at 8:59 PM, Peter Smith wrote:
    > > >
    > > > The CREATE SUBSCRIPTION documentation [1] includes a list of "WITH"
    > > > options, which are currently in some kind of quasi alphabetical /
    > > > random order which I found unnecessarily confusing.
    > > >
    > > > I can't think of any good reason for the current ordering, so PSA my
    > > > patch which has identical content but just re-orders that option list
    > > > to be alphabetical.
    > > >
    > > > AFAICS there is not reason to use a random order here. I think this parameter
    > > > list is in frequency of use. Your patch looks good to me.
    > > >
    > >
    > > I also agree that the current order is quite random. One idea is to
    > > keep them in alphabetical order as suggested by Peter and the other
    > > could be to arrange parameters based on properties, for example, there
    > > are few parameters like binary, streaming, copy_data which are in some
    > > way related to the data being replicated and others are more of slot
    > > properties (create_slot, slot_name). I see that few parameters among
    > > these have some dependencies on other parameters as well. I noticed
    > > that the other DDL commands like Create Table, Create Index doesn't
    > > have the WITH clause parameters in any alphabetical order so it might
    > > be better if the related parameters can be together here but if we
    > > think it is not a good idea in this case due to some reason then
    > > probably keeping them in alphabetical order makes sense.
    > >
    >
    > Yes, if there were dozens of list items then I would agree that they
    > should be grouped somehow. But there aren't.
    >
    
    I think this list is going to grow in the future as we enhance this
    subsystem. For example, the pending 2PC patch will add another
    parameter to this list.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  6. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Peter Smith <smithpb2250@gmail.com> — 2021-08-09T02:42:35Z

    v1 -> v2
    
    Rebased.
    
    ------
    Kind Regards,
    Peter Smith.
    Fujitsu Australia
    
  7. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-09-04T18:53:02Z

    Amit Kapila <amit.kapila16@gmail.com> writes:
    > On Mon, Apr 19, 2021 at 10:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
    >> Yes, if there were dozens of list items then I would agree that they
    >> should be grouped somehow. But there aren't.
    
    > I think this list is going to grow in the future as we enhance this
    > subsystem. For example, the pending 2PC patch will add another
    > parameter to this list.
    
    Well, we've got nine now; growing to ten wouldn't be a lot.  However,
    I think that grouping the options would be helpful because the existing
    presentation seems extremely confusing.  In particular, I think it might
    help to separate the options that only determine what happens during
    CREATE SUBSCRIPTION from those that control how replication behaves later.
    (Are the latter set the same ones that are shared with ALTER
    SUBSCRIPTION?)
    
    Also, I think a lot of these descriptions desperately need copy-editing.
    The grammar is shoddy and the markup is inconsistent.
    
    			regards, tom lane
    
    
    
    
  8. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Amit Kapila <amit.kapila16@gmail.com> — 2021-09-06T03:14:02Z

    On Sun, Sep 5, 2021 at 12:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >
    > Amit Kapila <amit.kapila16@gmail.com> writes:
    > > On Mon, Apr 19, 2021 at 10:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
    > >> Yes, if there were dozens of list items then I would agree that they
    > >> should be grouped somehow. But there aren't.
    >
    > > I think this list is going to grow in the future as we enhance this
    > > subsystem. For example, the pending 2PC patch will add another
    > > parameter to this list.
    >
    > Well, we've got nine now; growing to ten wouldn't be a lot.  However,
    > I think that grouping the options would be helpful because the existing
    > presentation seems extremely confusing.  In particular, I think it might
    > help to separate the options that only determine what happens during
    > CREATE SUBSCRIPTION from those that control how replication behaves later.
    >
    
    +1. I think we can group them as (a) create_slot, slot_name, enabled,
    connect, and (b) copy_data, synchronous_commit, binary, streaming,
    two_phase. The first controls what happens during Create Subscription
    and the later ones control the replication behavior later.
    
    > (Are the latter set the same ones that are shared with ALTER
    > SUBSCRIPTION?)
    >
    
    If we agree with the above categorization then not all of them fall
    into the latter category.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  9. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Peter Smith <smithpb2250@gmail.com> — 2021-09-08T06:54:01Z

    v2 --> v3
    
    The subscription_parameter names are now split into 2 groups using
    Amit's suggestion [1] on how to categorise them.
    
    I also made some grammar improvements to their descriptions.
    
    PSA.
    
    ------
    [1] https://www.postgresql.org/message-id/CAA4eK1Kmu74xHk2jcHTmKq8HBj3xK6n%3DRfiJB6dfV5zVSqqiFg%40mail.gmail.com
    
    Kind Regards,
    Peter Smith.
    Fujitsu Australia
    
  10. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Amit Kapila <amit.kapila16@gmail.com> — 2021-09-09T04:20:17Z

    On Wed, Sep 8, 2021 at 12:24 PM Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > v2 --> v3
    >
    > The subscription_parameter names are now split into 2 groups using
    > Amit's suggestion [1] on how to categorise them.
    >
    > I also made some grammar improvements to their descriptions.
    >
    
    I have made minor edits to your first patch and it looks good to me. I
    am not sure what exactly Tom has in mind related to grammatical
    improvements, so it is better if he can look into that part of your
    proposal (basically second patch
    v4-0002-PG-Docs-Create-Subscription-options-rewording).
    
    -- 
    With Regards,
    Amit Kapila.
    
  11. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Amit Kapila <amit.kapila16@gmail.com> — 2021-09-13T05:34:58Z

    On Thu, Sep 9, 2021 at 9:50 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Wed, Sep 8, 2021 at 12:24 PM Peter Smith <smithpb2250@gmail.com> wrote:
    > >
    > > v2 --> v3
    > >
    > > The subscription_parameter names are now split into 2 groups using
    > > Amit's suggestion [1] on how to categorise them.
    > >
    > > I also made some grammar improvements to their descriptions.
    > >
    >
    > I have made minor edits to your first patch and it looks good to me.
    >
    
    Pushed the first patch. I am not so sure about the second one so I
    won't do anything for the same. I'll close this CF entry in a day or
    two unless there is an interest in the second patch.
    
    With Regards,
    Amit Kapila.
    
    
    
    
  12. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-09-13T18:28:55Z

    Amit Kapila <amit.kapila16@gmail.com> writes:
    > Pushed the first patch. I am not so sure about the second one so I
    > won't do anything for the same. I'll close this CF entry in a day or
    > two unless there is an interest in the second patch.
    
    Sorry for not reviewing this more promptly.
    
    I made some further edits in the 0002 patch and pushed it.
    
    			regards, tom lane
    
    
    
    
  13. Re: PG Docs - CREATE SUBSCRIPTION option list order

    Amit Kapila <amit.kapila16@gmail.com> — 2021-09-14T03:09:17Z

    On Mon, Sep 13, 2021 at 11:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >
    > Amit Kapila <amit.kapila16@gmail.com> writes:
    > > Pushed the first patch. I am not so sure about the second one so I
    > > won't do anything for the same. I'll close this CF entry in a day or
    > > two unless there is an interest in the second patch.
    >
    > Sorry for not reviewing this more promptly.
    >
    > I made some further edits in the 0002 patch and pushed it.
    >
    
    Thanks.
    
    -- 
    With Regards,
    Amit Kapila.