public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bill Kim <[email protected]>
To: Tom Lane <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: [email protected]
Subject: Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses
Date: Tue, 30 Jun 2026 00:27:21 +0900
Message-ID: <CAMQXxch6yrJQdeHQK9Mypa9cmDKjQOc5DJ=SL72pom1djL_OLg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAMQXxcgbtD2LXfX0tpgvOizxP-XxrCHV2ZDy4By_TZnJMsxXWQ@mail.gmail.com>
	<[email protected]>
	<CAMQXxch1N=sZ1fsfx57VhTe=8qpmb1K1vnaB5WVyDbAs8bqExg@mail.gmail.com>
	<[email protected]>

missed the file and please refer to attached.


2026년 6월 30일 (화) 오전 12:04, Tom Lane <[email protected]>님이 작성:

> Bill Kim <[email protected]> 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
>


Attachments:

  [application/octet-stream] 0001-btree_gist-respect-NaN-ordering-in-float4-float8-opc.patch (12.7K, ../CAMQXxch6yrJQdeHQK9Mypa9cmDKjQOc5DJ=SL72pom1djL_OLg@mail.gmail.com/3-0001-btree_gist-respect-NaN-ordering-in-float4-float8-opc.patch)
  download | inline diff:
From 80d760d268b0e1a72d2a1260ebc81c0876687384 Mon Sep 17 00:00:00 2001
From: Bill Kim <[email protected]>
Date: Sun, 28 Jun 2026 00:19:30 +0900
Subject: [PATCH] btree_gist: respect NaN ordering in float4/float8 opclasses

The float4 and float8 btree_gist opclasses compared keys with raw C
operators (==, <, >).  IEEE 754 makes every comparison involving NaN
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.

Replace the five boolean comparators with float[4|8]_{gt,ge,eq,le,lt}
from utils/float.h, and the picksplit comparator with
float[4|8]_cmp_internal().  No on-disk format change.

Add regression coverage for the three scenarios above.

Reported-by: Man Zeng <[email protected]>
Reported-by: Yuelin Wang <[email protected]>
Discussion: https://www.postgresql.org/message-id/19501-3bff3bbc97f1e7c9%40postgresql.org
Discussion: https://www.postgresql.org/message-id/19524-9559d302c8455664%40postgresql.org
---
 contrib/btree_gist/btree_float4.c      | 31 +++++++++-------
 contrib/btree_gist/btree_float8.c      | 31 +++++++++-------
 contrib/btree_gist/expected/float4.out | 51 ++++++++++++++++++++++++++
 contrib/btree_gist/expected/float8.out | 51 ++++++++++++++++++++++++++
 contrib/btree_gist/sql/float4.sql      | 29 +++++++++++++++
 contrib/btree_gist/sql/float8.sql      | 29 +++++++++++++++
 6 files changed, 196 insertions(+), 26 deletions(-)

diff --git a/contrib/btree_gist/btree_float4.c b/contrib/btree_gist/btree_float4.c
index c076918..083c616 100644
--- a/contrib/btree_gist/btree_float4.c
+++ b/contrib/btree_gist/btree_float4.c
@@ -26,30 +26,38 @@ PG_FUNCTION_INFO_V1(gbt_float4_penalty);
 PG_FUNCTION_INFO_V1(gbt_float4_same);
 PG_FUNCTION_INFO_V1(gbt_float4_sortsupport);
 
+/*
+ * Use the NaN-aware comparators from utils/float.h.  Raw C operators treat
+ * NaN as unordered, which makes the GiST consistent / picksplit logic
+ * disagree with the regular btree opclass: GiST would then miss NaN rows on
+ * index scans, let EXCLUDE constraints accept duplicate NaNs, etc.  The
+ * helpers below match btree's total order (all NaNs equal, NaN sorts after
+ * every non-NaN value).
+ */
 static bool
 gbt_float4gt(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float4 *) a) > *((const float4 *) b));
+	return float4_gt(*((const float4 *) a), *((const float4 *) b));
 }
 static bool
 gbt_float4ge(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float4 *) a) >= *((const float4 *) b));
+	return float4_ge(*((const float4 *) a), *((const float4 *) b));
 }
 static bool
 gbt_float4eq(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float4 *) a) == *((const float4 *) b));
