Re: BUG #19597: getQuadrant: impossible case is reachable

From: John Naylor <johncnaylorls(at)gmail(dot)com>
To: Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com>
Cc: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, malis(at)pgrust(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #19597: getQuadrant: impossible case is reachable
Date: 2026-09-29 13:38:48
Message-ID: CANWCAZb5t_Au9Wcz1y+xr0VSJY+c27+Gy6tQ6jYaZ8QBxPgr4A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Mon, Aug 3, 2026 at 4:22 AM Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com> wrote:
> The approach looks right to me: keep the fuzzy arms, fall back to exact comparisons for the finite gap, and leave the
> elog for NaN.
>
> Here is a suggested v2 on top of your patch.

Note: "on top of" implies applying both -- v1 and then v2. v2 is
independent, and CI applies only the latest.

"not mutually exhaustive near some power-of-two boundaries"

The reporter stumbled across something interesting, but you actually
don't need to be near a boundary:

drop table if exists g2;
create table g2(p point);
insert into g2 select point(10000000000 + i* 0.001, 5)
from generate_series(1,3000) i;
create index on g2 using spgist(p);
ERROR: getQuadrant: impossible case

Note also that this doesn't need a separate insert. I found on master
and REL_18_STABLE that the reporter's case didn't need one either --
the error happened for me already when building the index.

What I think is happening (from rubber-ducking with Claude):
picksplit() calculates the centroid as the mean of a page of points.
For this binade [2^33, 2^34), where both 17179869183.999998 and
10000000000 are found, the error happens if any point on that page is
exactly 1 ulp (unit in the last place) from the centroid, or 1.9e-6.
This is because this ulp is between EPSILON and 2*EPSILON. A point
exactly 1 ulp from the centroid is then more than EPSILON away, so
FPeq is false. But centroid +/- EPSILON rounds onto that point, so
FPgt and FPlt are false too. Other binades have similar gaps, but it's
probably harder to use them to reach this elog from SQL.

> 1. Write the exact fallback as y-then-x checks, which I find a bit easier to match to the quadrant diagram and the axis tie-breaking rule.

+ if (tst->y > centroid->y)
+ return (tst->x >= centroid->x) ? 1 : 4;
+ if (tst->y < centroid->y)
+ return (tst->x >= centroid->x) ? 2 : 3;
+ if (tst->y == centroid->y)
+ return (tst->x >= centroid->x) ? 1 : 3;

I preferred the style of Ayush's patch. -- this looks really different
from the coding for the fuzzy case. Let's make the exact stanza
similar to the fuzzy stanza, including testing y before x.

--
John Naylor
Amazon Web Services

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2026-09-29 14:01:33 BUG #19727: pg-combinebackup fails to link
Previous Message Grigorev Jurij 2026-09-29 08:37:02 Re: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2026-09-29 13:47:54 Re: parallel autovacuum: Propagate track_cost_delay_timing to parallel workers
Previous Message Bharath Rupireddy 2026-09-29 13:26:17 Re: parallel autovacuum: Propagate track_cost_delay_timing to parallel workers