Horton

Thread

Commits

  1. btree_gist: fix NaN handling in float4/float8 opclasses.

  1. BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Bill Kim <billkimjh@gmail.com> — 2026-06-28T02:14:35Z

     Hi all,

  I have a patch for this and `make check` passes (core + all contrib, plus
  new btree_gist NaN cases covering the three scenarios in the report).
  Posting here first in case someone is already further along — if not,
  I'll send it to pgsql-hackers and register it in the open CF in a couple
  of days.

  The fix swaps the five gbt_float{4,8}{gt,ge,eq,le,lt} comparators and
  gbt_float{4,8}key_cmp for the NaN-aware float{4,8}_* helpers and
  float{4,8}_cmp_internal() from utils/float.h — same total order the
  btree opclass already uses. No on-disk change.

  Bill

  2. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Daniel Gustafsson <daniel@yesql.se> — 2026-06-28T18:54:02Z

    > On 28 Jun 2026, at 04:14, Bill Kim <billkimjh@gmail.com> wrote:
> 
>  Hi all,
> 
>   I have a patch for this and `make check` passes (core + all contrib, plus
>   new btree_gist NaN cases covering the three scenarios in the report).
>   Posting here first in case someone is already further along — if not,
>   I'll send it to pgsql-hackers and register it in the open CF in a couple
>   of days.

Even if someone else is working on it, always feel free to send your patch.
Multiple implementations of a fix is not a problem but an opportunity for
everyone to learn from each other.

--
Daniel Gustafsson

  3. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Bill Kim <billkimjh@gmail.com> — 2026-06-29T12:04:52Z

      Hi,

  contrib/btree_gist's float4/float8 GiST opclasses compared keys with
  raw C operators (==, <, >). Under IEEE 754 every comparison involving
  NaN returns false, so GiST disagreed with the regular btree opclass,
  which uses float[4|8]_cmp_internal() (all NaNs equal, NaN sorts after
  every non-NaN value).

  The disagreement was reachable from SQL:

  * an index scan on a GiST index over a float column returned no rows
    for `WHERE a = 'NaN'`, while a sequential scan returned the NaN row;
  * `EXCLUDE USING gist (a WITH =)` accepted two rows whose key was NaN,
    because gbt_float8eq(NaN, NaN) was false;
  * an RLS predicate `USING (a != 'NaN')` leaked NaN rows on index scan.

  The fix swaps the five gbt_float{4,8}{gt,ge,eq,le,lt} comparators and
  the picksplit gbt_float{4,8}key_cmp for the NaN-aware
  float{4,8}_{gt,ge,eq,le,lt} / float{4,8}_cmp_internal() helpers in
  utils/float.h — same total order the btree opclass already uses. No
  on-disk format change.

  Added regression coverage in contrib/btree_gist/sql/float{4,8}.sql for
  the three scenarios above. `make check` is green: core 245/245 +
  contrib btree_gist 32/32 + all 48 contrib modules.
  Originally reported as BUG #19501 and BUG #19524:

https://www.postgresql.org/message-id/19501-3bff3bbc97f1e7c9%40postgresql.org

https://www.postgresql.org/message-id/19524-9559d302c8455664%40postgresql.org

  I used an LLM (Claude Code) to help with the analysis and to draft the
  patch. I reviewed and tested the diff myself before sending.

  Regards,
  Bill Kim

2026년 6월 29일 (월) 오전 3:54, Daniel Gustafsson <daniel@yesql.se>님이 작성:

> > On 28 Jun 2026, at 04:14, Bill Kim <billkimjh@gmail.com> wrote:
> >
> >  Hi all,
> >
> >   I have a patch for this and `make check` passes (core + all contrib,
> plus
> >   new btree_gist NaN cases covering the three scenarios in the report).
> >   Posting here first in case someone is already further along — if not,
> >   I'll send it to pgsql-hackers and register it in the open CF in a
> couple
> >   of days.
>
> Even if someone else is working on it, always feel free to send your patch.
> Multiple implementations of a fix is not a problem but an opportunity for
> everyone to learn from each other.
>
> --
> Daniel Gustafsson
>
>

  4. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-29T15:04:41Z

    Bill Kim <billkimjh@gmail.com> writes:
>   I used an LLM (Claude Code) to help with the analysis and to draft the
>   patch. I reviewed and tested the diff myself before sending.

I don't see any actual patch attached?

			regards, tom lane

  5. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Bill Kim <billkimjh@gmail.com> — 2026-06-29T15:27:21Z

    missed the file and please refer to attached.


2026년 6월 30일 (화) 오전 12:04, Tom Lane <tgl@sss.pgh.pa.us>님이 작성:

> Bill Kim <billkimjh@gmail.com> writes:
> >   I used an LLM (Claude Code) to help with the analysis and to draft the
> >   patch. I reviewed and tested the diff myself before sending.
>
> I don't see any actual patch attached?
>
>                         regards, tom lane
>

  6. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-29T15:35:10Z

    Bill Kim <billkimjh@gmail.com> writes:
> missed the file and please refer to attached.

OK, thanks.  I didn't go over this in detail yet, but at first
glance it looks reasonable.

What I am wondering right now is whether it's better to back-patch
this or leave well enough alone.  Given that we've now had two
independent complaints, maybe there are enough people using btree_gist
with NaNs to justify a back-patch, even though it'd force them to
reindex.

In any case, sneaking it into v19 seems reasonable.

			regards, tom lane

  7. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Bill Kim <billkimjh@gmail.com> — 2026-06-29T15:50:11Z

    Symptoms are silent wrong answers + EXCLUDE bypass leans me
toward back-patching despite the reindex cost.
Happy with whatever call you make.

2026년 6월 30일 (화) 오전 12:35, Tom Lane <tgl@sss.pgh.pa.us>님이 작성:

> Bill Kim <billkimjh@gmail.com> writes:
> > missed the file and please refer to attached.
>
> OK, thanks.  I didn't go over this in detail yet, but at first
> glance it looks reasonable.
>
> What I am wondering right now is whether it's better to back-patch
> this or leave well enough alone.  Given that we've now had two
> independent complaints, maybe there are enough people using btree_gist
> with NaNs to justify a back-patch, even though it'd force them to
> reindex.
>
> In any case, sneaking it into v19 seems reasonable.
>
>                         regards, tom lane
>

  8. Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-29T23:08:42Z

    I studied this and decided that it only partially fixes the problem.
Yes, we've got to fix the comparison functions, but we also have to
fix the penalty and distance functions.  As things stand, the penalty
functions will probably return NaN for any input including a NaN,
which gistpenalty will clamp to zero, which we do not want because
it'll basically break all decisions about where to put things,
leading to a seriously inefficient index.  (Maybe gistpenalty should
do something else with a NaN, but I'm hesitant to touch that right
now.)  The penalty logic is also under the misapprehension that
multiplying everything by 0.49 keeps it from having to worry about
overflows; that won't fix things for infinities.  The distance
functions will do the wrong things for NaN as well, probably breaking
any KNN search that happens across a NaN index entry.

So I fixed all that, and then was dismayed to find that the new
penalty logic wasn't reached at all in the regression tests, per code
coverage testing.  We don't compute any penalties during GiST index
build unless it's a multicolumn index, so most of the per-datatype
tests aren't reaching that.  It didn't seem like adding more index
entries partway through the test would fit very well into the
structure of float[48].sql, so instead I added simple tests of
two-column float indexes to improve the coverage.  Also, I thought
we should test this logic by adding NaN (and infinities for good
measure) to the initial data load, rather than creating a new small
table which would only exercise the logic very minimally.

At the moment I'm leaning to back-patching this.  We'd have to
release-note the need to reindex any btree_gist indexes containing
NaNs, but we've done similar things many times before.

			regards, tom lane