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