Fix gistkillitems for GiST root page

GiST index killitems feature misbehaves for single-page GiST index,
i.e. one that has only a root page. This is caused by the GiST scan's
curBlkno variable not being initialized for the first-to-scan page,
which is the root page.  Fix this by moving the initializing of
curBlkno into gistScanPage(), where we also set the related curPageLSN
variable.

Commit 377b7ab145 actually added a regression test for this already,
but it merely noted that it's not working and memorized the result
where the items were not killed. Now they are, as the test shows.

This has been broken all along, but since it's just a very minor
performance issue on tiny tables, I didn't bother backpatching it.

Author: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Soumya S Murali <soumyamurali.work@gmail.com>
Discussion: https://postgr.es/m/CALdSSPgZWX_D8%2BFx4YQqRN5eW5iSx_rJdqQhCfdWTvqKXVfJ4w%40mail.gmail.com
Discussion: https://postgr.es/m/lxzj26ga6ippdeunz6kuncectr5gfuugmm2ry22qu6hcx6oid6@lzx3sjsqhmt6

diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index 4d7c100d737811403f19b61c5558363f02a00d90..e8a1e4562876738f0ed902d4192c83cb6fa5b4bf 100644 (file)
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -409,10 +409,12 @@ gistScanPage(IndexScanDesc scan, GISTSearchItem *pageItem,
                MemoryContextReset(so->pageDataCxt);
 
        /*
-        * We save the LSN of the page as we read it, so that we know whether it
-        * is safe to apply LP_DEAD hints to the page later. This allows us to
-        * drop the pin for MVCC scans, which allows vacuum to avoid blocking.
+        * Save the current page's block number for a possible gistkillitems()
+        * call later.  We also save its LSN, so that we know whether it is safe
+        * to apply the LP_DEAD hints to the page later.  This allows us to drop
+        * the pin for MVCC scans, which allows vacuum to avoid blocking.
         */
+       so->curBlkno = pageItem->blkno;
        so->curPageLSN = BufferGetLSNAtomic(buffer);
 
        /*
@@ -730,9 +732,6 @@ gistgettuple(IndexScanDesc scan, ScanDirection dir)
 
                                CHECK_FOR_INTERRUPTS();
 
-                               /* save current item BlockNumber for next gistkillitems() call */
-                               so->curBlkno = item->blkno;
-
                                /*
                                 * While scanning a leaf page, ItemPointers of matching heap
                                 * tuples are stored in so->pageData.  If there are any on

diff --git a/src/test/modules/index/expected/killtuples.out b/src/test/modules/index/expected/killtuples.out
index a3db2c409368975c95808ecac59016f55c09de3e..1d944b493c25eb86e9358a3ed9a4cc993ca4372b 100644 (file)
--- a/src/test/modules/index/expected/killtuples.out
+++ b/src/test/modules/index/expected/killtuples.out
@@ -223,7 +223,7 @@ step flush: SELECT FROM pg_stat_force_next_flush();
 step result: SELECT ((heap_blks_read + heap_blks_hit - counter.heap_accesses) > 0) AS has_new_heap_accesses FROM counter, pg_statio_all_tables WHERE relname = 'kill_prior_tuple';
 has_new_heap_accesses
 ---------------------
-t                    
+f                    
 (1 row)
 
 step drop_table: DROP TABLE IF EXISTS kill_prior_tuple;

diff --git a/src/test/modules/index/specs/killtuples.spec b/src/test/modules/index/specs/killtuples.spec
index 3b98ff9f76d8db988e7914647126f2add46063b4..59e58adf610d25fbfdf9c3df10b51401bcc427c4 100644 (file)
--- a/src/test/modules/index/specs/killtuples.spec
+++ b/src/test/modules/index/specs/killtuples.spec
@@ -96,7 +96,8 @@ permutation
   measure access flush result
   drop_table drop_ext_btree_gist
 
-# Test gist, but with fewer rows - shows that killitems doesn't work anymore!
+# Test gist, but with fewer rows (tests for an old bug where
+# gistkillitems didn't remove items on the root page)
 permutation
   create_table fill_10 create_ext_btree_gist create_gist flush
   disable_seq disable_bitmap