From: | Emre Hasegeli <emre(at)hasegeli(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | Re: "pgoutput" options missing on documentation |
Date: | 2023-12-15 13:35:46 |
Message-ID: | CAE2gYzzpoNDdrb6_OnKzdHh9kXmvfyiPnA6yZ=joPJCv8arATg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> I saw that the original "publication_names" error was using
> errcode(ERRCODE_INVALID_PARAMETER_VALUE), but TBH since there is no
> option given at all I felt ERRCODE_SYNTAX_ERROR might be more
> appropriate errcode for those 2 mandatory option errors.
It makes sense to me. Changed.
> 2.
>
> I saw that the chapter "55.4. Streaming Replication Protocol" [1]
> introduces "START_REPLICATION SLOT slot_name LOGICAL ..." command and
> it says
> ---
> option_name
> The name of an option passed to the slot's logical decoding plugin.
> ---
>
> Perhaps that part should now include a reference to your new information:
>
> SUGGESTION
> option_name
> The name of an option passed to the slot's logical decoding plugin.
> Please see <XXX (55.5.1)> for details about options that are accepted
> by the standard (pgoutput) plugin.
Good idea. Incorporated.
> 3.
> <para>
> - The logical replication <literal>START_REPLICATION</literal> command
> - accepts following parameters:
> + Using the <literal>START_REPLICATION</literal> command,
> + <literal>pgoutput</literal>) accepts the following options:
>
>
> Oops, you copied my typo. There is a spurious ')'.
Fixed.
> 4.
> +<!-- Backpack through version 16. -->
> + <varlistentry>
> + <term>
> + origin
> + </term>
> + <listitem>
> + <para>
> + String option to send only changes by an origin. It also gets
> + the option "none" to send the changes that have no origin associated,
> + and the option "any" to send the changes regardless of their origin.
> + This can be used to avoid loops (infinite replication of the same data)
> + among replication nodes.
> + </para>
> + </listitem>
> + </varlistentry>
> </variablelist>
>
> AFAIK pgoutput only understands origin values "any" and "none" and
> nothing else; I felt the "It also gets..." part implies it is more
> flexible than it is.
>
> SUGGESTION
> Possible values are "none" (to only send the changes that have no
> origin associated), or "any" (to send the changes regardless of their
> origin).
Oh, it's not how I understood it. I think you are right. Changed.
> OK, to deal with that can't you just include "origin" in the first
> group which has no special protocol requirements?
I think it'd be confusing because the option is not available before
version 16. I think it should really check for the version number and
complain if it's less than 4.
> SUGGESTION
> -proto_version
> -publication_names
> -binary
> -messages
> -origin
>
> Requires minimum protocol version 2:
> -streaming (boolean)
>
> Requires minimum protocol version 3:
> -two_phase
>
> Requires minimum protocol version 4:
> -streaming (parallel)
I am still not sure if this is any better. I don't like that
"streaming" appears twice, and I wouldn't know how to format this
nicely.
The new versions are attached.
I also added "Required." for "proto_version" and "publication_names".
Attachment | Content-Type | Size |
---|---|---|
v02-0001-pgoutput-Improve-error-messages-for-options.patch | application/octet-stream | 8.7 KB |
v02-0002-pgoutput-Document-missing-options.patch | application/octet-stream | 7.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2023-12-15 13:42:57 | Re: Clean up find_typedefs and add support for Mac |
Previous Message | Pavel Stehule | 2023-12-15 13:19:07 | Re: [PATCH] Add --syntax to postgres for SQL syntax checking |