Thread: Re: More reliable nbtree detection of unsatisfiable RowCompare quals involving a leading NULL key/element

On Fri, 27 Dec 2024 at 23:03, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Mon, Dec 23, 2024 at 1:02 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > Attached patch fixes the problem by moving detection of RowCompare
> > unsatisfiable-due-to-NULL cases into _bt_fix_scankey_strategy.
>
> Attached is v2, which adds several new regression tests, giving
> certain relevant nbtree code paths test coverage.
>
> v2 also makes some small tweaks to _bt_check_rowcompare(), the actual
> comparator used by scans with a RowCompare qual. We no longer need to
> account for the possibility that the first row member from a
> RowCompare scan key contains a null once the scan is underway (we know
> that _bt_preprocess_keys would have recognized the qual as
> unsatisfiable had the RowCompare looked like that).

backend/access/nbtree/nbtsearch.c:_bt_first
> +             * Cannot be a NULL in the first row member (that would make the
> +             * qual unsatisfiable, preventing it from ever getting this far)

This doesn't really clarify _why_ we'd never get this far, so I'd word that as

+             * Cannot be a NULL in the first row member: _bt_preprocess_keys
+             * would've marked the qual as unsatisfyable, preventing us from
+             * ever getting this far.

Apart from that minor issue, LGTM.

Kind regards,

Matthias van de Meent

On Tue, Jan 7, 2025 at 7:58 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> This doesn't really clarify _why_ we'd never get this far, so I'd word that as
>
> +             * Cannot be a NULL in the first row member: _bt_preprocess_keys
> +             * would've marked the qual as unsatisfyable, preventing us from
> +             * ever getting this far.
>
> Apart from that minor issue, LGTM.

Pushed this just now. I used your suggested wording in the committed patch.

Thanks for the review!

--
Peter Geoghegan