| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | John Naylor <johncnaylorls(at)gmail(dot)com> |
| Cc: | Andrey Rachitskiy <pl0h0yp1(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 14:59:17 |
| Message-ID: | CAJTYsWX4xaFpk+nC_o8JCSOT9Rk_3VBr3Tyec+F4UDMQEnQo1w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs pgsql-hackers |
Hi,
On Tue, 29 Sept 2026 at 19:09, John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
>
> 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.
Thanks a lot for the review, John!
Attaching v3 with updated test case and earlier fuzzy comparisons.
Regards,
Ayush
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Fix-reachable-getQuadrant-impossible-case.patch | application/octet-stream | 3.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-09-29 15:30:39 | Re: BUG #19727: pg-combinebackup fails to link |
| Previous Message | Tom Lane | 2026-09-29 14:48:20 | Re: BUG #19727: pg-combinebackup fails to link |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-09-29 15:11:50 | Re: remove_useless_joins vs. bug #19560 |
| Previous Message | Álvaro Herrera | 2026-09-29 14:58:12 | Re: ATTACH PARTITION cost grows linearly with pg_constraint size (seqscan in CloneFkReferenced), much worse since not-null constraints are in pg_constraint (PG 18) |