Hi Shubham, here are my review comments for the TAP tests patch v27-0002 ====== Commit message
Tap tests for 'include-generated-columns' ~ But, it's more than that-- these are the TAP tests for all combinations of replication related to generated columns. i.e. both with and without 'include_generated_columns' option enabled. ====== src/test/subscription/t/011_generated.pl I was mistaken, thinking that the v27-0002 had already been refactored according to Vignesh's last review but it is not done yet, so I am not going to post detailed review comments until the restructuring is completed. ~ OTOH, there are some problems I felt have crept into v26-0001 (TAP test is same as v27-0002), so maybe try to also take care of them (see below) in v28-0002. In no particular order: * I felt it is almost useless now to have the "combo" ( "regress_pub_combo") publication. It used to have many tables when you first created it but with every version posted it is publishing less and less so now there are only 2 tables in it. Better to have a specific publication for each table now and forget about "combos" * The "TEST tab_gen_to_gen initial sync" seems to be not even checking the table data. Why not? e.g. Even if you expect no data, you should test for it. * The "TEST tab_gen_to_gen replication" seems to be not even checking the table data. Why not? * Multiple XXX comments like "... it needs more study to determine if the above result was actually correct, or a PG17 bug..." should be removed. AFAIK we should well understand the expected results for all combinations by now. * The "TEST tab_order replication" is now getting an error saying <missing replicated column: "c">, Now, that may now be the correct error for this situation, but in that case, then I think the test is not longer testing what it was intended to test (i.e. that column order does not matter....) Probably the table definition needs adjusting to make sure we are testing whenwe want to test, and not just making some random scenario "PASS". * The test "# TEST tab_alter" expected empty result also seems unhelpful. It might be related to the previous bullet. ====== Kind Regards, Peter Smith. Fujitsu Australia
