Invalidate binary search bounds consistently.

_bt_check_unique() failed to invalidate binary search bounds in the
event of a live conflict following commit e5adcb78.  This resulted in
problems after waiting for the conflicting xact to commit or abort.  The
subsequent call to _bt_check_unique() would restore the initial binary
search bounds, rather than starting a new search.  Fix by explicitly
invalidating bounds when it becomes clear that there is a live conflict
that insertion will have to wait to resolve.

Ashutosh Sharma, with a few additional tweaks by me.

Author: Ashutosh Sharma
Reported-By: Ashutosh Sharma
Diagnosed-By: Ashutosh Sharma
Discussion: https://postgr.es/m/CAE9k0PnQp-qr-UYKMSCzdC2FBzdE4wKP41hZrZvvP26dKLonLg@mail.gmail.com

diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 96b7593fc1c22b8d69db10a60f6d74d7bd6c26d2..4fda0efe9ab400b68c8a2a48f8994328d8633210 100644 (file)
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -347,6 +347,7 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel,
     * This also saves the binary search bounds in insertstate.  We use them
     * in the fastpath below, but also in the _bt_findinsertloc() call later.
     */
+   Assert(!insertstate->bounds_valid);
    offset = _bt_binsrch_insert(rel, insertstate);
 
    /*
@@ -447,7 +448,8 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel,
                     * check, then don't bother checking if the tuple is being
                     * updated in another transaction. Just return the fact
                     * that it is a potential conflict and leave the full
-                    * check till later.
+                    * check till later. Don't invalidate binary search
+                    * bounds.
                     */
                    if (checkUnique == UNIQUE_CHECK_PARTIAL)
                    {
@@ -470,6 +472,8 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel,
                            _bt_relbuf(rel, nbuf);
                        /* Tell _bt_doinsert to wait... */
                        *speculativeToken = SnapshotDirty.speculativeToken;
+                       /* Caller releases lock on buf immediately */
+                       insertstate->bounds_valid = false;
                        return xwait;
                    }
 
@@ -526,6 +530,7 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel,
                        _bt_relbuf(rel, nbuf);
                    _bt_relbuf(rel, insertstate->buf);
                    insertstate->buf = InvalidBuffer;
+                   insertstate->bounds_valid = false;
 
                    {
                        Datum       values[INDEX_MAX_KEYS];
@@ -601,6 +606,7 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel,
            }
            maxoff = PageGetMaxOffsetNumber(page);
            offset = P_FIRSTDATAKEY(opaque);
+           /* Don't invalidate binary search bounds */
        }
    }