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

On Mon, Jul 13, 2026 at 6:20 PM Peter Geoghegan <pg@bowt.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.

--
Peter Geoghegan

On 7/15/26 00:39, Peter Geoghegan wrote:

On Mon, Jul 13, 2026 at 6:20 PM Peter Geoghegan <pg@bowt.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

On Wed, Jul 15, 2026 at 7:06 AM Tomas Vondra <tomas@vondra.me> wrote:

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.

Yeah, it's quite a narrow issue.

Pushed a slightly improved version of this fix just now, backpatching
all the way. I improved the comments in line with your feedback.

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

Yes, it's definitely better this way. We're literally doing exactly
the same thing when we handle the xs_itup and xs_hitup cases, so it
makes no sense to handle the xs_hitup directly while only having
xs_itup-related logic in the separate StoreIndexTuple helper function.

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.

I fixed that. I also cut down the comments quite a bit in the committed patch.

Maybe this should say "may not match"?

Agreed.

* 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 committed version has comments mentioning rare cases involving
nonmatching column alignment. We no longer mention any opclass by name
(except for btree's name_ops, which StoreIndexTuple must handle as a
special case).

I concluded that discussing the specific issue in detail wasn't
necessary. There's likely to be nbtree opclasses that we'd have subtly
broken if we somehow made the same mistake with nbtree/xs_itup (which
never happened, likely because doing so would break name_ops in an
obvious way). It would make about as much sense to mention these
never-affected nbtree opclasses as it would to mention the historical
issue with gist/anyrange. Let's mention neither.

Thanks for the review!
--
Peter Geoghegan