Skip to content

Commit 486c2ea

Browse files
Fix nbtree array unsatisfied inequality check.
_bt_advance_array_keys didn't take sufficient care at the point where it decides whether to start a new primitive index scan based on a call to _bt_check_compare against finaltup (a call with the scan direction flipped around). The final decision was conditioned on rules about how the scan key offset sktrig that initially triggered array advancement (passed to _bt_advance_array_keys from its _bt_checkkeys caller) compared to the offset set by its own _bt_check_compare finaltup call. This approach was faulty, in that it allowed _bt_advance_array_keys to incorrectly start a new primitive index scan, that landed on the same leaf page (on assert-enabled builds it led to an assertion failure). In general, scans with array keys are expected to never have to read the same leaf page more than once (barring cases involving cursors, and cases where the scan restores a marked position for the inner side of a merge join). This principle was established by commit 5bf748b. To fix, make the final decision based on whether the scan key offset set by the _bt_check_compare finaltup call is an offset to an inequality strategy scan key. An unsatisfied required inequality strategy scan key indicates that all of the scan's required equality strategy scan keys must also be satisfied by finaltup (not just by caller's tuple), and that there is a decent chance that _bt_first will be able to reposition the scan to a position many leaf pages ahead of the current leaf page. Oversight in commit 5bf748b. Discussion: https://postgr.es/m/CAH2-Wz=DyHbcg7o6zXqzyiin8WE8vzk4tvU8Lrnh-a=EAvO0TQ@mail.gmail.com
1 parent dbca346 commit 486c2ea

File tree

1 file changed

+10
-20
lines changed

1 file changed

+10
-20
lines changed

src/backend/access/nbtree/nbtutils.c

+10-20
Original file line numberDiff line numberDiff line change
@@ -2368,30 +2368,20 @@ _bt_advance_array_keys(IndexScanDesc scan, BTReadPageState *pstate,
23682368
&continuescanflip, &opsktrig);
23692369

23702370
/*
2371-
* If we ended up here due to the all_required_satisfied criteria,
2372-
* test opsktrig in a way that ensures that finaltup contains the same
2373-
* prefix of key columns as caller's tuple (a prefix that satisfies
2374-
* earlier required-in-current-direction scan keys).
2375-
*
2376-
* If we ended up here due to the oppodir_inequality_sktrig criteria,
2377-
* test opsktrig in a way that ensures that the same scan key that our
2378-
* caller found to be unsatisfied (by the scan's tuple) was also the
2379-
* one unsatisfied just now (by finaltup). That way we'll only start
2380-
* a new primitive scan when we're sure that both tuples _don't_ share
2381-
* the same prefix of satisfied equality-constrained attribute values,
2382-
* and that finaltup has a non-NULL attribute value indicated by the
2383-
* unsatisfied scan key at offset opsktrig/sktrig. (This depends on
2384-
* _bt_check_compare not caring about the direction that inequalities
2385-
* are required in whenever NULL attribute values are unsatisfied. It
2386-
* only cares about the scan direction, and its relationship to
2387-
* whether NULLs are stored first or last relative to non-NULLs.)
2371+
* Only start a new primitive index scan when finaltup has a required
2372+
* unsatisfied inequality (unsatisfied in the opposite direction)
23882373
*/
23892374
Assert(all_required_satisfied != oppodir_inequality_sktrig);
23902375
if (unlikely(!continuescanflip &&
2391-
((all_required_satisfied && opsktrig > sktrig) ||
2392-
(oppodir_inequality_sktrig && opsktrig >= sktrig))))
2376+
so->keyData[opsktrig].sk_strategy != BTEqualStrategyNumber))
23932377
{
2394-
Assert(so->keyData[opsktrig].sk_strategy != BTEqualStrategyNumber);
2378+
/*
2379+
* It's possible for the same inequality to be unsatisfied by both
2380+
* caller's tuple (in scan's direction) and finaltup (in the
2381+
* opposite direction) due to _bt_check_compare's behavior with
2382+
* NULLs
2383+
*/
2384+
Assert(opsktrig >= sktrig); /* not opsktrig > sktrig due to NULLs */
23952385

23962386
/*
23972387
* Make sure that any non-required arrays are set to the first

0 commit comments

Comments
 (0)