On Fri, Oct 4, 2024 at 9:43 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Hi Shubham, I don't have any new comments for the patch v36-0002. > > But, according to my records, there are multiple old comments not yet > addressed for this patch. I am giving reminders for those below so > they don't get accidentally overlooked. Please re-confirm and at the > next posted version please respond individually to each of these to > say if they are addressed or not. > > ====== > > 1. General > From review v31 [1] comment #1. Patches 0001 and 0002 should be merged. > > ====== > src/backend/replication/logical/tablesync.c > > make_copy_attnamelist: > > 2. > From review v31 [1] comment #4. Make the detailed useful error message > common if possible. > > ~~~
This comment is still open. Will fix this in next versions of patches. > > fetch_remote_table_info: > > 3. > From review v31 [1] comment #5. I was not sure if this logic is > sophisticated enough to handle the case when the same table has > gencols but there are multiple subscribed publications and the > 'publish_generated_columns' parameter differs. Is this scenario > tested? > > ~ > > 4. > + * Get column lists for each relation, and check if any of the > + * publications have the 'publish_generated_columns' parameter enabled. > > From review v32 [2] comment #1. This needs some careful testing. I was > not sure if sufficient to just check the 'publish_generated_columns' > flag. Now that "column lists take precedence" it is quite possible for > all publications to say 'publish_generated_columns=false', but the > publication can still publish gencols *anyway* if they are specified > in a column list. > > ====== > [1] review v31 18/9 - > https://www.postgresql.org/message-id/flat/CAHv8Rj%2BKOoh58Uf5k2MN-%3DA3VdV60kCVKCh5ftqYxgkdxFSkqg%40mail.gmail.com#f2f3b48080f96ea45e1410f5b1cd9735 > [2] review v32 24/9 - > https://www.postgresql.org/message-id/CAHut%2BPu7EcK_JTgWS7GzeStHk6Asb1dmEzCJU2TJf%2BW1Zy30LQ%40mail.gmail.com > I have fixed the comments and posted the v37 patches for them. Please refer to the updated v37 Patches here in [1]. See [1] for the changes added. [1] https://www.postgresql.org/message-id/CAHv8Rj%2BRnw%2B_SfSyyrvWL49AfJzx4O8YVvdU9gB%2BSQdt3%3DqF%2BA%40mail.gmail.com Thanks and Regards, Shubham Khanna.