+	return float4_eq(*((const float4 *) a), *((const float4 *) b));
 }
 static bool
 gbt_float4le(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float4 *) a) <= *((const float4 *) b));
+	return float4_le(*((const float4 *) a), *((const float4 *) b));
 }
 static bool
 gbt_float4lt(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float4 *) a) < *((const float4 *) b));
+	return float4_lt(*((const float4 *) a), *((const float4 *) b));
 }
 
 static int
@@ -57,16 +65,13 @@ gbt_float4key_cmp(const void *a, const void *b, FmgrInfo *flinfo)
 {
 	float4KEY  *ia = (float4KEY *) (((const Nsrt *) a)->t);
 	float4KEY  *ib = (float4KEY *) (((const Nsrt *) b)->t);
+	int			res;
 
-	if (ia->lower == ib->lower)
-	{
-		if (ia->upper == ib->upper)
-			return 0;
-
-		return (ia->upper > ib->upper) ? 1 : -1;
-	}
+	res = float4_cmp_internal(ia->lower, ib->lower);
+	if (res != 0)
+		return res;
 
-	return (ia->lower > ib->lower) ? 1 : -1;
+	return float4_cmp_internal(ia->upper, ib->upper);
 }
 
 static float8
diff --git a/contrib/btree_gist/btree_float8.c b/contrib/btree_gist/btree_float8.c
index d7386e8..5c80948 100644
--- a/contrib/btree_gist/btree_float8.c
+++ b/contrib/btree_gist/btree_float8.c
@@ -27,30 +27,38 @@ PG_FUNCTION_INFO_V1(gbt_float8_same);
 PG_FUNCTION_INFO_V1(gbt_float8_sortsupport);
 
 
+/*
+ * Use the NaN-aware comparators from utils/float.h.  Raw C operators treat
+ * NaN as unordered, which makes the GiST consistent / picksplit logic
+ * disagree with the regular btree opclass: GiST would then miss NaN rows on
+ * index scans, let EXCLUDE constraints accept duplicate NaNs, etc.  The
+ * helpers below match btree's total order (all NaNs equal, NaN sorts after
+ * every non-NaN value).
+ */
 static bool
 gbt_float8gt(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float8 *) a) > *((const float8 *) b));
+	return float8_gt(*((const float8 *) a), *((const float8 *) b));
 }
 static bool
 gbt_float8ge(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float8 *) a) >= *((const float8 *) b));
+	return float8_ge(*((const float8 *) a), *((const float8 *) b));
 }
 static bool
 gbt_float8eq(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float8 *) a) == *((const float8 *) b));
+	return float8_eq(*((const float8 *) a), *((const float8 *) b));
 }
 static bool
 gbt_float8le(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float8 *) a) <= *((const float8 *) b));
+	return float8_le(*((const float8 *) a), *((const float8 *) b));
 }
 static bool
 gbt_float8lt(const void *a, const void *b, FmgrInfo *flinfo)
 {
-	return (*((const float8 *) a) < *((const float8 *) b));
+	return float8_lt(*((const float8 *) a), *((const float8 *) b));
 }
 
 static int
@@ -58,16 +66,13 @@ gbt_float8key_cmp(const void *a, const void *b, FmgrInfo *flinfo)
 {
 	float8KEY  *ia = (float8KEY *) (((const Nsrt *) a)->t);
 	float8KEY  *ib = (float8KEY *) (((const Nsrt *) b)->t);
+	int			res;
 
-	if (ia->lower == ib->lower)
-	{
-		if (ia->upper == ib->upper)
-			return 0;
-
-		return (ia->upper > ib->upper) ? 1 : -1;
-	}
+	res = float8_cmp_internal(ia->lower, ib->lower);
+	if (res != 0)
+		return res;
 
-	return (ia->lower > ib->lower) ? 1 : -1;
+	return float8_cmp_internal(ia->upper, ib->upper);
 }
 
 static float8
diff --git a/contrib/btree_gist/expected/float4.out b/contrib/btree_gist/expected/float4.out
index dfe7320..3109ca3 100644
--- a/contrib/btree_gist/expected/float4.out
+++ b/contrib/btree_gist/expected/float4.out
@@ -89,3 +89,54 @@ SELECT a, a <-> '-179.0' FROM float4tmp ORDER BY a <-> '-179.0' LIMIT 3;
  -158.17741 | 20.822586
 (3 rows)
 
