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
