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
This commit is contained in:
Peter Geoghegan 2019-04-04 09:38:08 -07:00
parent 87e16db5eb
commit 74eb2176bf

View File

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