+-- NaN handling: GiST should agree with the regular btree opclass
+-- (NaN equals NaN, NaN sorts after any non-NaN value).
+CREATE TABLE float4nan (a float4);
+INSERT INTO float4nan VALUES (1.0), (2.0), ('NaN'), (3.0);
+CREATE INDEX float4nan_idx ON float4nan USING gist (a);
+SET enable_seqscan = on;
+SET enable_indexscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM float4nan WHERE a = 'NaN'::float4;
+ count 
+-------
+     1
+(1 row)
+
+SELECT count(*) FROM float4nan WHERE a > 1.0::float4;
+ count 
+-------
+     3
+(1 row)
+
+SET enable_seqscan = off;
+SET enable_indexscan = on;
+SET enable_bitmapscan = on;
+SELECT count(*) FROM float4nan WHERE a = 'NaN'::float4;
+ count 
+-------
+     1
+(1 row)
+
+SELECT count(*) FROM float4nan WHERE a > 1.0::float4;
+ count 
+-------
+     3
+(1 row)
+
+-- EXCLUDE constraint must block a duplicate NaN, same as it does for finite
+-- values.
+CREATE TABLE float4excl (a float4, EXCLUDE USING gist (a WITH =));
+INSERT INTO float4excl VALUES ('NaN'::float4);
+INSERT INTO float4excl VALUES ('NaN'::float4);  -- expect: violates EXCLUDE
+ERROR:  conflicting key value violates exclusion constraint "float4excl_a_excl"
+DETAIL:  Key (a)=(NaN) conflicts with existing key (a)=(NaN).
+SELECT count(*) FROM float4excl;
+ count 
+-------
+     1
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_indexscan;
+RESET enable_bitmapscan;
diff --git a/contrib/btree_gist/expected/float8.out b/contrib/btree_gist/expected/float8.out
index ebd0ef3..02a616e 100644
--- a/contrib/btree_gist/expected/float8.out
+++ b/contrib/btree_gist/expected/float8.out
@@ -89,3 +89,54 @@ SELECT a, a <-> '-1890.0' FROM float8tmp ORDER BY a <-> '-1890.0' LIMIT 3;
   -1769.73634 | 120.26366000000007
 (3 rows)
 
+-- NaN handling: GiST should agree with the regular btree opclass
+-- (NaN equals NaN, NaN sorts after any non-NaN value).
+CREATE TABLE float8nan (a float8);
+INSERT INTO float8nan VALUES (1.0), (2.0), ('NaN'), (3.0);
+CREATE INDEX float8nan_idx ON float8nan USING gist (a);
+SET enable_seqscan = on;
+SET enable_indexscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM float8nan WHERE a = 'NaN'::float8;
+ count 
+-------
+     1
+(1 row)
+
+SELECT count(*) FROM float8nan WHERE a > 1.0::float8;
+ count 
+-------
+     3
+(1 row)
+
+SET enable_seqscan = off;
+SET enable_indexscan = on;
+SET enable_bitmapscan = on;
+SELECT count(*) FROM float8nan WHERE a = 'NaN'::float8;
+ count 
+-------
+     1
+(1 row)
+
+SELECT count(*) FROM float8nan WHERE a > 1.0::float8;
+ count 
+-------
+     3
+(1 row)
+
+-- EXCLUDE constraint must block a duplicate NaN, same as it does for finite
+-- values.
+CREATE TABLE float8excl (a float8, EXCLUDE USING gist (a WITH =));
+INSERT INTO float8excl VALUES ('NaN'::float8);
+INSERT INTO float8excl VALUES ('NaN'::float8);  -- expect: violates EXCLUDE
+ERROR:  conflicting key value violates exclusion constraint "float8excl_a_excl"
+DETAIL:  Key (a)=(NaN) conflicts with existing key (a)=(NaN).
+SELECT count(*) FROM float8excl;
+ count 
+-------
+     1
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_indexscan;
+RESET enable_bitmapscan;
diff --git a/contrib/btree_gist/sql/float4.sql b/contrib/btree_gist/sql/float4.sql
index 3da1ce9..7c95e7f 100644
--- a/contrib/btree_gist/sql/float4.sql
+++ b/contrib/btree_gist/sql/float4.sql
@@ -35,3 +35,32 @@ SELECT count(*) FROM float4tmp WHERE a >  -179.0::float4;
 EXPLAIN (COSTS OFF)
 SELECT a, a <-> '-179.0' FROM float4tmp ORDER BY a <-> '-179.0' LIMIT 3;
 SELECT a, a <-> '-179.0' FROM float4tmp ORDER BY a <-> '-179.0' LIMIT 3;
