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. ~~~ 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 Kind Regards, Peter Smith. Fujitsu Australia
