RE: Data is copied twice when specifying both child and parent table in publication - Mailing list 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