From eab68b1be6aa05e42330d5d7a00006e7dac8f997 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 946dabc4527..e038c998472 100644 --- a/src/backend/access/spgist/spgquadtreeproc.c +++ b/src/backend/access/spgist/spgquadtreeproc.c @@ -41,6 +41,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. * @@ -56,6 +82,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) || @@ -76,8 +105,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 2e911285600..064b0a69cae 100644 --- a/src/test/regress/expected/spgist.out +++ b/src/test/regress/expected/spgist.out @@ -94,3 +94,19 @@ select box(point(i,j)) from generate_series(1,100,5) i, generate_series(1,10,5) j; -- leave this table around, to help in testing dump/restore +-- 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 4828ede68c3..c7bfae347db 100644 --- a/src/test/regress/sql/spgist.sql +++ b/src/test/regress/sql/spgist.sql @@ -89,3 +89,16 @@ select box(point(i,j)) from generate_series(1,100,5) i, generate_series(1,10,5) j; -- leave this table around, to help in testing dump/restore + +-- 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;