From 0e95444395db94610eb09aec92b6ce89202b8805 Mon Sep 17 00:00:00 2001 From: John Naylor Date: Wed, 14 Oct 2026 18:19:39 +0700 Subject: [PATCH] Plug holes in SP-GiST quad tree getQuadrant() getQuadrant() places a point on each axis with the fuzzy comparisons FPlt(), FPgt() and FPeq(), which do not cover every case. FPeq() compares the rounded difference against EPSILON, while FPlt() and FPgt() round the sum of one operand and EPSILON. When the centroid plus EPSILON rounds up, a point equal to that rounded sum is more than EPSILON above the centroid, so FPeq() is false. FPgt() compares it with the same rounded sum, so it is false too, and FPlt() is false because the point is above the centroid. The same gap exists below the centroid, when the centroid minus EPSILON rounds down. No quadrant matches, and index builds, inserts and searches fail with "getQuadrant: impossible case". To fix, fall back to deciding each axis separately: Use fuzzy tests first and then tiebreak with exact comparisons. That gives every finite value a quadrant (NaN coordinates are still an error). Points that matched a fuzzy test before are routed as before, so existing indexes remain valid. Bug: #19597 Reported-by: Michael Malis Author: Ayush Tiwari Reviewed-by: Andrey Rachitskiy Reviewed-by: Pierre Forstmann Discussion: https://postgr.es/m/19597-39c532e61d78dff6@postgresql.org Backpatch-through: 14 --- src/backend/access/spgist/spgquadtreeproc.c | 43 ++++++++++++++++++++- src/test/regress/expected/spgist.out | 16 ++++++++ src/test/regress/sql/spgist.sql | 13 +++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/backend/access/spgist/spgquadtreeproc.c b/src/backend/access/spgist/spgquadtreeproc.c index a52d924fdc9..01c419c2825 100644 --- a/src/backend/access/spgist/spgquadtreeproc.c +++ b/src/backend/access/spgist/spgquadtreeproc.c @@ -39,6 +39,32 @@ spg_quad_config(PG_FUNCTION_ARGS) #define SPTEST(f, x, y) \ DatumGetBool(DirectFunctionCall2(f, PointPGetDatum(x), PointPGetDatum(y))) +/* + * Compare one coordinate of a point against that of the centroid + */ +static int +getAxisSide(float8 tst, float8 centroid) +{ + /* + * Test FPeq first so that points on a centroid axis get the same result + * as getQuadrant. + */ + if (FPeq(tst, centroid)) + return 0; + if (FPgt(tst, centroid)) + return 1; + if (FPlt(tst, centroid)) + return -1; + /* fall back to exact comparison */ + if (tst > centroid) + return 1; + if (tst < centroid) + return -1; + + elog(ERROR, "getQuadrant: impossible case"); + return 0; +} + /* * Determine which quadrant a point falls into, relative to the centroid. * @@ -54,6 +80,9 @@ spg_quad_config(PG_FUNCTION_ARGS) static int16 getQuadrant(Point *centroid, Point *tst) { + int xside; + int yside; + if ((SPTEST(point_above, tst, centroid) || SPTEST(point_horiz, tst, centroid)) && (SPTEST(point_right, tst, centroid) || @@ -74,8 +103,18 @@ getQuadrant(Point *centroid, Point *tst) SPTEST(point_left, tst, centroid)) return 4; - elog(ERROR, "getQuadrant: impossible case"); - return 0; + /* + * The fuzzy tests above can all fail on one axis, because FPeq() rounds a + * difference while FPlt() and FPgt() round a sum. Compare exactly only + * on that axis. On the other axis, keep the result of the fuzzy tests, + * because searches route by that result too. + */ + xside = getAxisSide(tst->x, centroid->x); + yside = getAxisSide(tst->y, centroid->y); + + if (xside >= 0) + return (yside >= 0) ? 1 : 2; + return (yside <= 0) ? 3 : 4; } /* Returns bounding box of a given quadrant inside given bounding box */ diff --git a/src/test/regress/expected/spgist.out b/src/test/regress/expected/spgist.out index 1688e0e0a32..28fd12f7bff 100644 --- a/src/test/regress/expected/spgist.out +++ b/src/test/regress/expected/spgist.out @@ -86,3 +86,19 @@ select * from spgist_domain_tbl where f1 = 'fo'; fo (1 row) +-- Bug #19597: coverage of getQuadrant for a point just over EPSILON from the centroid +create table spgist_quad_fp_tbl(p point); +insert into spgist_quad_fp_tbl select point(i, 5) from generate_series(1, 300) i; +create index spgist_quad_fp_idx on spgist_quad_fp_tbl using spgist(p); +insert into spgist_quad_fp_tbl values (point(3, 5 + 1e-6)); +-- Bug #19597: needs exact comparison on one axis, within EPSILON on the other +insert into spgist_quad_fp_tbl values (point(114 - 5e-7, 5 + 1e-6)); +set enable_seqscan = off; +select count(*) from spgist_quad_fp_tbl + where p <@ box(point(113.999999, 5.0000004), point(113.9999999, 5.0000016)); + count +------- + 1 +(1 row) + +reset enable_seqscan; diff --git a/src/test/regress/sql/spgist.sql b/src/test/regress/sql/spgist.sql index 7644f344a91..cf1dd909179 100644 --- a/src/test/regress/sql/spgist.sql +++ b/src/test/regress/sql/spgist.sql @@ -80,3 +80,16 @@ insert into spgist_domain_tbl values('fee'), ('fi'), ('fo'), ('fum'); explain (costs off) select * from spgist_domain_tbl where f1 = 'fo'; select * from spgist_domain_tbl where f1 = 'fo'; + +-- Bug #19597: coverage of getQuadrant for a point just over EPSILON from the centroid +create table spgist_quad_fp_tbl(p point); +insert into spgist_quad_fp_tbl select point(i, 5) from generate_series(1, 300) i; +create index spgist_quad_fp_idx on spgist_quad_fp_tbl using spgist(p); +insert into spgist_quad_fp_tbl values (point(3, 5 + 1e-6)); + +-- Bug #19597: needs exact comparison on one axis, within EPSILON on the other +insert into spgist_quad_fp_tbl values (point(114 - 5e-7, 5 + 1e-6)); +set enable_seqscan = off; +select count(*) from spgist_quad_fp_tbl + where p <@ box(point(113.999999, 5.0000004), point(113.9999999, 5.0000016)); +reset enable_seqscan;