From cd96389d713787b025c84869ba417c6f2f008a86 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 6 Jun 2019 09:46:52 +0300 Subject: [PATCH] Fix confusion on different kinds of slots in IndexOnlyScans. We used the same slot to store a tuple from the index, and to store a tuple from the table. That's not OK. It worked with the heap, because heapam_getnextslot() stores a HeapTuple to the slot, and doesn't care how large the tts_values/nulls arrays are. But when I played with a toy table AM implementation that used a virtual tuple, it caused memory overruns. In the passing, tidy up comments on the ioss_PscanLen fields. --- src/backend/executor/nodeIndexonlyscan.c | 16 +++++++++++++--- src/include/nodes/execnodes.h | 6 ++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c index ee5b1c493b..8a4d795d1a 100644 --- a/src/backend/executor/nodeIndexonlyscan.c +++ b/src/backend/executor/nodeIndexonlyscan.c @@ -166,10 +166,10 @@ IndexOnlyNext(IndexOnlyScanState *node) * Rats, we have to visit the heap to check visibility. */ InstrCountTuples2(node, 1); - if (!index_fetch_heap(scandesc, slot)) + if (!index_fetch_heap(scandesc, node->ioss_TableSlot)) continue; /* no visible tuple, try next index entry */ - ExecClearTuple(slot); + ExecClearTuple(node->ioss_TableSlot); /* * Only MVCC snapshots are supported here, so there should be no @@ -528,7 +528,17 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags) */ tupDesc = ExecTypeFromTL(node->indextlist); ExecInitScanTupleSlot(estate, &indexstate->ss, tupDesc, - table_slot_callbacks(currentRelation)); + &TTSOpsVirtual); + + /* + * We need another slot, in a format that's suitable for the table AM, + * for when we need to fetch a tuple from the table for rechecking + * visibility. + */ + indexstate->ioss_TableSlot = + ExecAllocTableSlot(&estate->es_tupleTable, + RelationGetDescr(currentRelation), + table_slot_callbacks(currentRelation)); /* * Initialize result type and projection info. The node's targetlist will diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 112a9a5368..99b9fa414f 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1359,7 +1359,7 @@ typedef struct * SortSupport for reordering ORDER BY exprs * OrderByTypByVals is the datatype of order by expression pass-by-value? * OrderByTypLens typlens of the datatypes of order by expressions - * pscan_len size of parallel index scan descriptor + * PscanLen size of parallel index scan descriptor * ---------------- */ typedef struct IndexScanState @@ -1403,8 +1403,9 @@ typedef struct IndexScanState * RuntimeContext expr context for evaling runtime Skeys * RelationDesc index relation descriptor * ScanDesc index scan descriptor + * TableSlot slot for holding tuples fetched from the table * VMBuffer buffer in use for visibility map testing, if any - * ioss_PscanLen Size of parallel index-only scan descriptor + * PscanLen size of parallel index-only scan descriptor * ---------------- */ typedef struct IndexOnlyScanState @@ -1421,6 +1422,7 @@ typedef struct IndexOnlyScanState ExprContext *ioss_RuntimeContext; Relation ioss_RelationDesc; struct IndexScanDescData *ioss_ScanDesc; + TupleTableSlot *ioss_TableSlot; Buffer ioss_VMBuffer; Size ioss_PscanLen; } IndexOnlyScanState;