+
+-- NaN handling: GiST should agree with the regular btree opclass
+-- (NaN equals NaN, NaN sorts after any non-NaN value).
+CREATE TABLE float4nan (a float4);
+INSERT INTO float4nan VALUES (1.0), (2.0), ('NaN'), (3.0);
+CREATE INDEX float4nan_idx ON float4nan USING gist (a);
+
+SET enable_seqscan = on;
+SET enable_indexscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM float4nan WHERE a = 'NaN'::float4;
+SELECT count(*) FROM float4nan WHERE a > 1.0::float4;
+
+SET enable_seqscan = off;
+SET enable_indexscan = on;
+SET enable_bitmapscan = on;
+SELECT count(*) FROM float4nan WHERE a = 'NaN'::float4;
+SELECT count(*) FROM float4nan WHERE a > 1.0::float4;
+
+-- EXCLUDE constraint must block a duplicate NaN, same as it does for finite
+-- values.
+CREATE TABLE float4excl (a float4, EXCLUDE USING gist (a WITH =));
+INSERT INTO float4excl VALUES ('NaN'::float4);
+INSERT INTO float4excl VALUES ('NaN'::float4);  -- expect: violates EXCLUDE
+SELECT count(*) FROM float4excl;
+
+RESET enable_seqscan;
+RESET enable_indexscan;
+RESET enable_bitmapscan;
diff --git a/contrib/btree_gist/sql/float8.sql b/contrib/btree_gist/sql/float8.sql
index e1e819b..eeed36b 100644
--- a/contrib/btree_gist/sql/float8.sql
+++ b/contrib/btree_gist/sql/float8.sql
@@ -35,3 +35,32 @@ SELECT count(*) FROM float8tmp WHERE a >  -1890.0::float8;
 EXPLAIN (COSTS OFF)
 SELECT a, a <-> '-1890.0' FROM float8tmp ORDER BY a <-> '-1890.0' LIMIT 3;
 SELECT a, a <-> '-1890.0' FROM float8tmp ORDER BY a <-> '-1890.0' LIMIT 3;
+
+-- NaN handling: GiST should agree with the regular btree opclass
+-- (NaN equals NaN, NaN sorts after any non-NaN value).
+CREATE TABLE float8nan (a float8);
+INSERT INTO float8nan VALUES (1.0), (2.0), ('NaN'), (3.0);
+CREATE INDEX float8nan_idx ON float8nan USING gist (a);
+
+SET enable_seqscan = on;
+SET enable_indexscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM float8nan WHERE a = 'NaN'::float8;
+SELECT count(*) FROM float8nan WHERE a > 1.0::float8;
+
+SET enable_seqscan = off;
+SET enable_indexscan = on;
+SET enable_bitmapscan = on;
+SELECT count(*) FROM float8nan WHERE a = 'NaN'::float8;
+SELECT count(*) FROM float8nan WHERE a > 1.0::float8;
+
+-- EXCLUDE constraint must block a duplicate NaN, same as it does for finite
+-- values.
+CREATE TABLE float8excl (a float8, EXCLUDE USING gist (a WITH =));
+INSERT INTO float8excl VALUES ('NaN'::float8);
+INSERT INTO float8excl VALUES ('NaN'::float8);  -- expect: violates EXCLUDE
+SELECT count(*) FROM float8excl;
+
+RESET enable_seqscan;
+RESET enable_indexscan;
+RESET enable_bitmapscan;
-- 
2.52.0



view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses
  In-Reply-To: <CAMQXxch6yrJQdeHQK9Mypa9cmDKjQOc5DJ=SL72pom1djL_OLg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox