> 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".
v02-0001-pgoutput-Improve-error-messages-for-options.patch
Description: Binary data
v02-0002-pgoutput-Document-missing-options.patch
Description: Binary data
