On Mon, Jun 6, 2022 1:14 PM vignesh C <vignes...@gmail.com> wrote:
>
> The attached v18 patch has the fixes for the same.
>
Thanks for updating the patch, here are some comments.
0002 patch
==============
1.
+ <varlistentry>
+ <term><literal>origin</literal> (<type>string</type>)</term>
+ <listitem>
+ <para>
It maybe better if the type of "origin" parameter is enum, as it cannot be any
string and only has two valid values.
2.
@@ -607,6 +626,11 @@ CreateSubscription(ParseState *pstate,
CreateSubscriptionStmt *stmt,
LOGICALREP_TWOPHASE_STATE_PENDING :
LOGICALREP_TWOPHASE_STATE_DISABLED);
values[Anum_pg_subscription_subdisableonerr - 1] =
BoolGetDatum(opts.disableonerr);
+ if (opts.origin)
+ values[Anum_pg_subscription_suborigin - 1] =
+ CStringGetTextDatum(opts.origin);
+ else
+ nulls[Anum_pg_subscription_suborigin - 1] = true;
values[Anum_pg_subscription_subconninfo - 1] =
CStringGetTextDatum(conninfo);
if (opts.slot_name)
Document of "CREATE SUBSCRIPTION" says, the default value of "origin" is "any",
so why not set
suborigin to "any" when user doesn't specify this parameter?
0003 patch
==============
1.
@@ -300,6 +310,11 @@ CREATE SUBSCRIPTION <replaceable
class="parameter">subscription_name</replaceabl
replication from the publisher. The default is
<literal>false</literal>.
</para>
+ <para>
+ There is some interaction between the <literal>origin</literal>
+ parameter and <literal>copy_data</literal> parameter. Refer to the
+ <xref linkend="sql-createsubscription-notes" /> for details.
+ </para>
</listitem>
</varlistentry>
I think this change should be put together with "origin" parameter, instead of
"disable_on_error".
0004 patch
==============
1.
+ <para>
+ Now the bidirectional logical replication setup is complete between
+ <literal>node1</literal>, <literal>node2</literal> and
+ <literal>node2</literal>. Any subsequent changes in one node will
+ replicate the changes to the other nodes.
+ </para>
I think "node1, node2 and node2" should be "node1, node2 and node3".
Regards,
Shi yu
