RE: Handle infinite recursion in logical replication setup

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

From: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
To: vignesh C <vignesh21@gmail.com>, Peter Smith <smithpb2250@gmail.com>
Cc: 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-30T03:47:24Z
Lists: pgsql-hackers
On Tue, Jun 28, 2022 2:18 PM vignesh C <vignesh21@gmail.com> wrote:
> 
> Thanks for the comments, the attached  v25 patch has the changes for the
> same.
> 

Thanks for updating the patch. Here are some comments.

0002 patch:
==============
1.
+# Test the CREATE SUBSCRIPTION 'origin' parameter and its interaction with
+# 'copy_data' parameter.

It seems we should move "and its interaction with 'copy_data' parameter" to
0003 patch.

0003 patch
==============
1.
When using ALTER SUBSCRIPTION ... REFRESH, subscription will throw an error if
any table is subscribed in publisher, even if the table has been subscribed
before refresh (which won't do the initial copy when refreshing). It looks the
previously subscribed tables don't need this check. Would it be better that we
only check the tables which need to do the initial copy?

2.
+				errmsg("table:%s.%s might have replicated data in the publisher",
+					   nspname, relname),

I think the table name needs to be enclosed in double quotes, which is
consistent with other messages.

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