On Thu, 21 Sept 2023 at 15:17, Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Wed, Sep 20, 2023 at 5:07 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Tue, Sep 19, 2023 at 1:48 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > This also makes sense. I've rephrased the comment.
>
> The revised patch is attached. It contains better comments and the
> commit message. Peter, could you please check if you're OK with this?
Hi, Alexander!
I looked at the patch code and I agree with this optimization.
Implementation also looks good to me except change :
+ if (key->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD) &&
+ !(key->sk_flags & SK_ROW_HEADER))
+ requiredDir = true;
...
- if ((key->sk_flags & SK_BT_REQFWD) &&
- ScanDirectionIsForward(dir))
- *continuescan = false;
- else if ((key->sk_flags & SK_BT_REQBKWD) &&
- ScanDirectionIsBackward(dir))
+ if (requiredDir)
*continuescan = false;
looks like changing behavior in the case when key->sk_flags &
SK_BT_REQFWD && (! ScanDirectionIsForward(dir)) &&
(!requiredDirMatched)
Originally it doesn't set *continuescan = false; and with the patch it will set.
This may be relevant for the first page when requiredDirMatched is
intentionally skipped to be set and for call
_bt_checkkeys(scan, itup, truncatt, dir, &continuescan, false);
Maybe I missed something and this can not appear for some reason?
Also naming of requiredDirMatched and requiredDir seems semantically
hard to understand the meaning without looking at the patch commit
message. But I don't have better proposals yet, so maybe it's
acceptable.
Kind regards,
Pavel Borisov
Supabase.
