database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

Plug holes in SP-GiST quad tree getQuadrant()

Browse Source 
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 <malis@pgrust.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Pierre Forstmann <pierre.forstmann@gmail.com>
Discussion: https://postgr.es/m/19597-39c532e61d78dff6@postgresql.org
Backpatch-through: 14
This commit is contained in:
John Naylor
2026-10-14 18:19:39 +07:00
parent 3d6e246264
commit 0e95444395
3 changed files with 70 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/backend/access/spgist/spgquadtreeproc.c
View File

@@ -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 */

16
src/test/regress/expected/spgist.out
View File

@@ -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;

13
src/test/regress/sql/spgist.sql
View File

@@ -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;