On Monday, February 28, 2022 12:57 PM Amit Kapila <amit.kapila16@gmail.com>
> On Mon, Feb 28, 2022 at 8:49 AM osumi.takamichi@fujitsu.com
> <osumi.takamichi@fujitsu.com> wrote:
> >
> > On Monday, February 28, 2022 11:34 AM Amit Kapila
> <amit.kapila16@gmail.com> wrote:
> > > On Sat, Feb 26, 2022 at 1:35 PM osumi.takamichi@fujitsu.com
> > > <osumi.takamichi@fujitsu.com> wrote:
> > > >
> > > > On Saturday, February 26, 2022 11:51 AM Amit Kapila
> > > <amit.kapila16@gmail.com> wrote:
> > > > > I have reviewed the latest version and made a few changes along
> > > > > with fixing some of the pending comments by Peter Smith. The
> > > > > changes are as
> > > > > follows: (a) Removed m_databaseid in PgStat_MsgSubscriptionError
> > > > > as that is not required now; (b) changed the struct name
> > > > > PgStat_MsgSubscriptionPurge to PgStat_MsgSubscriptionDrop to
> > > > > make it similar to DropDb; (c) changed the view name to
> > > > > pg_stat_subscription_stats, we can reconsider it in future if
> > > > > there is a consensus on some other name, accordingly changed the
> > > > > reset function name to pg_stat_reset_subscription_stats; (d)
> > > > > moved some of the newly added subscription stats functions
> > > > > adjacent to slots to main the consistency in code; (e) changed
> > > > > comments at few places;
> > > > > (f) added LATERAL back to system_views query as we refer
> > > pg_subscription's oid in the function call, previously that was not clear.
> > > > >
> > > > > Do let me know what you think of the attached?
> > > > Hi, thank you for updating the patch !
> > > > I have a couple of comments on v4.
> > > >
> > > > (1)
> > > >
> > > > I'm not sure if I'm correct, but I'd say the sync_error_count can
> > > > come next to the subname as the order of columns.
> > > > I felt there's case that the column order is somewhat related to
> > > > the time/processing order (I imagined pg_stat_replication's LSN
> > > > related columns).
> > > > If this was right, table sync related column could be the first
> > > > column as a counter within this patch.
> > > >
> > >
> > > I am not sure if there is such a correlation but even if it is there
> > > it doesn't seem to fit here completely as sync errors can happen
> > > after apply errors in multiple ways like via Alter Subscription ... Refresh ...
> > >
> > > So, I don't see the need to change the order here. What do you or others
> think?
> > In the alter subscription case, any errors after the table sync would
> > increment apply_error_count.
> >
>
> Sure, but the point I was trying to explain was that there is no certainty in the
> order of these errors.
I got it. Thank you so much for your explanation.
I don't have other new comments on this patch.
It looks good to me as well.
Best Regards,
Takamichi Osumi