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
-
Avoid syncing data twice for the 'publish_via_partition_root' option.
- 062a84442424 16.0 landed
-
Fix partition table's REPLICA IDENTITY checking on the subscriber.
- 52d5ea9adb01 14.5 cited
-
Prohibit combining publications with different column lists.
- fd0b9dcebda7 15.0 cited
-
Fix double publish of child table's data.
- 3f06c00cf6dc 13.6 landed
- 614b77d65a38 14.2 landed
- 5e97905a2c76 15.0 landed
-
Support adding partitioned tables to publication
- 17b9e7f9fe23 13.0 cited