From 075c4e5dc0c7ae2f915d6ec742a94e2414167fdc Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Tue, 8 Sep 2026 12:35:25 -0400 Subject: [PATCH v3] Restore slot->tts_tid in ExecForceStoreHeapTuple's buffer-slot path 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 (which is why the row data itself is right and only the ctid system column is wrong). The consequences range from silently wrong to a crash: - ctid-keyed dedup and UPDATE ... WHERE ctid = ... silently match the one row that was returned without queueing, instead of all of them. No error is raised. - With row locking, the invalid tid reaches heap_lock_tuple(), which asserts in ItemPointerIsValid(). In a non-assert build there is no assert to catch it and InvalidBlockNumber == P_NEW, so ReadBuffer() extends the relation on disk before failing with "attempted to lock invisible tuple", leaving an uninitialized block behind that makes later sequential scans fail with "invalid page in block N". The regression test uses core GiST only: thin diagonal triangles, so the bounding-box distance strictly under-estimates the true polygon distance, gist_poly_consistent sets recheck, was_exact comes out false and the tuples are pushed to the reorder queue. Note that the first tuple is returned directly without queueing and so keeps its real ctid; a check at LIMIT 1 therefore passes even on unpatched code, which is why the test looks at five rows. Present in all supported branches (REL_14_STABLE .. master); the affected code in ExecForceStoreHeapTuple() is byte-identical across them. Co-authored-by: Virender Singla Co-authored-by: Greg Burd Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/CAM6Zo8wZOLnCWRO_tuuXVX9J4N4JN6GsEnk8WJtT0%3D_0zy-1dw%40mail.gmail.com Discussion: https://postgr.es/m/0498c10f-839b-4f68-9994-c29b454e55a4%40app.fastmail.com Backpatch-through: 14 --- src/backend/executor/execTuples.c | 8 +++++ src/test/regress/expected/gist.out | 54 ++++++++++++++++++++++++++++++ src/test/regress/sql/gist.sql | 38 +++++++++++++++++++++ 3 files changed, 100 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..66817c303c5 100644 --- a/src/test/regress/expected/gist.out +++ b/src/test/regress/expected/gist.out @@ -423,6 +423,60 @@ 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(). Note the first +-- tuple is returned without queueing, so any check must look past LIMIT 1. +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) + +-- no row may report the invalid-tid sentinel +select count(*) as invalid_ctids +from (select ctid from gist_knn_ctid order by p <-> point(100,4) limit 5) s +where ctid = '(4294967295,0)'::tid; + invalid_ctids +--------------- + 0 +(1 row) + +-- 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) + +-- and row locking must not be handed the invalid tid, which asserts in +-- ItemPointerIsValid() (or, in a non-assert build, tries to lock +-- InvalidBlockNumber == P_NEW and extends the relation on disk) +begin; +select count(*) as locked +from (select id from gist_knn_ctid order by p <-> point(100,4) limit 5 + for update) s; + locked +-------- + 5 +(1 row) + +rollback; +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..72c0764f499 100644 --- a/src/test/regress/sql/gist.sql +++ b/src/test/regress/sql/gist.sql @@ -198,6 +198,44 @@ 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(). Note the first +-- tuple is returned without queueing, so any check must look past LIMIT 1. +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; + +-- no row may report the invalid-tid sentinel +select count(*) as invalid_ctids +from (select ctid from gist_knn_ctid order by p <-> point(100,4) limit 5) s +where ctid = '(4294967295,0)'::tid; + +-- 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; + +-- and row locking must not be handed the invalid tid, which asserts in +-- ItemPointerIsValid() (or, in a non-assert build, tries to lock +-- InvalidBlockNumber == P_NEW and extends the relation on disk) +begin; +select count(*) as locked +from (select id from gist_knn_ctid order by p <-> point(100,4) limit 5 + for update) s; +rollback; + +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