| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | shihao zhong <zhong950419(at)gmail(dot)com> |
| Cc: | kehan5800(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19705: One NaN box makes a BRIN box_inclusion_ops index omit unrelated rows |
| Date: | 2026-09-22 12:12:56 |
| Message-ID: | CALdSSPgBZXc0Arj-VwFWsuGF19yTkzRtkq0JzR+feaYZukUTTw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Hi!
I think your fix is basically correct and comes in sync with previous
fix of this kind [0]
Your 0001 makes the BRIN symptom go away, including on the back branches.
A few observations and an alternative (your 0001 adjusted) I have been
working on (PFA)
On Tue, 22 Sept 2026 at 04:50, shihao zhong <zhong950419(at)gmail(dot)com> wrote:
>
> Hi,
>
> I can reproduce this on master, and your analysis is right. bound_box()
> lets the NaN into the range summary, and then every box operator says
> the range cannot match.
>
> You suggested adding a mergeable support function, like inet has. That
> works, but it needs a new pg_amproc row, so it cannot go to the back
> branches. The same code is in 14 and later.
Yes, but for HEAD it's OK and will be an idiomatic way to fix. So, we
can make different patches for HEAD and back branches, where the
back-branched version will miss pg_amproc fix (and misbehave on index
scan, but looks like we can live with that). Example:
CREATE TABLE b (v box);
INSERT INTO b SELECT box '(0,0),(1,1)' FROM generate_series(1, 1000);
INSERT INTO b VALUES (box '(NaN,NaN),(0,0)');
CREATE INDEX bi ON b USING brin (v);
SET enable_seqscan = off;
SELECT count(*) FROM b WHERE v && box '(-2,-2),(2,2)'; -- seq: 1000
SELECT count(*) FROM b WHERE v @> point '(0.5,0.5)'; -- seq: 1000
SELECT count(*) FROM b WHERE v ~= box '(0,0),(1,1)'; -- seq: 1000
SELECT count(*) FROM b WHERE v ~= box '(NaN,NaN),(0,0)'; -- seq:
1 (your v1 will return 0)
> The attached 0001 fixes bound_box() instead. When an input coordinate is
> NaN, the result is infinite on that side. The summary then matches any
> query on that axis and the recheck does the rest. Only the NaN axis
> becomes lossy, the other axis still prunes.
>
> Existing summaries that hold a NaN stay broken until REINDEX.
>
> 0002 adds tests and is optional.
>
> Thanks,
> Shihao
>
>
As the reporter already measured, GiST has the same defect, so 0002
fixes that too.
The GiST issue is in fallbackSplit(), gist_box_picksplit etc build the
groups union keys by copying the first entry
as-is `*leftBox = *box` and then growing the copy, so a NaN box placed
first in its group produces a NaN key. To fix this, there is a small
NaN-aware copy helper function. Also, adjustBoxnow uses two NaN-aware
helper functions, so they replace NaN with inf values too. This is
what makes the NaN keys not propagate to the root on the insert path.
Note that this efficiently means we will insert different tuples in
index (without NaNs but with Inf). See gist_page_items output for
reference.
GiST index failing test case is the same:
db2=#
CREATE TABLE b (v box);
INSERT INTO b SELECT box '(0,0),(1,1)' FROM generate_series(1, 1000);
INSERT INTO b VALUES (box '(NaN,NaN),(0,0)'); -- one row
CREATE TABLE
INSERT 0 1000
INSERT 0 1
db2=# CREATE INDEX bi ON b USING gist (v);
CREATE INDEX
db2=# select count(*) from b where v && box(point(1,1), point(4,4));
count
-------
835
(1 row)
db2=# drop index bi;
DROP INDEX
db2=# select count(*) from b where v && box(point(1,1), point(4,4));
count
-------
1000
(1 row)
before/after 0002 with pageinspect.
```
db2=# SELECT * FROM gist_page_items(get_raw_page('gist_nan_tbl_index',
0), 'gist_nan_tbl_index');
itemoffset | ctid | itemlen | dead | keys
------------+------------+---------+------+-----------------------------
1 | (1,65535) | 40 | f | (b)=("(82,82),(0,0)")
2 | (2,65535) | 40 | f | (b)=("(165,165),(83,83)")
3 | (3,65535) | 40 | f | (b)=("(NaN,NaN),(0,0)")
4 | (4,65535) | 40 | f | (b)=("(331,331),(249,249)")
5 | (5,65535) | 40 | f | (b)=("(414,414),(332,332)")
6 | (6,65535) | 40 | f | (b)=("(497,497),(415,415)")
7 | (7,65535) | 40 | f | (b)=("(580,580),(498,498)")
8 | (8,65535) | 40 | f | (b)=("(663,663),(581,581)")
9 | (9,65535) | 40 | f | (b)=("(746,746),(664,664)")
10 | (10,65535) | 40 | f | (b)=("(829,829),(747,747)")
11 | (11,65535) | 40 | f | (b)=("(912,912),(830,830)")
12 | (12,65535) | 40 | f | (b)=("(999,999),(913,913)")
```
```
reshke=# SELECT * FROM
gist_page_items(get_raw_page('gist_nan_tbl_index', 0),
'gist_nan_tbl_index');
itemoffset | ctid | itemlen | dead | keys
------------+------------+---------+------+-----------------------------------
1 | (1,65535) | 40 | f | (b)=("(Infinity,Infinity),(0,0)")
2 | (2,65535) | 40 | f | (b)=("(165,165),(83,83)")
3 | (3,65535) | 40 | f | (b)=("(248,248),(166,166)")
4 | (4,65535) | 40 | f | (b)=("(331,331),(249,249)")
5 | (5,65535) | 40 | f | (b)=("(414,414),(332,332)")
6 | (6,65535) | 40 | f | (b)=("(497,497),(415,415)")
7 | (7,65535) | 40 | f | (b)=("(580,580),(498,498)")
8 | (8,65535) | 40 | f | (b)=("(663,663),(581,581)")
9 | (9,65535) | 40 | f | (b)=("(746,746),(664,664)")
10 | (10,65535) | 40 | f | (b)=("(829,829),(747,747)")
11 | (11,65535) | 40 | f | (b)=("(912,912),(830,830)")
12 | (12,65535) | 40 | f | (b)=("(999,999),(913,913)")
(12 rows)
```
GiST indexes obviously need to be REINDEX-ed after that.
[0] https://github.com/postgres/postgres/commit/1acf7572554
--
Best regards,
Kirill Reshke
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Fix-NaN-handling-in-BRIN-box_inclusion_ops-and-Gi.patch | application/octet-stream | 14.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-09-22 12:37:03 | Re: BUG #19705: One NaN box makes a BRIN box_inclusion_ops index omit unrelated rows |
| Previous Message | PG Bug reporting form | 2026-09-22 10:12:35 | BUG #19712: MultiXact Recovery Deadlock |