On Thu, Nov 25, 2021 at 10:06 PM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: > > On Thur, Nov 25, 2021 8:29 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Thu, Nov 25, 2021 at 1:57 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Wed, Nov 24, 2021 at 5:14 PM Masahiko Sawada > > <sawada.m...@gmail.com> wrote: > > > > > > > > Changed. I've removed first_error_time as per discussion on the > > > > thread for adding xact stats. > > > > > > > > > > We also agreed to change the column names to start with last_error_* > > > [1]. Is there a reason to not make those changes? Do you think that we > > > can change it just before committing that patch? I thought it might be > > > better to do it that way now itself. > > > > Oh, I thought that you think that we change the column names when adding > > xact > > stats to the view. But these names also make sense even without the xact > > stats. > > I've attached an updated patch. It also incorporated comments from Vignesh > > and Greg. > > > Hi, > > I only noticed some minor things in the testcases > > 1) > +$node_publisher->append_conf('postgresql.conf', > + qq[ > +logical_decoding_work_mem = 64kB > +]); > > It seems we don’t need set the decode_work_mem since we don't test streaming ? > > 2) > +$node_publisher->safe_psql('postgres', > + q[ > +CREATE PUBLICATION tap_pub FOR TABLE test_tab1, test_tab2; > +]); > > There are a few places where only one command exists in the 'q[' or 'qq[' > like the above code. > To be consistent, I think it might be better to remove the wrap here, maybe > we can write like: > $node_publisher->safe_psql('postgres', > ' CREATE PUBLICATION tap_pub FOR TABLE test_tab1, test_tab2;'); >
Indeed. Attached an updated patch. Thanks! Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
v26-0001-Add-a-subscription-worker-statistics-view-pg_sta.patch
Description: Binary data