Re: Handle infinite recursion in logical replication setup

I checked the latest v9-0001 patch. Below are my review comments.

Other than these few trivial comments this 0001 patch looks good to me.

~~~

1. src/backend/replication/pgoutput/pgoutput.c - whitespace

@@ -1696,6 +1714,10 @@ static bool
pgoutput_origin_filter(LogicalDecodingContext *ctx,
RepOriginId origin_id)
{
+ PGOutputData *data = (PGOutputData *) ctx->output_plugin_private;
+
+ if (data->local_only && origin_id != InvalidRepOriginId)
+ return true;
return false;
}

Suggest to add a blank line after the return true;

~~~

2. src/bin/psql/tab-complete.c - not alphabetical

@@ -1874,7 +1874,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("(", "PUBLICATION");
/* ALTER SUBSCRIPTION <name> SET ( */
else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) &&
TailMatches("SET", "("))
- COMPLETE_WITH("binary", "slot_name", "streaming",
"synchronous_commit", "disable_on_error");
+ COMPLETE_WITH("binary", "slot_name", "streaming", "local_only",
"synchronous_commit", "disable_on_error");

2a. AFAIK the code intended that these options be listed in
alphabetical order (I think the recent addition of disable_on_error is
also wrong here). So "local_only" should be moved.

@@ -3156,7 +3156,7 @@ psql_completion(const char *text, int start, int end)
/* Complete "CREATE SUBSCRIPTION <name> ... WITH ( <opt>" */
else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH", "("))
COMPLETE_WITH("binary", "connect", "copy_data", "create_slot",
- "enabled", "slot_name", "streaming",
+ "enabled", "slot_name", "streaming", "local_only",
"synchronous_commit", "two_phase", "disable_on_error");

2b. ditto

~~~

3. src/test/subscription/t/032_localonly.pl - wrong message

+$node_C->wait_for_catchup($appname_B2);
+$node_B->wait_for_catchup($appname_A);
+
+$result = $node_B->safe_psql('postgres', "SELECT * FROM tab_full;");
+is( $result, qq(11
+12
+13),
+ 'Inserted successfully without leading to infinite recursion in
circular replication setup'
+);
+
+# check that the data published from node_C to node_B is not sent to node_A
+$result = $node_A->safe_psql('postgres', "SELECT * FROM tab_full;");
+is( $result, qq(11
+12),
+ 'Inserted successfully without leading to infinite recursion in
circular replication setup'
+);
+

The new test looked good, but the cut/paste text message ('Inserted
successfully without leading to infinite recursion in circular
replication setup') maybe needs changing because there is nothing
really "circular" about this test case.

------
Kind Regards,
Peter Smith.
Fujitsu Australia.