Re: Handle infinite recursion in logical replication setup

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, Peter Smith <smithpb2250@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-07-11T11:33:34Z
Lists: pgsql-hackers

Attachments

On Sat, Jul 9, 2022 at 8:11 PM vignesh C <vignesh21@gmail.com> wrote:
>

Thanks, a few more comments on v30_0002*
1.
+/*
+ * Represents whether copy_data parameter is specified with off, on or force.

A comma is required after on.

2.
  qsort(subrel_local_oids, list_length(subrel_states),
    sizeof(Oid), oid_cmp);

+ check_pub_table_subscribed(wrconn, sub->publications, copy_data,
+    sub->origin, subrel_local_oids,
+    list_length(subrel_states));

We can avoid using list_length by using an additional variable in this case.

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

Why ':' is used after the table in the above message? I don't see such
a convention at other places in the code. Also, having might in the
error messages makes it less clear, so, can we slightly change the
message as in the attached patch?

4. I have made some additional changes in the comments, kindly check
the attached and merge those if you are okay.

5.
+$node_C->safe_psql(
+     'postgres', "
+        DELETE FROM tab_full");
+$node_B->safe_psql(
+     'postgres', "
+        DELETE FROM tab_full where a = 13");

Don't we need to wait for these operations to replicate?

-- 
With Regards,
Amit Kapila.

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