From 9e85b20da7cd89eb0fc503d4dd48f5fc86b45f64 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 19 Dec 2024 11:08:53 -0500 Subject: [PATCH] Avoid nbtree index scan SAOP scanBehind confusion. Consistently reset so->scanBehind at the beginning of nbtree array advancement, even during sktrig_required=false calls (calls where array advancement is triggered by an unsatisfied non-required array scan key). Otherwise, it's possible for queries to fail to return all relevant tuples to the scan given a low-order required scan key that was previously deemed "satisfied" by a truncated high key attribute value. This only happened at the point where a later non-required array scan key needed to be "advanced" once on the next leaf page (that is, once the right sibling of the truncated high key page was reached). The underlying issue was that later code within _bt_advance_array_keys assumed that the so->scanBehind flag must have been set using the current page's high key (not the previous page's high key). Any later successful recheck call to _bt_check_compare would therefore spuriously be prevented from making _bt_advance_array_keys return true, based on the faulty belief that the truncated attribute must be from the scan's current tuple (i.e. the non-pivot tuple at the start of the next page). _bt_advance_array_keys would return false for the tuple, ultimately resulting in _bt_checkkeys failing to return a matching tuple. Oversight in commit 5bf748b8, which enhanced nbtree ScalarArrayOp execution. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzkJKncfqyAUTeuB5GgRhT1vhsWO2q11dbZNqKmvjopP_g@mail.gmail.com Backpatch: 17-, where commit 5bf748b8 first appears. --- src/backend/access/nbtree/nbtutils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index 8d047d8873f..878f4b2e7b8 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -1800,6 +1800,12 @@ _bt_advance_array_keys(IndexScanDesc scan, BTReadPageState *pstate, all_required_satisfied = true, all_satisfied = true; + /* + * Unset so->scanBehind in case it is still set from back when we dealt + * with the previous page's high key/finaltup + */ + so->scanBehind = false; + if (sktrig_required) { /* @@ -1808,8 +1814,6 @@ _bt_advance_array_keys(IndexScanDesc scan, BTReadPageState *pstate, Assert(!_bt_tuple_before_array_skeys(scan, dir, tuple, tupdesc, tupnatts, false, 0, NULL)); - so->scanBehind = false; /* reset */ - /* * Required scan key wasn't satisfied, so required arrays will have to * advance. Invalidate page-level state that tracks whether the