Re: Handle infinite recursion in logical replication setup

On Sat, Jul 9, 2022 at 8:11 PM vignesh C <vignesh21(at)gmail(dot)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.