From 610f3ecdac387fb527461caf1e8bccbe8fb8ef98 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Tue, 8 Sep 2026 12:35:25 -0400 Subject: [PATCH v1] ExecForceStoreHeapTuple() loses the tuple's item pointer when the target slot is a TTS_IS_BUFFERTUPLE slot. That branch calls ExecClearTuple(), whose tts_buffer_heap_clear() does ItemPointerSetInvalid(&slot->tts_tid), then installs bslot->base.tuple = heap_copytuple(tuple) but never copies tuple->t_self back into slot->tts_tid. The slot is therefore left advertising InvalidBlockNumber. The sibling routine ExecStoreHeapTuple() -> tts_heap_store_tuple() does set slot->tts_tid = tuple->t_self, so the omission looks like a plain asymmetry rather than an intentional choice. This is user-visible because slot_getsysattr() answers SelfItemPointerAttributeNumber straight out of slot->tts_tid. Any plan that re-stores a heap tuple into a buffer slot through ExecForceStoreHeapTuple() and then projects ctid gets (4294967295,0). nodeIndexscan.c's reorder queue is one such path: reorderqueue_pop() hands the palloc'd copy to ExecForceStoreHeapTuple(). So for any index AM that sets xs_recheckorderby = true, every tuple that passes through the reorder queue projects the invalid-tid sentinel instead of its real heap tid, even though the AM set xs_heaptid correctly. Author: Greg Burd Backpatch-through: 13 --- src/backend/executor/execTuples.c | 8 ++++++++ src/test/regress/expected/gist.out | 31 ++++++++++++++++++++++++++++++ src/test/regress/sql/gist.sql | 23 ++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index b8e8f52c64c..f14c6d6be07 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -1768,6 +1768,14 @@ ExecForceStoreHeapTuple(HeapTuple tuple, slot->tts_flags |= TTS_FLAG_SHOULDFREE; MemoryContextSwitchTo(oldContext); + /* + * ExecClearTuple() above invalidated tts_tid; restore it from the + * tuple so that projecting ctid (slot_getsysattr() reads tts_tid) + * yields the real heap tid rather than InvalidBlockNumber. This + * matches what tts_heap_store_tuple() does for heap slots. + */ + slot->tts_tid = tuple->t_self; + if (shouldFree) pfree(tuple); } diff --git a/src/test/regress/expected/gist.out b/src/test/regress/expected/gist.out index ac79f94aa80..97c0253546f 100644 --- a/src/test/regress/expected/gist.out +++ b/src/test/regress/expected/gist.out @@ -423,6 +423,37 @@ select lower(r) = repeat('7', 200)::numeric as lower_ok, (1 row) drop table gist_ios_tupdesc; +-- Test that tuples passing through nodeIndexscan.c's reorder queue keep their +-- real ctid. poly_ops' distance is only a lower bound (the bounding box), so +-- gist_poly_consistent sets recheck and the ORDER BY value is recomputed; thin +-- diagonal triangles make the estimate strictly low, forcing the requeue path, +-- which re-stores the tuple with ExecForceStoreHeapTuple(). +create table gist_knn_ctid (id int, p polygon); +insert into gist_knn_ctid +select i, ('((' || i*10 || ',0),(' || (i*10+9) || ',9),(' + || (i*10+9) || ',0))')::polygon +from generate_series(1,20) i; +create index gist_knn_ctid_idx on gist_knn_ctid using gist (p); +vacuum analyze gist_knn_ctid; +explain (costs off) +select ctid, id from gist_knn_ctid order by p <-> point(100,4) limit 5; + QUERY PLAN +----------------------------------------------------------- + Limit + -> Index Scan using gist_knn_ctid_idx on gist_knn_ctid + Order By: (p <-> '(100,4)'::point) +(3 rows) + +-- every row must be findable by the ctid it reported +select count(*) as ctid_matches +from (select ctid, id from gist_knn_ctid order by p <-> point(100,4) limit 5) s + join gist_knn_ctid t on t.ctid = s.ctid and t.id = s.id; + ctid_matches +-------------- + 5 +(1 row) + +drop table gist_knn_ctid; -- test deletion of LP_DEAD-marked index tuples create table gist_prune_tbl (k int, p point); create index gist_prune_tbl_p_index on gist_prune_tbl using gist (p); diff --git a/src/test/regress/sql/gist.sql b/src/test/regress/sql/gist.sql index 57dcc082450..550e3aeb26e 100644 --- a/src/test/regress/sql/gist.sql +++ b/src/test/regress/sql/gist.sql @@ -198,6 +198,29 @@ select lower(r) = repeat('7', 200)::numeric as lower_ok, drop table gist_ios_tupdesc; +-- Test that tuples passing through nodeIndexscan.c's reorder queue keep their +-- real ctid. poly_ops' distance is only a lower bound (the bounding box), so +-- gist_poly_consistent sets recheck and the ORDER BY value is recomputed; thin +-- diagonal triangles make the estimate strictly low, forcing the requeue path, +-- which re-stores the tuple with ExecForceStoreHeapTuple(). +create table gist_knn_ctid (id int, p polygon); +insert into gist_knn_ctid +select i, ('((' || i*10 || ',0),(' || (i*10+9) || ',9),(' + || (i*10+9) || ',0))')::polygon +from generate_series(1,20) i; +create index gist_knn_ctid_idx on gist_knn_ctid using gist (p); +vacuum analyze gist_knn_ctid; + +explain (costs off) +select ctid, id from gist_knn_ctid order by p <-> point(100,4) limit 5; + +-- every row must be findable by the ctid it reported +select count(*) as ctid_matches +from (select ctid, id from gist_knn_ctid order by p <-> point(100,4) limit 5) s + join gist_knn_ctid t on t.ctid = s.ctid and t.id = s.id; + +drop table gist_knn_ctid; + -- test deletion of LP_DEAD-marked index tuples create table gist_prune_tbl (k int, p point); create index gist_prune_tbl_p_index on gist_prune_tbl using gist (p); -- 2.50.1