On Tuesday, November 16, 2021 2:31 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > Right. I've fixed this issue and attached an updated patch. > Hi,
I have few comments for the testcases. 1) +my $appname = 'tap_sub'; +$node_subscriber->safe_psql( + 'postgres', + "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (streaming = off, two_phase = on);"); +my $appname_streaming = 'tap_sub_streaming'; +$node_subscriber->safe_psql( + 'postgres', + "CREATE SUBSCRIPTION tap_sub_streaming CONNECTION '$publisher_connstr application_name=$appname_streaming' PUBLICATION tap_pub_streaming WITH (streaming = on, two_phase = on);"); + I think we can remove the 'application_name=$appname', so that the command could be shorter. 2) +...(streaming = on, two_phase = on);"); Besides, is there some reasons to set two_phase to ? If so, It might be better to add some comments about it. 3) +CREATE PUBLICATION tap_pub_streaming FOR TABLE test_tab_streaming; +]); + It seems there's no tests to use the table test_tab_streaming. I guess this table is used to test streaming change error, maybe we can add some tests for it ? Best regards, Hou zj
