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