Re: Minor _bt_checkkeys comment improvements - Mailing list pgsql-hackers

From Peter Geoghegan
Subject Re: Minor _bt_checkkeys comment improvements
Date
Msg-id CAH2-Wznvj74CAwVydAs_Lh1hR3PGi_aN1s7BiRKE0iJYNdoixQ@mail.gmail.com
Whole thread Raw
In response to Minor _bt_checkkeys comment improvements  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
On Thu, Jan 8, 2026 at 6:27 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> I noticed a trivial misspelling in the comment in
> _bt_tuple_before_array_skeys, and when I started to look at the
> surrounding comments, I noticed a few other things that could be
> improved. Patch attached, but let me go through the changes one by one:

I'm fine with all these changes.

> Some other functions take a "bool *continuescan" parameter, but this
> function sets it in 'pstate'.

FWIW this is an obsolete comment -- it used to have a continuescan parameter.

> > @@ -1197,9 +1197,9 @@ _bt_checkkeys(IndexScanDesc scan, BTReadPageState *pstate, bool arrayKeys,
> >
> >         /*
> >          * Only one _bt_check_compare call is required in the common case where
> > -        * there are no equality strategy array scan keys.  Otherwise we can only
> > -        * accept _bt_check_compare's answer unreservedly when it didn't set
> > -        * pstate.continuescan=false.
> > +        * there are no equality strategy array scan keys.  With array keys, we
> > +        * can only accept _bt_check_compare's answer unreservedly when it set
> > +        * pstate.continuescan=true.
> >          */
> >         if (!arrayKeys || pstate->continuescan)
> >                 return res;
>
> Double negative: "didn't set to false" means "did set to true" in this
> context. And the sentence was a little difficult to parse in other ways
> too: it takes some effort to decipher that "otherwise" means that there
> are array keys, and "only accept unreservedly" sound like more negation.
> (I didn't change the "only accept unreservedly" part)
>
> Are there other kind of array keys than "equality strategy array keys"?

Yes. There are inequality array keys.

Preprocessing will replace the scan key's sk_argument with the array's
extremal key, so there isn't an array left behind after preprocessing.
But there is still a SK_SEARCHARRAY marking.

Maybe we should have preprocessing unset SK_SEARCHARRAY for such an
inequality key, so that there really wouldn't be such a thing as an
array inequality. That would allow us to stop testing each
SK_SEARCHARRAY key's strategy in code like _bt_advance_array_keys.

--
Peter Geoghegan



pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Implement waiting for wal lsn replay: reloaded
Next
From: Bryan Green
Date:
Subject: Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows