On Wed, Nov 24, 2021 at 12:13 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Nov 24, 2021 at 7:51 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Thu, Nov 18, 2021 at 12:52 PM houzj.f...@fujitsu.com > > <houzj.f...@fujitsu.com> wrote: > > > > > > 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. > > > > But we wait for the subscription to catch up by using > > wait_for_catchup() with application_name, no? > > > > Yeah, but you can directly use the subscription name in > wait_for_catchup because we internally use that as > fallback_application_name. If application_name is not specified in the > connection string as suggested by Hou-San then > fallback_application_name will be considered. Both ways are okay and I > see we use both ways in the tests but it seems there are more places > where we use the method Hou-San is suggesting in subscription tests.
Okay, thanks! I referred to tests that set application_name. ISTM it's better to unite them so as not to confuse them in future tests. Anyway, I'll remove it in the next version patch that I'll submit soon. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
