On Friday, July 30, 2021 12:02 PM Peter Smith <smithpb2250@gmail.com>wrote:
>
> Please find attached the latest patch set v100*
>
> v99-0002 --> v100-0001
>
Thanks for your patch. A few comments on the test file:
1. src/test/subscription/t/022_twophase_cascade.pl
1.1
I saw your test cases for "PREPARE / COMMIT PREPARED" and "PREPARE with a nested ROLLBACK TO SAVEPOINT", but didn't see
casesfor "PREPARE / ROLLBACK PREPARED". Is it needless or just missing?
1.2
+# check inserts are visible at subscriber(s).
+# All the streamed data (prior to the SAVEPOINT) should be rolled back.
+# (3, 'foobar') should be committed.
I think it should be (9999, 'foobar') here.
1.3
+$result = $node_B->safe_psql('postgres', "SELECT count(*) FROM test_tab where b = 'foobar';");
+is($result, qq(1), 'Rows committed are present on subscriber B');
+$result = $node_B->safe_psql('postgres', "SELECT count(*) FROM test_tab;");
+
It seems the test is not finished yet. We didn't check the value of 'result'. Besides, maybe we should also check
node_C,right?
1.4
+$node_B->append_conf('postgresql.conf', qq(max_prepared_transactions = 10));
+$node_B->append_conf('postgresql.conf', qq(logical_decoding_work_mem = 64kB));
You see, the first line uses a TAB but the second line uses a space.
Also, we could use only one statement to append these two settings to run tests a bit faster. Thoughts?
Something like:
$node_B->append_conf(
'postgresql.conf', qq(
max_prepared_transactions = 10
logical_decoding_work_mem = 64kB
));
Regards
Tang
