Re: Bug: GiST index-only scans can use tuple descriptor with incorrect alignment when deforming

On 7/15/26 00:39, Peter Geoghegan wrote:
> On Mon, Jul 13, 2026 at 6:20 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>> Attached bugfix teaches nodeIndexonlyscan.c to deform using the
>> authoritative xs_hitupdesc descriptor used by the index AM, rather
>> than assuming that the descriptor used by the scan's virtual slot is
>> 100% compatible with xs_hitupdesc.
>
> Attached V2 polishes the test case, and improves the comments and
> commit message (no substantive change).
>
> The conditions under which this bug could lead to wrong answers were
> fairly subtle, so it's worth clarifying those aspects in the committed
> test case.
>
>> The patch includes a test case demonstrating a query where this leads
>> to incorrect behavior when scanning a multicolumn GiST index. Without
>> the fix the executor gets confused about where the second index column
>> begins, leading to a spurious error (e.g., "ERROR: type with OID nnnnn
>> does not exist"). I haven't investigated whether the consequences
>> could be worse than just an error.
>
> I can now confirm that this bug might cause a SIGSEGV.
>

Thanks. I took a look at the v2 fix, and I think it's correct / fine. It
does surprise me a bit we've never seen any reports of failures, but I
guess the problematic opclasses are not used very often. Or not with an
index that would trigger it.

I think moving the logic to StoreIndexTuple is a clear improvement.

A couple comment nitpicks:

* In either case we must deform the tuple using the tupdesc the AM
* formed it with (xs_hitupdesc or xs_itupdesc), not the slot's tupdesc.
* An AM builds that descriptor from its opclass, so a column's type
* there can differ ...

It's not entirely clear which descriptior "that descriptor" refers to.

Maybe this should say "may not match"?

* anyrange forms its tuples with that type's alignment, which need not
* match the alignment of the actual type.

I wonder if we want to mention particular opclasses in comments,
references like this are easy to go stale / obsolete. But it's probably
worth it, not sure.

* The one core opclass that goes further, storing a datum that isn't
* even binary compatible with the indexed column, is btree's name_ops,

thanks

--
Tomas Vondra