Here are some review comments for the v3-0001 test code.
======
src/test/regress/sql/subscription.sql
1.
+-- fail - utilizing streaming = parallel with time-delayed
replication is not supported
+CREATE SUBSCRIPTION regress_testsub CONNECTION
'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect =
false, streaming = parallel, min_send_delay = 123);
"utilizing" --> "specifying"
~~~
2.
+-- success -- min_send_delay value without unit is take as milliseconds
+CREATE SUBSCRIPTION regress_testsub CONNECTION
'dbname=regress_doesnotexit' PUBLICATION testpub WITH (connect =
false, min_send_delay = 123);
+\dRs+
"without unit is take as" --> "without units is taken as"
~~~
3.
+-- success -- min_send_delay value with unit is converted into ms and
stored as an integer
+ALTER SUBSCRIPTION regress_testsub SET (min_send_delay = '1 d');
+\dRs+
"with unit is converted into ms" --> "with units other than ms is
converted to ms"
~~~
4. Missing tests?
Why have the previous ALTER SUBSCRIPTION tests been removed? AFAIK,
currently, there are no regression tests for error messages like:
test_sub=# ALTER SUBSCRIPTION sub1 SET (min_send_delay = 123);
ERROR: cannot set min_send_delay for subscription in parallel streaming mode
======
src/test/subscription/t/001_rep_changes.pl
5.
+# This test is successful if and only if the LSN has been applied with at least
+# the configured apply delay.
+ok( time() - $publisher_insert_time >= $delay,
+ "subscriber applies WAL only after replication delay for
non-streaming transaction"
+);
It's not strictly an "apply delay". Maybe this comment only needs to
say like below:
SUGGESTION
# This test is successful only if at least the configured delay has elapsed.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
