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 */
                }
        }