Hi, Thanks for reviewing. Please see the v8 here [1]
Takamichi Osumi (Fujitsu) <osumi.takami...@fujitsu.com>, 1 Şub 2023 Çar, 06:05 tarihinde şunu yazdı: > (1) general comment > > I wondered if the addition of the new option/parameter can introduce some > confusion to the users. > > case 1. When binary = true and copy_format = text, the table sync is > conducted by text. > case 2. When binary = false and copy_format = binary, the table sync is > conducted by binary. > (Note that the case 2 can be accepted after addressing the 3rd comment of > Bharath-san in [1]. > I agree with the 3rd comment by itself.) > I replied to Bharath's comment [1], can you please check to see if that makes sense? > The name of the new subscription parameter looks confusing. > How about "init_sync_format" or something ? > The option to enable initial sync is named "copy_data", so I named the new parameter as "copy_format" to refer to that copy meant by "copy_data". Maybe "copy_data_format" would be better. I can change it if you think it's better. > (2) The commit message doesn't get updated. > Done > (3) whitespace errors. > Done > (4) pg_dump.c > Done > (5) describe.c > Done > (6) > > + * Extract the copy format value from a DefElem. > + */ > +char > +defGetCopyFormat(DefElem *def) > > Shouldn't this function be static and remove the change of > subscriptioncmds.h ? > I wanted to make "defGetCopyFormat" be consistent with "defGetStreamingMode" since they're basically doing the same work for different parameters. And that function isn't static, so I didn't make "defGetCopyFormat" static too. > (7) catalogs.sgml > Done (8) create_subscription.sgml > Done Also; The latest patch doesn't get updated for more than two weeks > after some review comments. If you don't have time, > I would like to help updating the patch for you and other reviewers. > > Kindly let me know your status. > Sorry for the delay. This patch is currently one of my priorities. Hopefully I will share quicker updates from now on. [1] https://www.postgresql.org/message-id/CAGPVpCQYi9AYQSS%3DRmGgVNjz5ZEnLB8mACwd9aioVhLmbgiAMA%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft
