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

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

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:

/messages/by-id/19501-3bff3bbc97f1e7c9@postgresql.org

/messages/by-id/19524-9559d302c8455664@postgresql.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>님이 작성:

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

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:

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

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>님이 작성:

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

Thanks.
v2 applies cleanly on master and contrib/btree_gist regression passes here.
Nothing to add.

regards,
Bill

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