database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

Fix GIN posting tree page deletion with incomplete splits.

Browse Source 
GIN posting tree page deletion failed to consider whether the target
page's left sibling page, or the deletion target itself, was marked as
incompletely split.  Page deletion finds the target page's left sibling
by walking the parent's downlinks, but an incompletely split page's new
right half is part of the sibling chain despite having no downlink.
Deletion could therefore overwrite the rightlink of the wrong page,
disconnecting the split's still-live right half from the sibling chain.
Scans would then silently miss tuples from that page.

To fix, teach the relevant page deletion path to avoid deleting a
posting tree page whose left sibling is marked incompletely split (and
to avoid doing so when the target page itself is so marked).  This is
essentially the same approach used by nbtree page deletion.

Claude Code found this problem.  The committed test case is a simplified
version of the one that it wrote to demonstrate this bug.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAH2-Wz=sKJcn+OtfVN9rdg+Ps9e4cuQWNP-9t12UE2d8nEG90Q@mail.gmail.com
Backpatch-through: 14
This commit is contained in:
Peter Geoghegan
2026-08-22 14:50:37 -04:00
parent 739f5a947d
commit 9448c3e7ba
4 changed files with 208 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
src/backend/access/gin/ginvacuum.c
View File

@@ -152,6 +152,8 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
page = BufferGetPage(dBuffer);
rightlink = GinPageGetOpaque(page)->rightlink;
Assert(GinPageGetOpaque(BufferGetPage(lBuffer))->rightlink == deleteBlkno);
/*
* Any insert which would have gone on the leaf block will now go to its
* right sibling.
@@ -306,8 +308,21 @@ ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
if (isempty)
{
/* we never delete the left- or rightmost branch */
if (BufferIsValid(me->leftBuffer) && !GinPageRightMost(page))
/*
* Proceed to the ginDeletePage() if target page is not the leftmost
* or the rightmost page.
*
* leftBuffer is the target's left sibling according to the parent
* level, which is not necessarily its left sibling in the sibling
* link chain (the rightlinks stored on pages): the new right half of
* an incompletely split page is in the sibling chain, but has no
* downlink yet. ginDeletePage isn't prepared to deal with that, so
* we must refuse to delete when either the target or its left
* sibling page is marked incompletely split.
*/
if (BufferIsValid(me->leftBuffer) && !GinPageRightMost(page) &&
!GinPageIsIncompleteSplit(page) &&
!GinPageIsIncompleteSplit(BufferGetPage(me->leftBuffer)))
{
Assert(!isRoot);
ginDeletePage(gvs, blkno, BufferGetBlockNumber(me->leftBuffer),

2
src/test/modules/gin/Makefile
View File

@@ -1,6 +1,6 @@
# src/test/modules/gin/Makefile
EXTRA_INSTALL = src/test/modules/injection_points
EXTRA_INSTALL = src/test/modules/injection_points contrib/pageinspect
REGRESS = gin_incomplete_splits

109
src/test/modules/gin/expected/gin_incomplete_splits.out
View File

@@ -19,7 +19,6 @@ SELECT injection_points_set_local();
(1 row)
-- Use the index for all the queries
set enable_seqscan=off;
-- Print a NOTICE whenever an incomplete split gets fixed
SELECT injection_points_attach('gin-finish-incomplete-split', 'notice');
@@ -192,4 +191,112 @@ SELECT injection_points_detach('gin-finish-incomplete-split');
(1 row)
--
-- Test that VACUUM does not delete the right sibling of an incompletely
-- split posting tree leaf page
--
create extension pageinspect;
-- Create a GIN index with a posting tree that has several leaf pages
create temp table gin_posting_tree(id int4, i int4[]);
insert into gin_posting_tree select g, '{1}' from generate_series(1, 55000) g;
create index gin_posting_tree_idx on gin_posting_tree using gin (i) with (fastupdate = off);
-- Free space in the middle of the index key space/heap. The later inserts
-- will get TIDs in the middle of the posting tree's key space, splitting a
-- leaf page that is neither the leftmost nor rightmost of the posting tree.
delete from gin_posting_tree where id between 10001 and 27500;
vacuum (index_cleanup on) gin_posting_tree;
-- Insert rows until a leaf page split fails, leaving the split incomplete
SELECT injection_points_attach('gin-leave-leaf-split-incomplete', 'error');
injection_points_attach
-------------------------
(1 row)
do $$
begin
for n in 1..200000 loop
begin
insert into gin_posting_tree values (n, '{1}');
exception when others then
return;
end;
end loop;
raise 'no leaf split after 200000 inserts';
end;
$$;
SELECT injection_points_detach('gin-leave-leaf-split-incomplete');
injection_points_detach
-------------------------
(1 row)
-- Locate the incompletely split page's new right half (reachable only
-- through its left sibling's rightlink), and the leaf to the right of
-- that (the page that VACUUM will delete). Errors out unless there is
-- exactly one incomplete split.
select o.rightlink::int as righthalf,
(gin_page_opaque_info(get_raw_page('gin_posting_tree_idx',
o.rightlink::int))).rightlink::int
as nextleaf
from generate_series(0, pg_relation_size('gin_posting_tree_idx') /
current_setting('block_size')::int - 1) blkno,
lateral gin_page_opaque_info(get_raw_page('gin_posting_tree_idx',
blkno::int)) o
where o.flags @> '{incomplete_split}'
\gset
-- Sanity check: scan will miss nothing if the right half held no live rows,
-- and VACUUM never deletes the rightmost page
select exists (select from gin_posting_tree
where ctid in (select unnest(tids) from gin_leafpage_items(
get_raw_page('gin_posting_tree_idx', :righthalf))))
as righthalf_has_live_rows,
(gin_page_opaque_info(get_raw_page('gin_posting_tree_idx',
:nextleaf))).rightlink <> 4294967295
as nextleaf_is_not_rightmost;
righthalf_has_live_rows | nextleaf_is_not_rightmost
-------------------------+---------------------------
t | t
(1 row)
-- Empty the page to the right of the right half, and have VACUUM consider
-- deleting it
delete from gin_posting_tree
where ctid in (select unnest(tids) from gin_leafpage_items(
get_raw_page('gin_posting_tree_idx', :nextleaf)));
vacuum (index_cleanup on) gin_posting_tree;
-- Count the remaining rows through a bitmap scan
explain (costs off)
select count(*) as bitmapscan_count from gin_posting_tree where i @> '{1}';
QUERY PLAN
-------------------------------------------------------
Aggregate
-> Bitmap Heap Scan on gin_posting_tree
Recheck Cond: (i @> '{1}'::integer[])
-> Bitmap Index Scan on gin_posting_tree_idx
Index Cond: (i @> '{1}'::integer[])
(5 rows)
select count(*) as bitmapscan_count from gin_posting_tree where i @> '{1}'
\gset
-- Verify that a sequential scan finds the same rows
set enable_seqscan=on;
set enable_bitmapscan=off;
explain (costs off)
select count(*) from gin_posting_tree where i @> '{1}';
QUERY PLAN
-----------------------------------------
Aggregate
-> Seq Scan on gin_posting_tree
Filter: (i @> '{1}'::integer[])
(3 rows)
select count(*) = :bitmapscan_count as seqscan_agrees
from gin_posting_tree where i @> '{1}';
seqscan_agrees
----------------
t
(1 row)
drop table gin_posting_tree;
drop extension pageinspect;
drop extension injection_points;

83
src/test/modules/gin/sql/gin_incomplete_splits.sql
View File

@@ -17,7 +17,6 @@ create extension injection_points;
-- Make all injection points local to this process, for concurrency.
SELECT injection_points_set_local();
-- Use the index for all the queries
set enable_seqscan=off;
-- Print a NOTICE whenever an incomplete split gets fixed
@@ -149,4 +148,86 @@ select verify(:next_i);
SELECT injection_points_detach('gin-finish-incomplete-split');
--
-- Test that VACUUM does not delete the right sibling of an incompletely
-- split posting tree leaf page
--
create extension pageinspect;
-- Create a GIN index with a posting tree that has several leaf pages
create temp table gin_posting_tree(id int4, i int4[]);
insert into gin_posting_tree select g, '{1}' from generate_series(1, 55000) g;
create index gin_posting_tree_idx on gin_posting_tree using gin (i) with (fastupdate = off);
-- Free space in the middle of the index key space/heap. The later inserts
-- will get TIDs in the middle of the posting tree's key space, splitting a
-- leaf page that is neither the leftmost nor rightmost of the posting tree.
delete from gin_posting_tree where id between 10001 and 27500;
vacuum (index_cleanup on) gin_posting_tree;
-- Insert rows until a leaf page split fails, leaving the split incomplete
SELECT injection_points_attach('gin-leave-leaf-split-incomplete', 'error');
do $$
begin
for n in 1..200000 loop
begin
insert into gin_posting_tree values (n, '{1}');
exception when others then
return;
end;
end loop;
raise 'no leaf split after 200000 inserts';
end;
$$;
SELECT injection_points_detach('gin-leave-leaf-split-incomplete');
-- Locate the incompletely split page's new right half (reachable only
-- through its left sibling's rightlink), and the leaf to the right of
-- that (the page that VACUUM will delete). Errors out unless there is
-- exactly one incomplete split.
select o.rightlink::int as righthalf,
(gin_page_opaque_info(get_raw_page('gin_posting_tree_idx',
o.rightlink::int))).rightlink::int
as nextleaf
from generate_series(0, pg_relation_size('gin_posting_tree_idx') /
current_setting('block_size')::int - 1) blkno,
lateral gin_page_opaque_info(get_raw_page('gin_posting_tree_idx',
blkno::int)) o
where o.flags @> '{incomplete_split}'
\gset
-- Sanity check: scan will miss nothing if the right half held no live rows,
-- and VACUUM never deletes the rightmost page
select exists (select from gin_posting_tree
where ctid in (select unnest(tids) from gin_leafpage_items(
get_raw_page('gin_posting_tree_idx', :righthalf))))
as righthalf_has_live_rows,
(gin_page_opaque_info(get_raw_page('gin_posting_tree_idx',
:nextleaf))).rightlink <> 4294967295
as nextleaf_is_not_rightmost;
-- Empty the page to the right of the right half, and have VACUUM consider
-- deleting it
delete from gin_posting_tree
where ctid in (select unnest(tids) from gin_leafpage_items(
get_raw_page('gin_posting_tree_idx', :nextleaf)));
vacuum (index_cleanup on) gin_posting_tree;
-- Count the remaining rows through a bitmap scan
explain (costs off)
select count(*) as bitmapscan_count from gin_posting_tree where i @> '{1}';
select count(*) as bitmapscan_count from gin_posting_tree where i @> '{1}'
\gset
-- Verify that a sequential scan finds the same rows
set enable_seqscan=on;
set enable_bitmapscan=off;
explain (costs off)
select count(*) from gin_posting_tree where i @> '{1}';
select count(*) = :bitmapscan_count as seqscan_agrees
from gin_posting_tree where i @> '{1}';
drop table gin_posting_tree;
drop extension pageinspect;
drop extension injection_points;