Re: Bug in nbtree optimization to skip > operator comparisons (or < comparisons in backwards scans)

Alexander,

On Tue, 26 Dec 2023 at 23:35, Alexander Korotkov <aekorotkov(at)gmail(dot)com>
wrote:

> Pavel,
>
> On Mon, Dec 25, 2023 at 8:32 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>
> wrote:
> > I've reviewed both patches:
> > 0001 - is a pure refactoring replacing argument transfer from via struct
> member to transfer explicitly as a function argument. It's justified by the
> fact firstPage is localized only to several places. The patch looks simple
> and good enough.
> >
> > 0002:
> > continuescanPrechecked is semantically much better than previous
> requiredMatchedByPrecheck which confused me earlier. Thanks!
> >
> > From the new comments, it looks a little bit hard to understand who does
> what. Semantics "if caller told" in comments looks more clear to me. Could
> you especially give attention to the comments:
> >
> > "If they wouldn't be matched, then the *continuescan flag would be set
> for the current item and the last item on the page accordingly."
> > "If the key is required for the opposite direction scan, we need to know
> there was already at least one matching item on the page. For those keys."
> >
> > > Prechecking the value of the continuescan flag for the last item on the
> > >+ * page (according to the scan direction).
> > Maybe, in this case, it would be more clear like: "...(for backwards
> scan it will be the first item on a page)"
> >
> > Otherwise the patch 0002 looks like a good fix for the bug to be pushed.
>
> Thank you for your review. I've revised comments to meet your suggestions.
>
Thank you for revised comments! I think they are good enough.

Regards,
Pavel