database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

Fix GiST index-only scan column alignment issue.

Browse Source 
An index-only scan filled its result slot from the HeapTuple an index AM
returns in scan->xs_hitup by deforming it with the virtual slot's own
tuple descriptor (during GiST and SP-GiST index-only scans).  But index
AMs form that heap tuple using their own descriptor, scan->xs_hitupdesc.
The AM's descriptor may disagree with the IoS virtual slot's descriptor
about each column's precise alignment, leading to "can't happen" errors
in certain rare edge cases.  Hard crashes were possible but much less
likely.

To fix, deform the tuple with the descriptor it was formed with.  This
is simpler, and makes xs_hitup handling (used by GiST and SP-GiST)
uniform with the nearby existing xs_itup handling (used by nbtree).

In practice this issue was very unlikely to be hit (it was found during
testing of a patch that will change the table AM API used during index
scans).  The only currently affected core opclass is GiST's range_ops.
It was only possible for the datum to be accessed at an incorrectly
aligned offset when reading the second or subsequent column from a
multicolumn GiST index.  This couldn't happen in the common case where
the datum used an unaligned short varlena header.  Moreover, an earlier
column had to leave the range datum at an offset where the two
alignments actually disagree (e.g., an odd-length varlena datum).

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/CAH2-WzkGXa2SKnebdW29RT1hCcQBo_p03v3iqif2u9bjzLB-aQ@mail.gmail.com
Backpatch-through: 14
This commit is contained in:
Peter Geoghegan
2026-07-17 15:53:07 -04:00
parent 3601be26b3
commit 1d4c81ad62
3 changed files with 123 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

111
src/backend/executor/nodeIndexonlyscan.c
View File

@@ -31,6 +31,7 @@
#include "postgres.h"
#include "access/genam.h"
#include "access/htup_details.h"
#include "access/relscan.h"
#include "access/tableam.h"
#include "access/tupdesc.h"
@@ -49,7 +50,7 @@
static TupleTableSlot *IndexOnlyNext(IndexOnlyScanState *node);
static void StoreIndexTuple(IndexOnlyScanState *node, TupleTableSlot *slot,
IndexTuple itup, TupleDesc itupdesc);
IndexScanDesc scandesc);
/* ----------------------------------------------------------------
@@ -193,27 +194,8 @@ IndexOnlyNext(IndexOnlyScanState *node)
tuple_from_heap = true;
}
/*
* Fill the scan tuple slot with data from the index. This might be
* provided in either HeapTuple or IndexTuple format. Conceivably an
* index AM might fill both fields, in which case we prefer the heap
* format, since it's probably a bit cheaper to fill a slot from.
*/
if (scandesc->xs_hitup)
{
/*
* We don't take the trouble to verify that the provided tuple has
* exactly the slot's format, but it seems worth doing a quick
* check on the number of fields.
*/
Assert(slot->tts_tupleDescriptor->natts ==
scandesc->xs_hitupdesc->natts);
ExecForceStoreHeapTuple(scandesc->xs_hitup, slot, false);
}
else if (scandesc->xs_itup)
StoreIndexTuple(node, slot, scandesc->xs_itup, scandesc->xs_itupdesc);
else
elog(ERROR, "no data returned for index-only scan");
/* Fill the scan tuple slot with data from the index */
StoreIndexTuple(node, slot, scandesc);
/*
* If the index was lossy, we have to recheck the index quals.
@@ -263,56 +245,79 @@ IndexOnlyNext(IndexOnlyScanState *node)
/*
* StoreIndexTuple
* Fill the slot with data from the index tuple.
* Fill the slot with the data the index AM returned.
*
* The data might be provided in either HeapTuple (xs_hitup) or IndexTuple
* (xs_itup) format. Conceivably an index AM might fill both fields, in which
* case we prefer the heap format, since it's probably a bit cheaper to fill a
* slot from.
*
* At some point this might be generally-useful functionality, but
* right now we don't need it elsewhere.
*/
static void
StoreIndexTuple(IndexOnlyScanState *node, TupleTableSlot *slot,
IndexTuple itup, TupleDesc itupdesc)
IndexScanDesc scandesc)
{
/*
* Note: we must use the tupdesc supplied by the AM in index_deform_tuple,
* not the slot's tupdesc, in case the latter has different datatypes
* (this happens for btree name_ops in particular). They'd better have
* the same number of columns though, as well as being datatype-compatible
* which is something we can't so easily check.
*/
Assert(slot->tts_tupleDescriptor->natts == itupdesc->natts);
ExecClearTuple(slot);
index_deform_tuple(itup, itupdesc, slot->tts_values, slot->tts_isnull);
/*
* Copy all name columns stored as cstrings back into a NAMEDATALEN byte
* sized allocation. We mark this branch as unlikely as generally "name"
* is used only for the system catalogs and this would have to be a user
* query running on those or some other user table with an index on a name
* column.
* We must deform the tuple using the tupdesc the index AM formed it with
* (xs_hitupdesc or xs_itupdesc), not the slot's tupdesc. The datums
* returned by the index AM must be binary compatible, but the descriptors
* may align each column differently in certain rare cases. (Actually,
* btree's "name" opclass stores cstring tuples that _aren't_ even binary
* compatible, in the strictest sense. We directly handle that here.)
*/
if (unlikely(node->ioss_NameCStringAttNums != NULL))
if (scandesc->xs_hitup)
{
int attcount = node->ioss_NameCStringCount;
Assert(slot->tts_tupleDescriptor->natts == scandesc->xs_hitupdesc->natts);
for (int idx = 0; idx < attcount; idx++)
heap_deform_tuple(scandesc->xs_hitup, scandesc->xs_hitupdesc,
slot->tts_values, slot->tts_isnull);
}
else if (scandesc->xs_itup)
{
Assert(slot->tts_tupleDescriptor->natts == scandesc->xs_itupdesc->natts);
index_deform_tuple(scandesc->xs_itup, scandesc->xs_itupdesc,
slot->tts_values, slot->tts_isnull);
/*
* Copy all name columns stored as cstrings back into a NAMEDATALEN
* byte sized allocation. We mark this branch as unlikely as
* generally "name" is used only for the system catalogs and this
* would have to be a user query running on those or some other user
* table with an index on a name column.
*/
if (unlikely(node->ioss_NameCStringAttNums != NULL))
{
int attnum = node->ioss_NameCStringAttNums[idx];
Name name;
int attcount = node->ioss_NameCStringCount;
/* skip null Datums */
if (slot->tts_isnull[attnum])
continue;
for (int idx = 0; idx < attcount; idx++)
{
int attnum = node->ioss_NameCStringAttNums[idx];
Name name;
/* allocate the NAMEDATALEN and copy the datum into that memory */
name = (Name) MemoryContextAlloc(node->ss.ps.ps_ExprContext->ecxt_per_tuple_memory,
NAMEDATALEN);
/* skip null Datums */
if (slot->tts_isnull[attnum])
continue;
/* use namestrcpy to zero-pad all trailing bytes */
namestrcpy(name, DatumGetCString(slot->tts_values[attnum]));
slot->tts_values[attnum] = NameGetDatum(name);
/*
* allocate the NAMEDATALEN and copy the datum into that
* memory
*/
name = (Name) MemoryContextAlloc(node->ss.ps.ps_ExprContext->ecxt_per_tuple_memory,
NAMEDATALEN);
/* use namestrcpy to zero-pad all trailing bytes */
namestrcpy(name, DatumGetCString(slot->tts_values[attnum]));
slot->tts_values[attnum] = NameGetDatum(name);
}
}
}
else
elog(ERROR, "no data returned for index-only scan");
ExecStoreVirtualTuple(slot);
}

