Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Fix bug in detecting concurrent page splits in GiST insert
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Wed, 20 Jan 2021 09:58:03 +0000 (11:58 +0200)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Wed, 20 Jan 2021 09:58:27 +0000 (11:58 +0200)
In commit 9eb5607e699, I got the condition on checking for split or
deleted page wrong: I used && instead of ||. The comment correctly said
"concurrent split _or_ deletion".

As a result, GiST insertion could miss a concurrent split, and insert to
wrong page. Duncan Sands demonstrated this with a test script that did a
lot of concurrent inserts.

Backpatch to v12, where this was introduced. REINDEX is required to fix
indexes that were affected by this bug.

Backpatch-through: 12
Reported-by: Duncan Sands
Discussion: https://www.postgresql.org/message-id/a9690483-6c6c-3c82-c8ba-dc1a40848f11%40deepbluecap.com

src/backend/access/gist/gist.c

index 12d8a68c6c16c36a908310fc17800cc48d5940b0..9a5a98a61eab296d7c0a45669cab7e8dd951fd42 100644 (file)
@@ -242,6 +242,9 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
    if (GistFollowRight(page))
        elog(ERROR, "concurrent GiST page split was incomplete");
 
+   /* should never try to insert to a deleted page */
+   Assert(!GistPageIsDeleted(page));
+
    *splitinfo = NIL;
 
    /*
@@ -857,7 +860,7 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace,
                     */
                }
                else if ((GistFollowRight(stack->page) ||
-                         stack->parent->lsn < GistPageGetNSN(stack->page)) &&
+                         stack->parent->lsn < GistPageGetNSN(stack->page)) ||
                         GistPageIsDeleted(stack->page))
                {
                    /*