Re: Handle infinite recursion in logical replication setup

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>

To: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.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-30T16:10:14Z

Lists: pgsql-hackers

Attachments

v26-0001-Add-a-missing-test-to-verify-only-local-paramete.patch (text/x-patch) patch v26-0001
v26-0004-Document-bidirectional-logical-replication-steps.patch (text/x-patch) patch v26-0004
v26-0002-Skip-replication-of-non-local-data.patch (text/x-patch) patch v26-0002
v26-0003-Check-and-throw-an-error-if-publication-tables-w.patch (text/x-patch) patch v26-0003

On Thu, Jun 30, 2022 at 9:17 AM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> 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.

Modified

> 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?

Modified

> 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.

Modified

Thanks for the comments, the attached v26 patch has the changes for the same.

Regards,
Vignesh

Commits

Improved CREATE SUBSCRIPTION message for clarity
- 1e7ca1189ccc 17.0 landed
- 842b65905046 16.0 landed
Fix the test case introduced by commit 8756930190.
- 032465157347 16.0 landed
Raise a warning if there is a possibility of data from multiple origins.
- 875693019053 16.0 landed
Add wait_for_subscription_sync for TAP tests.
- 0c20dd33db16 16.0 cited
Bump catversion for commit d8cd0c6c95c0120168df93aae095df4e0682a08a.
- 5f858dd3bebd 16.0 cited
Allow users to skip logical replication of data having origin.
- 366283961ac0 16.0 landed
Improve two comments related to a boolean DefElem's value
- 8445f5a21d40 16.0 cited
doc: Fix man page whitespace issues
- d7ab2a9a3c0a 15.0 cited