Re: Optimizing nbtree ScalarArrayOp execution, allowing multi-column ordered scans, skip scan

Поиск
Список
Период
Сортировка
От Peter Geoghegan
Тема Re: Optimizing nbtree ScalarArrayOp execution, allowing multi-column ordered scans, skip scan
Дата
Msg-id CAH2-Wz=xEygVvGoBNi-wecCF6J_azgS4fh9wUkDF7K83Q0nxSw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Optimizing nbtree ScalarArrayOp execution, allowing multi-column ordered scans, skip scan  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Optimizing nbtree ScalarArrayOp execution, allowing multi-column ordered scans, skip scan  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
On Sun, Apr 7, 2024 at 8:48 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Coverity pointed out something that looks like a potentially live
> problem in 5bf748b86:
>
> /srv/coverity/git/pgsql-git/postgresql/src/backend/access/nbtree/nbtutils.c: 2950 in _bt_preprocess_keys()
> 2944                              * need to make sure that we don't throw away an array
> 2945                              * scan key.  _bt_compare_scankey_args expects us to
> 2946                              * always keep arrays (and discard non-arrays).
> 2947                              */
> 2948                             Assert(j == (BTEqualStrategyNumber - 1));
> 2949                             Assert(xform[j].skey->sk_flags & SK_SEARCHARRAY);
> >>>     CID 1596256:  Null pointer dereferences  (FORWARD_NULL)
> >>>     Dereferencing null pointer "array".
> 2950                             Assert(xform[j].ikey == array->scan_key);
> 2951                             Assert(!(cur->sk_flags & SK_SEARCHARRAY));
> 2952                         }
> 2953                     }
> 2954                     else if (j == (BTEqualStrategyNumber - 1))
>
> Above this there is an assertion
>
>                     Assert(!array || array->num_elems > 0);
>
> which certainly makes it look like array->scan_key could be
> a null-pointer dereference.

But the "Assert(xform[j].ikey == array->scan_key)" assertion is
located in a block where it's been established that the scan key (the
one stored in xform[j] at this point in execution) must have an array.
It has been marked SK_SEARCHARRAY, and uses the equality strategy, so
it had better have one or we're in big trouble either way.

This is probably very hard for tools like Coverity to understand. We
also rely on the fact that only one of the two scan keys (only one of
the pair of scan keys that were passed to _bt_compare_scankey_args)
can have an array at the point of the assertion that Coverity finds
suspicious. It's possible that both of those scan keys actually did
have arrays, but _bt_compare_scankey_args just treats that as a case
of being unable to prove which scan key was redundant/contradictory
due to a lack of suitable cross-type support -- so the assertion won't
be reached.

Would Coverity stop complaining if I just removed the assertion? I
could just do that, I suppose, but that seems backwards to me.

--
Peter Geoghegan



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: [MASSMAIL]Coverity complains about simplehash.h's SH_STAT()
Следующее
От: Thomas Munro
Дата:
Сообщение: Re: Use streaming read API in ANALYZE