36
src/test/regress/expected/gist.out
View File

@@ -387,6 +387,42 @@ select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
ERROR: lossy distance functions are not supported in index-only scans
-- Test that an index-only scan deforms the tuple it reconstructs with the
-- descriptor the AM formed it with, not the scan slot's descriptor.
create temp table gist_ios_tupdesc (a inet, r numrange);
-- range_ops forms its tuples using the opclass input type, the polymorphic
-- anyrange (alignment 'd'), while the scan slot uses the actual range type
-- numrange (alignment 'i'). A buggy implementation will incorrectly access
-- the r/numrange column at the wrong offset.
--
-- The range bounds are made long so the value needs a four-byte varlena
-- header; shorter values get a one-byte header and are stored without
-- alignment padding, which would mask the problem.
insert into gist_ios_tupdesc
values (
'::1', -- shifts "r" datum value to differing offset
numrange(repeat('7', 200)::numeric, repeat('8', 200)::numeric));
create index on gist_ios_tupdesc using gist (a inet_ops, r);
vacuum analyze gist_ios_tupdesc;
explain (costs off)
select lower(r) = repeat('7', 200)::numeric as lower_ok,
upper(r) = repeat('8', 200)::numeric as upper_ok
from gist_ios_tupdesc where r && numrange(null, null);
QUERY PLAN
--------------------------------------------------------------------
Index Only Scan using gist_ios_tupdesc_a_r_idx on gist_ios_tupdesc
Index Cond: (r && '(,)'::numrange)
(2 rows)
select lower(r) = repeat('7', 200)::numeric as lower_ok,
upper(r) = repeat('8', 200)::numeric as upper_ok
from gist_ios_tupdesc where r && numrange(null, null);
lower_ok | upper_ok
----------+----------
t | t
(1 row)
drop table gist_ios_tupdesc;
-- Force an index build using buffering.
create index gist_tbl_box_index_forcing_buffering on gist_tbl using gist (p)
with (buffering=on, fillfactor=50);

29
src/test/regress/sql/gist.sql
View File

@@ -169,6 +169,35 @@ explain (verbose, costs off)
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
-- Test that an index-only scan deforms the tuple it reconstructs with the
-- descriptor the AM formed it with, not the scan slot's descriptor.
create temp table gist_ios_tupdesc (a inet, r numrange);
-- range_ops forms its tuples using the opclass input type, the polymorphic
-- anyrange (alignment 'd'), while the scan slot uses the actual range type
-- numrange (alignment 'i'). A buggy implementation will incorrectly access
-- the r/numrange column at the wrong offset.
--
-- The range bounds are made long so the value needs a four-byte varlena
-- header; shorter values get a one-byte header and are stored without
-- alignment padding, which would mask the problem.
insert into gist_ios_tupdesc
values (
'::1', -- shifts "r" datum value to differing offset
numrange(repeat('7', 200)::numeric, repeat('8', 200)::numeric));
create index on gist_ios_tupdesc using gist (a inet_ops, r);
vacuum analyze gist_ios_tupdesc;
explain (costs off)
select lower(r) = repeat('7', 200)::numeric as lower_ok,
upper(r) = repeat('8', 200)::numeric as upper_ok
from gist_ios_tupdesc where r && numrange(null, null);
select lower(r) = repeat('7', 200)::numeric as lower_ok,
upper(r) = repeat('8', 200)::numeric as upper_ok
from gist_ios_tupdesc where r && numrange(null, null);
drop table gist_ios_tupdesc;
-- Force an index build using buffering.
create index gist_tbl_box_index_forcing_buffering on gist_tbl using gist (p)
with (buffering=on, fillfactor=50);