> 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".