Hi Shubham,
Here are my review comments for patch v24-0001
I think the TAP tests have incorrect expected results for the nogen-to-gen case.
Whereas the HEAD code will cause "ERROR" for this test scenario, patch
0001 does not. IMO the behaviour should be unchanged for this scenario
which has no generated column on the publisher side. So it seems this
is a bug in patch 0001.
FYI, I have included "FIXME" comments in the attached top-up diff
patch to show which test cases I think are expecting wrong results.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/src/test/subscription/t/011_generated.pl
b/src/test/subscription/t/011_generated.pl
index 13499a1..a9430f7 100644
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -478,10 +478,11 @@ $node_publisher->safe_psql('postgres',
# regress_sub1_combo_nogen_to_gen: (include_generated_columns = false)
#
-# XXX
-# The test below shows that current PG17 behavior does not give an error,
-# But this conflicts with the copy_data=true behavior so it might be a PG17
bug.
-# Needs more study.
+# FIXME
+# I think the following expected result is wrong. IIUC it should give
+# the same error as HEAD -- e.g. something like:
+# ERROR: logical replication target relation "public.tab_nogen_to_gen" is
missing
+# replicated column: "b"
$node_publisher->wait_for_catchup('regress_sub1_combo_nogen_to_gen_nocopy');
$result =
$node_subscriber->safe_psql('postgres',
@@ -495,9 +496,11 @@ is( $result, qq(4|88
# When copy_data=false, no COPY error occurs.
# The col 'b' is not replicated; the subscriber-side generated value is
inserted.
#
-# XXX
-# It is correct for this to give the same result as above, but it needs more
-# study to determine if the above result was actually correct, or a PG17 bug.
+# FIXME
+# Since there is no generated column on the publishing side this should give
+# the same result as the previous test. -- e.g. something like:
+# ERROR: logical replication target relation "public.tab_nogen_to_gen" is
missing
+# replicated column: "b"
$node_publisher->wait_for_catchup('regress_sub2_combo_nogen_to_gen');
$result =
$node_subscriber2->safe_psql('postgres',
