[PATCH v2] Fix SP-GiST index-only scans that return only INCLUDE columns

Hi,

I'm starting a separate thread because this is a bug in the existing
SP-GiST amgettuple path, independent of the index prefetching work.

An SP-GiST index whose opclass cannot reconstruct the key value
(canReturnData = false) can still have INCLUDE columns, which are always
returnable. The planner can therefore choose an index-only scan for a
query that needs only INCLUDE columns. The scan nevertheless asks
leaf_consistent to return the key and stores whatever comes back as the
key column of the reconstructed tuple. For poly_ops that is the stored
BOX, which heap_form_tuple then treats as a POLYGON varlena.

Depending on the BOX's bytes, heap_form_tuple either reads past the BOX
or builds a zero-length polygon. In the latter case heap_deform_tuple
later reads the first INCLUDE column's bytes as the polygon's length
word, so the scan returns wrong values or crashes.

Here is a reproducer:

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 ORDER BY id DESC LIMIT 5;

The query should return 1898491265, 1898491263, and so on. On
unpatched branches it returns garbage. Using 1898491268 - i in the
INSERT can instead make it segfault in heap_deform_tuple().

The attached patch asks leaf_consistent for the key only when
canReturnData is true, returns the key as NULL otherwise (as
gistFetchTuple() does), and avoids copying that key into the ordered-scan
queue. It also adds regression coverage for both the original
large-varlena case and the integral-coordinate case.

Rui Zhao found and fixed the same problem in the new spggetbatch path
from the index-prefetching series [1], and that fix was incorporated
into v37. That series replaces the existing spggettuple path, but the
released branches still use the old path. The attached patch adapts
Rui's approach to that existing code for backpatching. I previously
posted it in the index-prefetching thread [2], where it did not receive
a response.

I tested the code with assertions enabled on branches 14 through 19 and
on master. Without the patch the new test aborts the server and the
reproducer returns garbage or crashes. With the patch the reproducer
returns the correct values and make installcheck-parallel passes. I
have also checked that v2 applies cleanly to the November 11 tips of
master and REL_15_STABLE through REL_19_STABLE.

The code is unchanged from v1. This v2 starts a standalone discussion,
adds the earlier post to the commit-message references, and changes
Backpatch-through to 15 because PostgreSQL 14 reaches end-of-life on
November 12. The bug itself has existed since INCLUDE support was
added to SP-GiST in v14.

Does this look suitable for backpatching through 15?

[1] https://postgr.es/m/CAHWVJhHccsjcBFdgFemRu_x8n_o0_kn_MMx5g+uAnYtYxKxO5w@mail.gmail.com
[2] https://postgr.es/m/CAPkprsycOwGY4tfU_14aX-3LyLS+D4++4a4umGS0QYgjW+hzMg@mail.gmail.com

Thanks,
Bilal