Re: display offset along with block number in vacuum errors

On Thu, 20 Aug 2020 at 21:12, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Aug 20, 2020 at 12:32 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Aug 20, 2020 at 12:18 PM Masahiko Sawada
> > <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > >
> > > On Thu, 20 Aug 2020 at 14:01, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > >
> > > > On Wed, Aug 19, 2020 at 12:54 PM Masahiko Sawada
> > > > <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > > >
> > > > Here, we can notice that for the index, we are getting context
> > > > information but not for the heap. The reason is that in
> > > > vacuum_error_callback, we are not printing additional information for
> > > > phases VACUUM_ERRCB_PHASE_SCAN_HEAP and VACUUM_ERRCB_PHASE_VACUUM_HEAP
> > > > when block number is invalid. If we want to cover the 'info' messages
> > > > then won't it be better if we print a message in those phases even
> > > > block number is invalid (something like 'while scanning relation
> > > > \"%s.%s\"")
> > >
> > > Yeah, there is an inconsistency. I agree to print the message even
> > > when the block number is invalid.
> > >
> >
> > Okay, I will update this and send this patch and rebased patch to
> > display offsets later today or tomorrow.
> >
>
> Attached are both the patches. The first one is to improve existing
> error context information, so I think we should back-patch to 13. The
> second one is to add additional vacuum error context information, so
> that is for only HEAD. Does that make sense? Also, let me know if you
> have any more comments.

Yes, makes sense to me.

I don't have comments on both patches. They look good to me.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services