RE: Data is copied twice when specifying both child and parent table in publication

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, Peter Smith <smithpb2250@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, vignesh C <vignesh21@gmail.com>, "Takamichi Osumi (Fujitsu)" <osumi.takamichi@fujitsu.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Amit Langote <amitlangote09@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Dilip Kumar <dilipbalaut@gmail.com>, Greg Nancarrow <gregn4422@gmail.com>
Date: 2023-03-22T06:31:50Z
Lists: pgsql-hackers
Dear Wang,

Thank you for updating patch! Following are comments form v19-0001.

01. logical-replication.sgml

I found a following statement in logical-replication.sgml. I think this may cause
mis-reading because it's OK when publishers list partitions and publish_via_root is true.

```
  <para>
   A subscriber node may have multiple subscriptions if desired.  It is
   possible to define multiple subscriptions between a single
   publisher-subscriber pair, in which case care must be taken to ensure
   that the subscribed publication objects don't overlap.
  </para>
```

How about adding "If publications are set publish_via_partition_root as true and
they publish partitions that have same partitioned table, only a change to partitioned
table is published from the publisher."or something like that?


02. filter_partitions

IIUC this function can refactor like following to avoid "skip" flag.
How do you think?

```
@@ -209,7 +209,6 @@ filter_partitions(List *table_infos)
 
        foreach(lc, table_infos)
        {
-               bool                            skip = false;
                List                       *ancestors = NIL;
                ListCell                   *lc2;
                published_rel      *table_info = (published_rel *) lfirst(lc);
@@ -224,13 +223,10 @@ filter_partitions(List *table_infos)
                        /* Is ancestor exists in the published table list? */
                        if (is_ancestor_member_tableinfos(ancestor, table_infos))
                        {
-                               skip = true;
+                               table_infos = foreach_delete_current(table_infos, lc);
                                break;
                        }
                }
-
-               if (skip)
-                       table_infos = foreach_delete_current(table_infos, lc);
        }
 }
```

03. fetch_table_list

```
+       /* Get the list of tables from the publisher. */
+       if (server_version >= 160000)
```

I think boolean variable can be used to check it like check_columnlist.
How about "use_extended_function" or something?


Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. Avoid syncing data twice for the 'publish_via_partition_root' option.

  2. Fix partition table's REPLICA IDENTITY checking on the subscriber.

  3. Prohibit combining publications with different column lists.

  4. Fix double publish of child table's data.

  5. Support adding partitioned tables to publication