On 7/15/26 00:39, Peter Geoghegan wrote: > On Mon, Jul 13, 2026 at 6:20 PM Peter Geoghegan <[email protected]> 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
