From d9b78866249d2c2b58b8c095c5bbd7426e3d920d Mon Sep 17 00:00:00 2001 From: John Naylor Date: Thu, 8 Oct 2026 11:24:12 +0700 Subject: [PATCH v4 2/2] Decide each axis separately in getQuadrant() fallback XXX Claude prototype; to be squashed into the previous commit if adopted. The exact-comparison fallback decides both axes exactly, even when only one of them fell into the gap between the fuzzy tests. If the other axis is within EPSILON of the centroid, the fuzzy tests treat the point as lying on that axis, and so do searches: spg_quad_inner_consistent() routes the query point for ~= and the box corners for <@ through the fuzzy arms. The fallback instead puts the point on its exact side, so a search can descend into a different quadrant and miss it. For example, with centroid (114, 5), the point (114 - 5e-7, fl(5 + EPSILON)) was stored in quadrant 4, while a <@ box containing it searched only quadrant 1. Instead, decide each axis with the fuzzy tests, FPeq() first as in the arms above, and compare exactly only on an axis they leave undecided. Inputs that matched an arm are unaffected. A ~= search can still miss such a point at a lower level, when the query is fuzzy-equal to the centroid on the gapped axis. That is the existing non-transitivity of FPeq() under ~= pruning, not something the fallback can settle. --- src/backend/access/spgist/spgquadtreeproc.c | 50 +++++++++++++++------ src/test/regress/expected/spgist.out | 11 +++++ src/test/regress/sql/spgist.sql | 7 +++ 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/backend/access/spgist/spgquadtreeproc.c b/src/backend/access/spgist/spgquadtreeproc.c index 7fe69e95649..2674141bd1a 100644 --- a/src/backend/access/spgist/spgquadtreeproc.c +++ b/src/backend/access/spgist/spgquadtreeproc.c @@ -41,6 +41,31 @@ spg_quad_config(PG_FUNCTION_ARGS) #define SPTEST(f, x, y) \ DatumGetBool(DirectFunctionCall2(f, PointPGetDatum(x), PointPGetDatum(y))) +/* + * Classify one coordinate of a point against the centroid's: 1 if greater, + * -1 if less, 0 if equal. This uses the fuzzy tests of getQuadrant(), with + * FPeq() taking precedence as it does there, and compares exactly only when + * all of them fail. That decides any finite value; a NaN fails the exact + * comparisons too and still reaches the error. + */ +static int +getAxisSide(float8 tst, float8 centroid) +{ + if (FPeq(tst, centroid)) + return 0; + if (FPgt(tst, centroid)) + return 1; + if (FPlt(tst, centroid)) + return -1; + 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 +81,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) || @@ -77,21 +105,17 @@ getQuadrant(Point *centroid, Point *tst) return 4; /* - * The fuzzy tests above can all fail on one axis, because FPeq() rounds - * a difference while FPlt() and FPgt() round a sum. Exact comparisons - * always pick a quadrant; use them with the same axis tie-breaking. + * 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. */ - if (tst->y >= centroid->y && tst->x >= centroid->x) - return 1; - if (tst->y < centroid->y && tst->x >= centroid->x) - return 2; - if (tst->y <= centroid->y && tst->x < centroid->x) - return 3; - if (tst->y > centroid->y && tst->x < centroid->x) - return 4; + xside = getAxisSide(tst->x, centroid->x); + yside = getAxisSide(tst->y, centroid->y); - elog(ERROR, "getQuadrant: impossible case"); - return 0; + 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 6e29bf5fd16..641122dc7e0 100644 --- a/src/test/regress/expected/spgist.out +++ b/src/test/regress/expected/spgist.out @@ -99,3 +99,14 @@ 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: point in the gap 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 b631a87fedb..b2b59ebb5d9 100644 --- a/src/test/regress/sql/spgist.sql +++ b/src/test/regress/sql/spgist.sql @@ -95,3 +95,10 @@ 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: point in the gap 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; -- 2.55.0