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

Wei Wang (Fujitsu) <wangw.fnst@fujitsu.com>

From: "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>
To: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.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>, vignesh C <vignesh21@gmail.com>
Date: 2022-10-17T05:52:32Z
Lists: pgsql-hackers
On Wed, Oct 5, 2022 at 23:05 PM Osumi, Takamichi/大墨 昂道 <osumi.takamichi@fujitsu.com> wrote:
> Hi, thank you for the updated patches!
> 
> 
> Here are my minor review comments for HEAD v12.

Thanks for your comments.

> (1) typo & suggestion to reword one comment
> 
> 
> +                        * Publications support partitioned tables. If
> +                        * publish_via_partition_root is false, all changes are replicated
> +                        * using leaf partition identity and schema, so we only need
> +                        * those. Otherwise, If publish_via_partition_root is true, get
> +                        * the partitioned table itself.
> 
> 
> The last sentence has "If" in the middle of the sentence.
> We can use the lower letter for it. Or, I think "Otherwise" by itself means
> "If publish_via_partition_root is true". So, I'll suggest a below change.
> 
> 
> FROM:
> Otherwise, If publish_via_partition_root is true, get the partitioned table itself.
> TO:
> Otherwise, get the partitioned table itself.

Improved.

> (2) Do we need to get "attnames" column from the publisher in the
> fetch_table_list() ?
> 
> When I was looking at v16 path, I didn't see any codes that utilize
> the "attnames" column information returned from the publisher.
> If we don't need it, could we remove it ?
> I can miss something greatly, but this might be affected by HEAD codes ?

Yes, it is affected by HEAD. I think we need this column to check whether the
same table has multiple column lists. (see commit fd0b9dc)

The new patch set were attached in [1].

[1] - https://www.postgresql.org/message-id/OS3PR01MB6275843B2BBE92870F7881C19E299%40OS3PR01MB6275.jpnprd01.prod.outlook.com

Regards,
Wang wei

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