Re: index prefetching

On Tue, Sep 29, 2026 at 3:51 AM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:
> The old amgettuple path already has the same type mismatch. It also
> reads past the 32-byte BOX, but in my run the BOX still pointed into a
> shared buffer mapping and the read did not fault. Since the query only
> projects the included id, the executor ignored the malformed polygon
> value and returned 7.

Hi,

On released branches this isn't always harmless. We ran into it while
fuzzing an 18-based build, as segfaults in heap_deform_tuple() during
index-only scans on an SP-GiST polygon index with an INCLUDE column.

If the box's first coordinate is a small integer, its first 4 bytes
are zero, so heap_form_tuple() doesn't over-read, it builds a polygon
of length 0 instead. heap_deform_tuple() then reads the INCLUDE
column's bytes as the polygon's length word. Depending on the value
you get a wrong result or a crash.

Repro:

CREATE TABLE t (p polygon, id int4);
INSERT INTO t SELECT polygon(box(point(i, i), point(i + 1, i + 1))),
1898491267 - 2 * i
FROM generate_series(1, 25) i;
CREATE INDEX ON t USING spgist (p) INCLUDE (id);
VACUUM ANALYZE t;
SET enable_seqscan = off;
SET enable_bitmapscan = off;
SELECT id FROM t LIMIT 5;

That should return 1898491265, 1898491263, ... but on 18 I get 2249,
2249, 2249, 2249, -1 with an assert build, and 131072, 131072,
-596975064, 131072, -1 with a release build. Odd ids give a short
length and the read lands in the next reconstructed tuple's header
(2249 is its datum_typeid, -1 its typmod). Even ids give a length of
hundreds of MB and it segfaults (try 1898491268 - i in the INSERT).

v37 fixes this in the new spggetbatch code and removes spggettuple, but
14 through 19 still have the old path in spgLeafTest() and
storeGettuple().

Attached is Rui's approach applied to the old code: only ask
leaf_consistent for the key when canReturnData is true, set the key to
NULL otherwise (like gistFetchTuple() does), and skip the datumCopy()
in spgNewHeapItem() for the same case. I included Rui's test, plus one
with integer coordinates, because Rui's test isn't reliable on the back
branches: unpatched, it returned 7 on 14, 17, 18 and 19 and crashed on
15 and 16 in my tests.

I built and tested it on every branch from 14 through 19 and on master,
with assertions enabled. Without the patch the new test aborts the
server and the repro returns garbage or segfaults; with it the repro
returns the right values and make installcheck-parallel passes.

Do you think this should go into the back branches separately from the
prefetching work?

Thanks,
Bilal