RE: Handle infinite recursion in logical replication setup

On Mon, Jun 6, 2022 1:14 PM vignesh C <vignesh21(at)gmail(dot)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