On Wed, Oct 5, 2022 at 23:05 PM Osumi, Takamichi/大墨 昂道 <osumi.takami...@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
