From bf6c59f4322aef22bfe583c3c3909da4a767b4df Mon Sep 17 00:00:00 2001 From: reshke Date: Mon, 21 Sep 2026 14:29:36 +0300 Subject: [PATCH v3 1/2] Fix NaN handling in BRIN box_inclusion_ops and GiST ops A box with a NaN coordinate fooled BRIN summaries and GiST union keys, masking completely unlrelated rows. Previous issue of this kind was fixed in 1acf757, adopt the same fix here. To fix, add helper functions to float.h and use them in box etc ops, replacing NaN box bounds with infinite bounds. Also add support function to box inclusion ops, to mark box with NaN bounds unmergable. In GiST, fix search keys to correctly support bounding box with NaN values. Author: Kirill Reshke Author: Shihao Zhong Reported-by: BUG #19705 Discussion: https://postgr.es/m/19705-548fda77321e062d@postgresql.org --- src/backend/access/gist/gistproc.c | 40 +++++++++++++++++--------- src/backend/utils/adt/geo_ops.c | 29 ++++++++++++++++--- src/include/catalog/pg_amproc.dat | 2 ++ src/include/catalog/pg_proc.dat | 3 ++ src/include/utils/float.h | 22 ++++++++++++++ src/test/regress/expected/brin.out | 34 ++++++++++++++++++++++ src/test/regress/expected/geometry.out | 20 +++++++++++++ src/test/regress/expected/gist.out | 31 ++++++++++++++++++++ src/test/regress/sql/brin.sql | 15 ++++++++++ src/test/regress/sql/geometry.sql | 6 ++++ src/test/regress/sql/gist.sql | 17 +++++++++++ 11 files changed, 201 insertions(+), 18 deletions(-) diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c index f1044f49d6c..2538eb50b07 100644 --- a/src/backend/access/gist/gistproc.c +++ b/src/backend/access/gist/gistproc.c @@ -141,19 +141,31 @@ gist_box_consistent(PG_FUNCTION_ARGS) } /* - * Increase BOX b to include addon. + * Increase BOX b to include addon. A NaN in either box grows b to the + * corresponding infinity. */ static void adjustBox(BOX *b, const BOX *addon) { - if (float8_lt(b->high.x, addon->high.x)) - b->high.x = addon->high.x; - if (float8_gt(b->low.x, addon->low.x)) - b->low.x = addon->low.x; - if (float8_lt(b->high.y, addon->high.y)) - b->high.y = addon->high.y; - if (float8_gt(b->low.y, addon->low.y)) - b->low.y = addon->low.y; + b->high.x = float8_bound_max(b->high.x, addon->high.x); + b->low.x = float8_bound_min(b->low.x, addon->low.x); + b->high.y = float8_bound_max(b->high.y, addon->high.y); + b->low.y = float8_bound_min(b->low.y, addon->low.y); +} + +/* Copy a BOX, mapping NaNs to infinities. */ +static void +snapBox(BOX *b, const BOX *box) +{ + *b = *box; + if (isnan(b->high.x)) + b->high.x = get_float8_infinity(); + if (isnan(b->high.y)) + b->high.y = get_float8_infinity(); + if (isnan(b->low.x)) + b->low.x = -get_float8_infinity(); + if (isnan(b->low.y)) + b->low.y = -get_float8_infinity(); } /* @@ -239,7 +251,7 @@ fallbackSplit(GistEntryVector *entryvec, GIST_SPLITVEC *v) if (unionL == NULL) { unionL = palloc_object(BOX); - *unionL = *cur; + snapBox(unionL, cur); } else adjustBox(unionL, cur); @@ -252,7 +264,7 @@ fallbackSplit(GistEntryVector *entryvec, GIST_SPLITVEC *v) if (unionR == NULL) { unionR = palloc_object(BOX); - *unionR = *cur; + snapBox(unionR, cur); } else adjustBox(unionR, cur); @@ -526,7 +538,7 @@ gist_box_picksplit(PG_FUNCTION_ARGS) { box = DatumGetBoxP(entryvec->vector[i].key); if (i == FirstOffsetNumber) - context.boundingBox = *box; + snapBox(&context.boundingBox, box); else adjustBox(&context.boundingBox, box); } @@ -715,7 +727,7 @@ gist_box_picksplit(PG_FUNCTION_ARGS) if (v->spl_nleft > 0) \ adjustBox(leftBox, box); \ else \ - *leftBox = *(box); \ + snapBox(leftBox, box); \ v->spl_left[v->spl_nleft++] = off; \ } while(0) @@ -724,7 +736,7 @@ gist_box_picksplit(PG_FUNCTION_ARGS) if (v->spl_nright > 0) \ adjustBox(rightBox, box); \ else \ - *rightBox = *(box); \ + snapBox(rightBox, box); \ v->spl_right[v->spl_nright++] = off; \ } while(0) diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c index 73324b91fe5..6a2ad14cb3f 100644 --- a/src/backend/utils/adt/geo_ops.c +++ b/src/backend/utils/adt/geo_ops.c @@ -4419,14 +4419,35 @@ boxes_bound_box(PG_FUNCTION_ARGS) container = palloc_object(BOX); - container->high.x = float8_max(box1->high.x, box2->high.x); - container->low.x = float8_min(box1->low.x, box2->low.x); - container->high.y = float8_max(box1->high.y, box2->high.y); - container->low.y = float8_min(box1->low.y, box2->low.y); + /* NaNs become infinite bounds, never NaNs. */ + container->high.x = float8_bound_max(box1->high.x, box2->high.x); + container->low.x = float8_bound_min(box1->low.x, box2->low.x); + container->high.y = float8_bound_max(box1->high.y, box2->high.y); + container->low.y = float8_bound_min(box1->low.y, box2->low.y); PG_RETURN_BOX_P(container); } +/* + * Boxes with NaN coordinates cannot be merged, since no operator can match a NaN summary. + * Used in BRIN box_inclusion_ops PROCNUM_MERGEABLE support. + */ +extern Datum box_mergeable(PG_FUNCTION_ARGS); +Datum +box_mergeable(PG_FUNCTION_ARGS) +{ + BOX *box1 = PG_GETARG_BOX_P(0), + *box2 = PG_GETARG_BOX_P(1); + + if (isnan(box1->high.x) || isnan(box1->high.y) || + isnan(box1->low.x) || isnan(box1->low.y) || + isnan(box2->high.x) || isnan(box2->high.y) || + isnan(box2->low.x) || isnan(box2->low.y)) + PG_RETURN_BOOL(false); + + PG_RETURN_BOOL(true); +} + /*********************************************************************** ** diff --git a/src/include/catalog/pg_amproc.dat b/src/include/catalog/pg_amproc.dat index 4a1efdbc899..db24e42e795 100644 --- a/src/include/catalog/pg_amproc.dat +++ b/src/include/catalog/pg_amproc.dat @@ -2033,6 +2033,8 @@ amproc => 'brin_inclusion_union' }, { amprocfamily => 'brin/box_inclusion_ops', amproclefttype => 'box', amprocrighttype => 'box', amprocnum => '11', amproc => 'bound_box' }, +{ amprocfamily => 'brin/box_inclusion_ops', amproclefttype => 'box', + amprocrighttype => 'box', amprocnum => '12', amproc => 'box_mergeable' }, { amprocfamily => 'brin/box_inclusion_ops', amproclefttype => 'box', amprocrighttype => 'box', amprocnum => '13', amproc => 'box_contain' }, diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f46427258e3..bbbce58a962 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -2140,6 +2140,9 @@ { oid => '4067', descr => 'bounding box of two boxes', proname => 'bound_box', prorettype => 'box', proargtypes => 'box box', prosrc => 'boxes_bound_box' }, +{ oid => '8400', descr => 'can two boxes be merged into a single summary', + proname => 'box_mergeable', prorettype => 'bool', proargtypes => 'box box', + prosrc => 'box_mergeable' }, { oid => '981', descr => 'box diagonal', proname => 'diagonal', prorettype => 'lseg', proargtypes => 'box', prosrc => 'box_diagonal' }, diff --git a/src/include/utils/float.h b/src/include/utils/float.h index ffa743d6273..843c4ecdef5 100644 --- a/src/include/utils/float.h +++ b/src/include/utils/float.h @@ -336,4 +336,26 @@ float8_max(const float8 val1, const float8 val2) return float8_gt(val1, val2) ? val1 : val2; } +/* + * float8_bound_max/min: like float8_max/float8_min, but a NaN is mapped + * to the corresponding infinity. Used for geometric bounds, where a NaN + * would make every comparison with the bound false and thus hide the + * values it was meant to cover. + */ +static inline float8 +float8_bound_max(const float8 val1, const float8 val2) +{ + if (isnan(val1) || isnan(val2)) + return get_float8_infinity(); + return float8_max(val1, val2); +} + +static inline float8 +float8_bound_min(const float8 val1, const float8 val2) +{ + if (isnan(val1) || isnan(val2)) + return -get_float8_infinity(); + return float8_min(val1, val2); +} + #endif /* FLOAT_H */ diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out index e1db2280cf9..a73f7b7e82d 100644 --- a/src/test/regress/expected/brin.out +++ b/src/test/regress/expected/brin.out @@ -589,3 +589,37 @@ CREATE INDEX brin_insert_optimization_idx ON brin_insert_optimization USING brin UPDATE brin_insert_optimization SET a = a; REINDEX INDEX CONCURRENTLY brin_insert_optimization_idx; DROP TABLE brin_insert_optimization; +-- a box with a NaN coordinate must not hide the other rows of its page +-- range (bug #19705) +CREATE TABLE brin_box_nan (v box); +INSERT INTO brin_box_nan SELECT box '(0,0),(1,1)' FROM generate_series(1, 100); +INSERT INTO brin_box_nan VALUES (box '(NaN,NaN),(0,0)'); +CREATE INDEX brin_box_nan_idx ON brin_box_nan USING brin (v); +SET enable_seqscan = off; +SELECT count(*) FROM brin_box_nan WHERE v && box '(-2,-2),(2,2)'; + count +------- + 100 +(1 row) + +SELECT count(*) FROM brin_box_nan WHERE v @> point '(0.5,0.5)'; + count +------- + 100 +(1 row) + +SELECT count(*) FROM brin_box_nan WHERE v ~= box '(0,0),(1,1)'; + count +------- + 100 +(1 row) + +-- the NaN row is found too: unmergeable ranges are always scanned +SELECT count(*) FROM brin_box_nan WHERE v ~= box '(NaN,NaN),(0,0)'; + count +------- + 1 +(1 row) + +RESET enable_seqscan; +DROP TABLE brin_box_nan; diff --git a/src/test/regress/expected/geometry.out b/src/test/regress/expected/geometry.out index 1d168b21cbc..1372d34a668 100644 --- a/src/test/regress/expected/geometry.out +++ b/src/test/regress/expected/geometry.out @@ -5321,3 +5321,23 @@ SELECT * FROM pg_input_error_info('(1,2),-1', 'circle'); invalid input syntax for type circle: "(1,2),-1" | | | 22P02 (1 row) +-- NaN becomes an infinite bound (bug #19705) +SELECT bound_box(box '(0,0),(1,1)', box '(NaN,3),(0,2)'); + bound_box +-------------------- + (Infinity,3),(0,0) +(1 row) + +-- box_mergeable() rejects NaN coordinates (BRIN box_inclusion_ops) +SELECT box_mergeable(box '(0,0),(1,1)', box '(NaN,3),(0,2)'); + box_mergeable +--------------- + f +(1 row) + +SELECT box_mergeable(box '(0,0),(1,1)', box '(2,3),(0,2)'); + box_mergeable +--------------- + t +(1 row) + diff --git a/src/test/regress/expected/gist.out b/src/test/regress/expected/gist.out index ac79f94aa80..82197b4dcef 100644 --- a/src/test/regress/expected/gist.out +++ b/src/test/regress/expected/gist.out @@ -463,3 +463,34 @@ create index gist_tbl_box_index on gist_tbl using gist (b); insert into gist_tbl select box(point(0.05*i, 0.05*i)) from generate_series(0,10) as i; drop table gist_tbl; +-- a box with a NaN coordinate must not poison the union keys (bug #19705) +create table gist_nan_tbl (b box); +insert into gist_nan_tbl select box '(1,1),(2,2)' from generate_series(1, 1000); +insert into gist_nan_tbl values (box '(NaN,NaN),(0,0)'); +create index gist_nan_tbl_index on gist_nan_tbl using gist (b); +-- also insert rows after the build, to exercise the insertion path +insert into gist_nan_tbl select box '(3,3),(4,4)' from generate_series(1, 1000); +insert into gist_nan_tbl values (box '(NaN,NaN),(0,0)'); +set enable_seqscan = off; +set enable_bitmapscan = off; +select count(*) from gist_nan_tbl where b && box(point(0,0), point(5,5)); + count +------- + 2000 +(1 row) + +select count(*) from gist_nan_tbl where b <@ box(point(0,0), point(5,5)); + count +------- + 2000 +(1 row) + +select count(*) from gist_nan_tbl where b @> point '(1.5,1.5)'; + count +------- + 1000 +(1 row) + +reset enable_seqscan; +reset enable_bitmapscan; +drop table gist_nan_tbl; diff --git a/src/test/regress/sql/brin.sql b/src/test/regress/sql/brin.sql index 7ea97f47c8d..3484869d676 100644 --- a/src/test/regress/sql/brin.sql +++ b/src/test/regress/sql/brin.sql @@ -534,3 +534,18 @@ CREATE INDEX brin_insert_optimization_idx ON brin_insert_optimization USING brin UPDATE brin_insert_optimization SET a = a; REINDEX INDEX CONCURRENTLY brin_insert_optimization_idx; DROP TABLE brin_insert_optimization; + +-- a box with a NaN coordinate must not hide the other rows of its page +-- range (bug #19705) +CREATE TABLE brin_box_nan (v box); +INSERT INTO brin_box_nan SELECT box '(0,0),(1,1)' FROM generate_series(1, 100); +INSERT INTO brin_box_nan VALUES (box '(NaN,NaN),(0,0)'); +CREATE INDEX brin_box_nan_idx ON brin_box_nan USING brin (v); +SET enable_seqscan = off; +SELECT count(*) FROM brin_box_nan WHERE v && box '(-2,-2),(2,2)'; +SELECT count(*) FROM brin_box_nan WHERE v @> point '(0.5,0.5)'; +SELECT count(*) FROM brin_box_nan WHERE v ~= box '(0,0),(1,1)'; +-- the NaN row is found too: unmergeable ranges are always scanned +SELECT count(*) FROM brin_box_nan WHERE v ~= box '(NaN,NaN),(0,0)'; +RESET enable_seqscan; +DROP TABLE brin_box_nan; diff --git a/src/test/regress/sql/geometry.sql b/src/test/regress/sql/geometry.sql index c3ea368da5e..6f45999310c 100644 --- a/src/test/regress/sql/geometry.sql +++ b/src/test/regress/sql/geometry.sql @@ -529,3 +529,9 @@ SELECT pg_input_is_valid('(1', 'circle'); SELECT * FROM pg_input_error_info('1,', 'circle'); SELECT pg_input_is_valid('(1,2),-1', 'circle'); SELECT * FROM pg_input_error_info('(1,2),-1', 'circle'); + +-- NaN becomes an infinite bound (bug #19705) +SELECT bound_box(box '(0,0),(1,1)', box '(NaN,3),(0,2)'); +-- box_mergeable() rejects NaN coordinates (BRIN box_inclusion_ops) +SELECT box_mergeable(box '(0,0),(1,1)', box '(NaN,3),(0,2)'); +SELECT box_mergeable(box '(0,0),(1,1)', box '(2,3),(0,2)'); diff --git a/src/test/regress/sql/gist.sql b/src/test/regress/sql/gist.sql index 57dcc082450..5e637ce11a4 100644 --- a/src/test/regress/sql/gist.sql +++ b/src/test/regress/sql/gist.sql @@ -236,3 +236,20 @@ create index gist_tbl_box_index on gist_tbl using gist (b); insert into gist_tbl select box(point(0.05*i, 0.05*i)) from generate_series(0,10) as i; drop table gist_tbl; + +-- a box with a NaN coordinate must not poison the union keys (bug #19705) +create table gist_nan_tbl (b box); +insert into gist_nan_tbl select box '(1,1),(2,2)' from generate_series(1, 1000); +insert into gist_nan_tbl values (box '(NaN,NaN),(0,0)'); +create index gist_nan_tbl_index on gist_nan_tbl using gist (b); +-- also insert rows after the build, to exercise the insertion path +insert into gist_nan_tbl select box '(3,3),(4,4)' from generate_series(1, 1000); +insert into gist_nan_tbl values (box '(NaN,NaN),(0,0)'); +set enable_seqscan = off; +set enable_bitmapscan = off; +select count(*) from gist_nan_tbl where b && box(point(0,0), point(5,5)); +select count(*) from gist_nan_tbl where b <@ box(point(0,0), point(5,5)); +select count(*) from gist_nan_tbl where b @> point '(1.5,1.5)'; +reset enable_seqscan; +reset enable_bitmapscan; +drop table gist_nan_tbl; -- 2.37.1 (Apple Git-137.1)