Attached patch adds a failing test case that shows wrong answers, authored by Opus 5. The failures stem from GiST not invalidating killedItems/numKilled across rescans: it's possible for the wrong index tuples to have their LP_DEAD bits set due to state from a previous rescan (namely killedItems and numKilled) getting mixed together with state from the current rescan ( namely curBlkno and curPageLSN). At a minimum, we should invalidate the gistkillitems-related state across rescans.
Apparently the test case makes GiST mark index tuples that contain downlinks in internal pages LP_DEAD, which is enough to make gistScanPage() completely ignore whole subtrees. This is possible even though such an index tuple should never be LP_DEAD marked in general (obviously only tuples that point to heap TIDs should ever get LP_DEAD-marked). gistScanPage has fairly uniform handling for leaf pages and internal pages, which is what makes the "ignore whole subtree" misbehavior possible. This is *probably* only a bug on master, since only master has commit 9c9ddf109, which taught gistkillitems to run against a single-page GiST index. However, we don't correctly reset the state on a rescan in any version, which seems dicey even without commit 9c9ddf109. I think that the proper fix here is have GiST deal with gistkillitems in the same way that nbtree deals with _bt_killitems: gistkillitems should be called in gistrescan and in gistendscan (not just in gistgettuple), so that we correctly invalidate the state, *without* neglecting to LP_DEAD-mark index tuples that really are dead-to-all. -- Peter Geoghegan
0001-Add-a-test-for-GiST-killtuples-state-left-over-by-a-.patch
Description: Binary data
