Horton

RE: Handle infinite recursion in logical replication setup

Yu Shi (Fujitsu) <shiy.fnst@fujitsu.com>

View thread View on postgr.es
From: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-06-23T01:35:15Z
Lists: pgsql-hackers 
On Mon, Jun 20, 2022 7:55 PM vignesh C <vignesh21@gmail.com> wrote:
> 
> Thanks for the comment, the v22 patch attached has the changes for the
> same.

Thanks for updating the patch, here are some comments on 0003 patch.

1. 032_origin.pl
+###############################################################################
+# Join 3rd node (node_C) to the existing 2 nodes(node_A & node_B) bidirectional
+# replication setup when the existing nodes (node_A & node_B) and the new node
+# (node_C) does not have any data.
+###############################################################################

"does not have any data" should be "do not have any data" I think.

2.
The comment for 032_origin.pl:

# Test the CREATE SUBSCRIPTION 'origin' parameter.

After applying this patch, this file tests no only 'origin' parameter, but also
"copy_data" parameter, so should we modify this comment?

Besides, should we change the file name in this patch? It looks more like test
cases for bidirectional logical replication.

3. subscriptioncmds.c
	/* Set default values for the boolean supported options. */
...
	if (IsSet(supported_opts, SUBOPT_CREATE_SLOT))
 		opts->create_slot = true;
 	if (IsSet(supported_opts, SUBOPT_COPY_DATA))
-		opts->copy_data = true;
+		opts->copy_data = COPY_DATA_ON;
 	if (IsSet(supported_opts, SUBOPT_REFRESH))
 		opts->refresh = true;
 	if (IsSet(supported_opts, SUBOPT_BINARY))

"copy_data" option is not Boolean now, which is inconsistent with the comment.
So maybe we can change the comment here? ("the boolean supported options" ->
"the supported options")

Regards,
Shi yu

Commits

  1. Improved CREATE SUBSCRIPTION message for clarity

  2. Fix the test case introduced by commit 8756930190.

  3. Raise a warning if there is a possibility of data from multiple origins.

  4. Add wait_for_subscription_sync for TAP tests.

  5. Bump catversion for commit d8cd0c6c95c0120168df93aae095df4e0682a08a.

  6. Allow users to skip logical replication of data having origin.

  7. Improve two comments related to a boolean DefElem's value

  8. doc: Fix man page whitespace issues