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

On Wed, Jul 15, 2026 at 7:06 AM Tomas Vondra <tomas(at)vondra(dot)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