From dbfa7684a0554c0e2d33b9667ef445425f67ad94 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Mon, 23 Jun 2025 22:46:05 -0400 Subject: [PATCH v3 1/2] nbtree: Make redundant scan keys non-required. Preprocessing will now only mark one key out of a pair of known contradictory/redundant keys as required to continue the scan in corner cases where it cannot just eliminate one particular key (or end the scan by recognizing that the scan's qual is contradictory) due to a lack of suitable cross-type support. This simplifies things for all downstream users of the preprocessed scan keys. Now the scan key that is used for initial positioning purposes when scanning forward must also be the only key that is capable of ending the scan when scanning backwards. This is needed so that scans with array keys that reset their arrays in the forcenonrequired=true path (added by bugfix commit 5f4d98d4) will consistently avoid infinite cycles. It also makes scans with array keys reliably avoid duplicative leaf page accesses, which seems like a good thing to insist upon going forward, on general robustness grounds. Scan keys that are no longer marked required due to being redundant are now relocated to the end of the so->keyData[] array. That way they'll be evaluated last. This makes things more in line with what code like _bt_checkkeys and _bt_advance_array_keys generally expects. The new scheme also obviates the need for _bt_advance_array_keys to make sure that any non-required SAOP arrays have their elements reset to the first element in the current scan direction, lest the array elements be used for initial positioning purposes within _bt_first later on. Follow-up to bugfix commit 5f4d98d4, which taught _bt_set_startikey to avoid the use of forcenonrequired mode iff it reached a row compare key. Now _bt_set_startikey will be robust against infinite cycles, even with arbitrary combinations of redundant/contradictory keys that couldn't be fully eliminated by preprocessing. --- src/backend/access/nbtree/nbtpreprocesskeys.c | 349 +++++++++++++++--- src/backend/access/nbtree/nbtsearch.c | 36 +- src/backend/access/nbtree/nbtutils.c | 136 +------ 3 files changed, 331 insertions(+), 190 deletions(-) diff --git a/src/backend/access/nbtree/nbtpreprocesskeys.c b/src/backend/access/nbtree/nbtpreprocesskeys.c index a136e4bbf..855b32e83 100644 --- a/src/backend/access/nbtree/nbtpreprocesskeys.c +++ b/src/backend/access/nbtree/nbtpreprocesskeys.c @@ -16,6 +16,7 @@ #include "postgres.h" #include "access/nbtree.h" +#include "common/int.h" #include "lib/qunique.h" #include "utils/array.h" #include "utils/lsyscache.h" @@ -56,6 +57,8 @@ static void _bt_skiparray_strat_decrement(IndexScanDesc scan, ScanKey arraysk, BTArrayKeyInfo *array); static void _bt_skiparray_strat_increment(IndexScanDesc scan, ScanKey arraysk, BTArrayKeyInfo *array); +static void _bt_unmark_extra_keys(IndexScanDesc scan, int *keyDataMap); +static int _bt_reorder_array_cmp(const void *a, const void *b); static ScanKey _bt_preprocess_array_keys(IndexScanDesc scan, int *new_numberOfKeys); static void _bt_preprocess_array_keys_final(IndexScanDesc scan, int *keyDataMap); static int _bt_num_array_keys(IndexScanDesc scan, Oid *skip_eq_ops_out, @@ -96,7 +99,7 @@ static int _bt_compare_array_elements(const void *a, const void *b, void *arg); * incomplete sets of cross-type operators, we may fail to detect redundant * or contradictory keys, but we can survive that.) * - * The output keys must be sorted by index attribute. Presently we expect + * Required output keys are sorted by index attribute. Presently we expect * (but verify) that the input keys are already so sorted --- this is done * by match_clauses_to_index() in indxpath.c. Some reordering of the keys * within each attribute may be done as a byproduct of the processing here. @@ -134,22 +137,17 @@ static int _bt_compare_array_elements(const void *a, const void *b, void *arg); * cannot compare two keys for lack of a suitable cross-type operator, * we cannot eliminate either. If there are two such keys of the same * operator strategy, the second one is just pushed into the output array - * without further processing here. We may also emit both >/>= or both - * />= or both 4::int AND x > 10::bigint", and we are unable to determine - * which key is more restrictive for lack of a suitable cross-type operator. - * _bt_first will arbitrarily pick one of the keys to do the initial - * positioning with. If it picks x > 4, then the x > 10 condition will fail - * until we reach index entries > 10; but we can't stop the scan just because - * x > 10 is failing. On the other hand, if we are scanning backwards, then - * failure of either key is indeed enough to stop the scan. (In general, when - * inequality keys are present, the initial-positioning code only promises to - * position before the first possible match, not exactly at the first match, - * for a forward scan; or after the last match for a backward scan.) + * We may not be able to eliminate redundant keys. Suppose we have a qual + * like "x > 4::int AND x > 10::bigint", and we are unable to determine which + * key is more restrictive for lack of a suitable cross-type operator. We'll + * handle this by arbitrarily picking one of the keys to marked as required. + * If we pick x > 4, then the x > 10 condition will fail until we reach index + * entries > 10. That won't end the scan, because x > 10 won't be marked + * required. Note that in addition to not marking these redundant keys as + * required, we also relocate them to the end of the array. * * As a byproduct of this work, we can detect contradictory quals such * as "x = 1 AND x > 2". If we see that, we return so->qual_ok = false, @@ -193,6 +191,7 @@ _bt_preprocess_keys(IndexScanDesc scan) ScanKey arrayKeyData; int *keyDataMap = NULL; int arrayidx = 0; + bool comparisonfailed = false; if (so->numberOfKeys > 0) { @@ -388,7 +387,8 @@ _bt_preprocess_keys(IndexScanDesc scan) xform[j].inkey = NULL; xform[j].inkeyi = -1; } - /* else, cannot determine redundancy, keep both keys */ + else + comparisonfailed = true; } /* track number of attrs for which we have "=" keys */ numberOfEqualCols++; @@ -409,6 +409,8 @@ _bt_preprocess_keys(IndexScanDesc scan) else xform[BTLessStrategyNumber - 1].inkey = NULL; } + else + comparisonfailed = true; } /* try to keep only one of >, >= */ @@ -426,6 +428,8 @@ _bt_preprocess_keys(IndexScanDesc scan) else xform[BTGreaterStrategyNumber - 1].inkey = NULL; } + else + comparisonfailed = true; } /* @@ -466,25 +470,6 @@ _bt_preprocess_keys(IndexScanDesc scan) /* check strategy this key's operator corresponds to */ j = inkey->sk_strategy - 1; - /* if row comparison, push it directly to the output array */ - if (inkey->sk_flags & SK_ROW_HEADER) - { - ScanKey outkey = &so->keyData[new_numberOfKeys++]; - - memcpy(outkey, inkey, sizeof(ScanKeyData)); - if (arrayKeyData) - keyDataMap[new_numberOfKeys - 1] = i; - if (numberOfEqualCols == attno - 1) - _bt_mark_scankey_required(outkey); - - /* - * We don't support RowCompare using equality; such a qual would - * mess up the numberOfEqualCols tracking. - */ - Assert(j != (BTEqualStrategyNumber - 1)); - continue; - } - if (inkey->sk_strategy == BTEqualStrategyNumber && (inkey->sk_flags & SK_SEARCHARRAY)) { @@ -591,11 +576,6 @@ _bt_preprocess_keys(IndexScanDesc scan) * We can't determine which key is more restrictive. Push * xform[j] directly to the output array, then set xform[j] to * the new scan key. - * - * Note: We do things this way around so that our arrays are - * always in the same order as their corresponding scan keys, - * even with incomplete opfamilies. _bt_advance_array_keys - * depends on this. */ ScanKey outkey = &so->keyData[new_numberOfKeys++]; @@ -607,6 +587,7 @@ _bt_preprocess_keys(IndexScanDesc scan) xform[j].inkey = inkey; xform[j].inkeyi = i; xform[j].arrayidx = arrayidx; + comparisonfailed = true; } } } @@ -622,6 +603,16 @@ _bt_preprocess_keys(IndexScanDesc scan) if (arrayKeyData) _bt_preprocess_array_keys_final(scan, keyDataMap); + /* + * If all prior preprocessing steps failed to eliminate any "extra" keys + * due to a lack of suitable cross-type support, unset their required + * markings now. This leaves so->keyData[] with only one required > or >= + * key, and only one required < or <= key. It'll always prefer to keep a + * required = key on an attr that also has a redundant inequality. + */ + if (unlikely(comparisonfailed && so->qual_ok)) + _bt_unmark_extra_keys(scan, keyDataMap); + /* Could pfree arrayKeyData/keyDataMap now, but not worth the cycles */ } @@ -847,9 +838,6 @@ _bt_compare_scankey_args(IndexScanDesc scan, ScanKey op, cmp_op; StrategyNumber strat; - Assert(!((leftarg->sk_flags | rightarg->sk_flags) & - (SK_ROW_HEADER | SK_ROW_MEMBER))); - /* * First, deal with cases where one or both args are NULL. This should * only happen when the scankeys represent IS NULL/NOT NULL conditions. @@ -924,6 +912,13 @@ _bt_compare_scankey_args(IndexScanDesc scan, ScanKey op, return true; } + /* + * We don't yet know how to determine redundancy when it involves a row + * compare key (barring simple cases involving IS NULL/IS NOT NULL) + */ + if ((leftarg->sk_flags | rightarg->sk_flags) & SK_ROW_HEADER) + return false; + /* * If either leftarg or rightarg are equality-type array scankeys, we need * specialized handling (since by now we know that IS NULL wasn't used) @@ -1467,6 +1462,272 @@ _bt_skiparray_strat_increment(IndexScanDesc scan, ScanKey arraysk, } } +/* + * _bt_unmark_extra_keys() -- make "extra" so->keyData[] keys nonrequired. + * + * When _bt_preprocess_keys failed to eliminate one or more contradictory + * keys, it calls here to make sure that there is no more than one > or >= key + * marked required, and no more than one < or <= key marked required. That + * way _bt_first and _bt_checkkeys will reliably have exactly the same ideas + * about which keys to use to start and end the scan. This is important + * during scans with array keys, where _bt_checkkeys/_bt_advance_array_keys + * relies on _bt_first to reposition the scan when scheduling a new primscan. + * + * We also relocate "extra" keys that were unmarked required to the end of the + * so->keyData[] array. This is for the benefit of _bt_advance_array_keys, + * which expects to be able to test if the scan's keys would end the scan were + * the scan direction to change. This is used to test whether _bt_first is + * able to relocate the scan to a later leaf page; if an inequality would end + * the scan if the scan direction were reversed, then _bt_first will be able + * to apply the same inequality to relocate the scan to a later leaf page + * (even when the current leaf page satisfies the scan's current = keys, + * including those used by one or more = arrays). + * + * Only call here when _bt_compare_scankey_args returned false at least once + * (otherwise, calling here will just waste cycles). We always unmark at + * least one key per call to _bt_compare_scankey_args that reported failure. + */ +static void +_bt_unmark_extra_keys(IndexScanDesc scan, int *keyDataMap) +{ + BTScanOpaque so = (BTScanOpaque) scan->opaque; + AttrNumber curattr; + ScanKey cur; + bool *unmarkikey; + int nunmark, + nunmarked, + nkept, + firsti; + ScanKey keepKeys, + unmarkKeys; + FmgrInfo *keepOrderProcs = NULL, + *unmarkOrderProcs = NULL; + bool haveEquals, + haveReqForward, + haveReqBackward; + + /* + * Do an initial pass over so->keyData[] that determines which keys to + * keeo as required, and which to mark non-required. + * + * When both equality and inequality keys remain on a single attribute, we + * *must* leave only the equality key as required to continue the scan. + * That way _bt_first will use the key for initial positioning purposes. + * + * This isn't optional. _bt_checkkeys() will stop the scan as soon as an + * equality qual fails. For example, if _bt_first were allowed to start + * at x=4 given a qual "x >= 4 AND x = 10", it would fail and stop before + * reaching x=10. If multiple equality quals on the same attribute have + * survived all prior preprocessing steps, we'll arbitrarily allow only + * one to remain marked required. + */ + unmarkikey = palloc0(so->numberOfKeys * sizeof(bool)); + nunmark = 0; + + /* Set things up for first key's attr */ + cur = so->keyData; + curattr = cur->sk_attno; + firsti = 0; + haveEquals = false; + haveReqForward = false; + haveReqBackward = false; + for (int i = 0; i < so->numberOfKeys; cur++, i++) + { + if (cur->sk_attno != curattr) + { + /* Reset for next attr */ + curattr = cur->sk_attno; + firsti = i; + + haveEquals = false; + haveReqForward = false; + haveReqBackward = false; + } + + /* First consider equalities */ + if (haveEquals) + { + /* + * We've already found the first "=" key for attr. We already + * decided that every other key on the same attr is to be unmarked + */ + Assert(!(cur->sk_flags & SK_SEARCHNULL)); + unmarkikey[i] = true; + nunmark++; + continue; + } + else if ((cur->sk_flags & SK_BT_REQFWD) && + (cur->sk_flags & SK_BT_REQBKWD)) + { + /* + * Found the first "=" key for attr. All other keys on the same + * attr will be unmarked. + */ + haveEquals = true; + for (int j = firsti; j < i; j++) + { + /* We'll unmark prior keys for the same attr after all */ + if (!unmarkikey[j]) + { + unmarkikey[j] = true; + nunmark++; + } + } + continue; + } + + /* Next consider inequalities */ + if ((cur->sk_flags & SK_BT_REQFWD) && !haveReqForward) + { + haveReqForward = true; + continue; + } + else if ((cur->sk_flags & SK_BT_REQBKWD) && !haveReqBackward) + { + haveReqBackward = true; + continue; + } + unmarkikey[i] = true; + nunmark++; + } + + /* + * We're only supposed to be called when _bt_compare_scankey_args reported + * failure when called from the main _bt_preprocess_keys loop + */ + Assert(nunmark > 0); + + /* + * Next, allocate temp arrays: one set for unchanged keys, another for + * keys that will be unmarked/made non-required + */ + unmarkKeys = palloc(so->numberOfKeys * sizeof(ScanKeyData)); + keepKeys = palloc(so->numberOfKeys * sizeof(ScanKeyData)); + nunmarked = 0; + nkept = 0; + if (so->numArrayKeys) + { + unmarkOrderProcs = palloc(so->numberOfKeys * sizeof(FmgrInfo)); + keepOrderProcs = palloc(so->numberOfKeys * sizeof(FmgrInfo)); + } + + /* + * Next, copy the contents of so->keyData[] into the appropriate temp + * array. + * + * Note: scans with array keys need us to maintain a mapping of the old + * so->keyData[] positions to the new ones. This is similar to what + * already just happened in _bt_preprocess_array_keys_final. + */ + for (int origikey = 0; origikey < so->numberOfKeys; origikey++) + { + ScanKey origkey = so->keyData + origikey; + ScanKey unmark; + + if (!unmarkikey[origikey]) + { + /* non-redundant key stays at the start of so->keyData[] */ + memcpy(keepKeys + nkept, origkey, sizeof(ScanKeyData)); + + if (so->numArrayKeys) + { + keyDataMap[origikey] = nkept; + memcpy(keepOrderProcs + nkept, &so->orderProcs[origikey], + sizeof(FmgrInfo)); + } + + nkept++; + continue; + } + + /* key will be unmarked */ + unmark = unmarkKeys + nunmarked; + memcpy(unmark, origkey, sizeof(ScanKeyData)); + + if (so->numArrayKeys) + { + keyDataMap[origikey] = (so->numberOfKeys - nunmark) + nunmarked; + memcpy(&unmarkOrderProcs[nunmarked], &so->orderProcs[origikey], + sizeof(FmgrInfo)); + } + + nunmarked++; + + /* clear requiredness flags on redundant key (and its subkeys) */ + unmark->sk_flags &= ~(SK_BT_REQFWD | SK_BT_REQBKWD); + if (unmark->sk_flags & SK_ROW_HEADER) + { + ScanKey subkey = (ScanKey) DatumGetPointer(origkey->sk_argument); + + Assert(subkey->sk_strategy == origkey->sk_strategy); + for (;;) + { + Assert(subkey->sk_flags & SK_ROW_MEMBER); + subkey->sk_flags &= ~(SK_BT_REQFWD | SK_BT_REQBKWD); + if (subkey->sk_flags & SK_ROW_END) + break; + subkey++; + } + } + } + + /* + * Copy temp arrays back into so->keyData[] + */ + Assert(nkept == so->numberOfKeys - nunmark); + Assert(nunmarked == nunmark); + memcpy(so->keyData, keepKeys, sizeof(ScanKeyData) * nkept); + memcpy(so->keyData + nkept, unmarkKeys, sizeof(ScanKeyData) * nunmarked); + + /* Done with temp arrays */ + pfree(unmarkikey); + pfree(keepKeys); + pfree(unmarkKeys); + + /* + * Now copy temp entries needed by scans with = array keys back into + * so->orderProcs[]. The order needs to continue to match the new order + * used by so->keyData[]. + */ + if (so->numArrayKeys) + { + memcpy(so->orderProcs, keepOrderProcs, sizeof(FmgrInfo) * nkept); + memcpy(so->orderProcs + nkept, unmarkOrderProcs, + sizeof(FmgrInfo) * nunmarked); + + /* Also fix-up array->scan_key references */ + for (int arridx = 0; arridx < so->numArrayKeys; arridx++) + { + BTArrayKeyInfo *array = &so->arrayKeys[arridx]; + + array->scan_key = keyDataMap[array->scan_key]; + } + + /* + * Also make sure that the scan's arrays appear in the expected order. + * This must match corresponding scan key/so->orderProcs[] entries. + */ + qsort(so->arrayKeys, so->numArrayKeys, sizeof(BTArrayKeyInfo), + _bt_reorder_array_cmp); + + /* Done with temp arrays */ + pfree(unmarkOrderProcs); + pfree(keepOrderProcs); + } +} + +/* + * qsort comparator for reordering so->arrayKeys[] BTArrayKeyInfo entries + */ +static int +_bt_reorder_array_cmp(const void *a, const void *b) +{ + BTArrayKeyInfo *arraya = (BTArrayKeyInfo *) a; + BTArrayKeyInfo *arrayb = (BTArrayKeyInfo *) b; + + return pg_cmp_s32(arraya->scan_key, arrayb->scan_key); +} + /* * _bt_preprocess_array_keys() -- Preprocess SK_SEARCHARRAY scan keys * diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index 36544ecfd..a40b0cf81 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -971,22 +971,13 @@ _bt_first(IndexScanDesc scan, ScanDirection dir) * of what initial positioning strategy to use. * * When the scan keys include cross-type operators, _bt_preprocess_keys - * may not be able to eliminate redundant keys; in such cases we will - * arbitrarily pick a usable one for each attribute. This is correct - * but possibly not optimal behavior. (For example, with keys like - * "x >= 4 AND x >= 5" we would elect to scan starting at x=4 when - * x=5 would be more efficient.) Since the situation only arises given - * a poorly-worded query plus an incomplete opfamily, live with it. - * - * When both equality and inequality keys appear for a single attribute - * (again, only possible when cross-type operators appear), we *must* - * select one of the equality keys for the starting point, because - * _bt_checkkeys() will stop the scan as soon as an equality qual fails. - * For example, if we have keys like "x >= 4 AND x = 10" and we elect to - * start at x=4, we will fail and stop before reaching x=10. If multiple - * equality quals survive preprocessing, however, it doesn't matter which - * one we use --- by definition, they are either redundant or - * contradictory. + * may not be able to eliminate redundant keys; in such cases it will + * arbitrarily pick a usable one for each attribute, making sure that the + * other key isn't marked required to continue the scan. We only use keys + * that were marked required (perhaps _only_ marked required in the scan + * direction opposite our own) here. There's no risk that we'll get + * confused about which key to use, since _bt_preprocess_keys will also + * relocate the other key to the end of so->keyData[]. * * In practice we rarely see any "attribute boundary key gaps" here. * Preprocessing can usually backfill skip array keys for any attributes @@ -996,10 +987,6 @@ _bt_first(IndexScanDesc scan, ScanDirection dir) * This happens with range skip arrays, which store inequality keys in the * array's low_compare/high_compare fields (used to find the first/last * set of matches, when = key will lack a usable sk_argument value). - * These are always preferred over any redundant "standard" inequality - * keys on the same column (per the usual rule about preferring = keys). - * Note also that any column with an = skip array key can never have an - * additional, contradictory = key. * * All keys (with the exception of SK_SEARCHNULL keys and SK_BT_SKIP * array keys whose array is "null_elem=true") imply a NOT NULL qualifier. @@ -1012,7 +999,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir) * * In this loop, row-comparison keys are treated the same as keys on their * first (leftmost) columns. We'll add on lower-order columns of the row - * comparison below, if possible. + * comparison below. * * The selected scan keys (at most one per index column) are remembered by * storing their addresses into the local startKeys[] array. @@ -1196,6 +1183,13 @@ _bt_first(IndexScanDesc scan, ScanDirection dir) impliesNN = NULL; } + /* + * Keys that are not marked required in either scan direction + * aren't eligible to be an initial positioning key + */ + if ((cur->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD)) == 0) + continue; + /* * Can we use this key as a starting boundary for this attr? * diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index c71d1b6f2..eb6dbfda3 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -44,7 +44,6 @@ static bool _bt_array_decrement(Relation rel, ScanKey skey, BTArrayKeyInfo *arra static bool _bt_array_increment(Relation rel, ScanKey skey, BTArrayKeyInfo *array); static bool _bt_advance_array_keys_increment(IndexScanDesc scan, ScanDirection dir, bool *skip_array_set); -static void _bt_rewind_nonrequired_arrays(IndexScanDesc scan, ScanDirection dir); static bool _bt_tuple_before_array_skeys(IndexScanDesc scan, ScanDirection dir, IndexTuple tuple, TupleDesc tupdesc, int tupnatts, bool readpagetup, int sktrig, bool *scanBehind); @@ -52,7 +51,6 @@ static bool _bt_advance_array_keys(IndexScanDesc scan, BTReadPageState *pstate, IndexTuple tuple, int tupnatts, TupleDesc tupdesc, int sktrig, bool sktrig_required); #ifdef USE_ASSERT_CHECKING -static bool _bt_verify_arrays_bt_first(IndexScanDesc scan, ScanDirection dir); static bool _bt_verify_keys_with_arraykeys(IndexScanDesc scan); #endif static bool _bt_oppodir_checkkeys(IndexScanDesc scan, ScanDirection dir, @@ -1034,73 +1032,6 @@ _bt_advance_array_keys_increment(IndexScanDesc scan, ScanDirection dir, return false; } -/* - * _bt_rewind_nonrequired_arrays() -- Rewind SAOP arrays not marked required - * - * Called when _bt_advance_array_keys decides to start a new primitive index - * scan on the basis of the current scan position being before the position - * that _bt_first is capable of repositioning the scan to by applying an - * inequality operator required in the opposite-to-scan direction only. - * - * Although equality strategy scan keys (for both arrays and non-arrays alike) - * are either marked required in both directions or in neither direction, - * there is a sense in which non-required arrays behave like required arrays. - * With a qual such as "WHERE a IN (100, 200) AND b >= 3 AND c IN (5, 6, 7)", - * the scan key on "c" is non-required, but nevertheless enables positioning - * the scan at the first tuple >= "(100, 3, 5)" on the leaf level during the - * first descent of the tree by _bt_first. Later on, there could also be a - * second descent, that places the scan right before tuples >= "(200, 3, 5)". - * _bt_first must never be allowed to build an insertion scan key whose "c" - * entry is set to a value other than 5, the "c" array's first element/value. - * (Actually, it's the first in the current scan direction. This example uses - * a forward scan.) - * - * Calling here resets the array scan key elements for the scan's non-required - * arrays. This is strictly necessary for correctness in a subset of cases - * involving "required in opposite direction"-triggered primitive index scans. - * Not all callers are at risk of _bt_first using a non-required array like - * this, but advancement always resets the arrays when another primitive scan - * is scheduled, just to keep things simple. Array advancement even makes - * sure to reset non-required arrays during scans that have no inequalities. - * (Advancement still won't call here when there are no inequalities, though - * that's just because it's all handled indirectly instead.) - * - * Note: _bt_verify_arrays_bt_first is called by an assertion to enforce that - * everybody got this right. - * - * Note: In practice almost all SAOP arrays are marked required during - * preprocessing (if necessary by generating skip arrays). It is hardly ever - * truly necessary to call here, but consistently doing so is simpler. - */ -static void -_bt_rewind_nonrequired_arrays(IndexScanDesc scan, ScanDirection dir) -{ - Relation rel = scan->indexRelation; - BTScanOpaque so = (BTScanOpaque) scan->opaque; - int arrayidx = 0; - - for (int ikey = 0; ikey < so->numberOfKeys; ikey++) - { - ScanKey cur = so->keyData + ikey; - BTArrayKeyInfo *array = NULL; - - if (!(cur->sk_flags & SK_SEARCHARRAY) || - cur->sk_strategy != BTEqualStrategyNumber) - continue; - - array = &so->arrayKeys[arrayidx++]; - Assert(array->scan_key == ikey); - - if ((cur->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD))) - continue; - - Assert(array->num_elems != -1); /* No non-required skip arrays */ - - _bt_array_set_low_or_high(rel, cur, array, - ScanDirectionIsForward(dir)); - } -} - /* * _bt_tuple_before_array_skeys() -- too early to advance required arrays? * @@ -1380,8 +1311,6 @@ _bt_start_prim_scan(IndexScanDesc scan, ScanDirection dir) */ if (so->needPrimScan) { - Assert(_bt_verify_arrays_bt_first(scan, dir)); - /* * Flag was set -- must call _bt_first again, which will reset the * scan's needPrimScan flag @@ -2007,14 +1936,7 @@ _bt_advance_array_keys(IndexScanDesc scan, BTReadPageState *pstate, */ else if (has_required_opposite_direction_only && pstate->finaltup && unlikely(!_bt_oppodir_checkkeys(scan, dir, pstate->finaltup))) - { - /* - * Make sure that any SAOP arrays that were not marked required by - * preprocessing are reset to their first element for this direction - */ - _bt_rewind_nonrequired_arrays(scan, dir); goto new_prim_scan; - } continue_scan: @@ -2045,8 +1967,6 @@ continue_scan: */ so->oppositeDirCheck = has_required_opposite_direction_only; - _bt_rewind_nonrequired_arrays(scan, dir); - /* * skip by setting "look ahead" mechanism's offnum for forwards scans * (backwards scans check scanBehind flag directly instead) @@ -2142,48 +2062,6 @@ end_toplevel_scan: } #ifdef USE_ASSERT_CHECKING -/* - * Verify that the scan's qual state matches what we expect at the point that - * _bt_start_prim_scan is about to start a just-scheduled new primitive scan. - * - * We enforce a rule against non-required array scan keys: they must start out - * with whatever element is the first for the scan's current scan direction. - * See _bt_rewind_nonrequired_arrays comments for an explanation. - */ -static bool -_bt_verify_arrays_bt_first(IndexScanDesc scan, ScanDirection dir) -{ - BTScanOpaque so = (BTScanOpaque) scan->opaque; - int arrayidx = 0; - - for (int ikey = 0; ikey < so->numberOfKeys; ikey++) - { - ScanKey cur = so->keyData + ikey; - BTArrayKeyInfo *array = NULL; - int first_elem_dir; - - if (!(cur->sk_flags & SK_SEARCHARRAY) || - cur->sk_strategy != BTEqualStrategyNumber) - continue; - - array = &so->arrayKeys[arrayidx++]; - - if (((cur->sk_flags & SK_BT_REQFWD) && ScanDirectionIsForward(dir)) || - ((cur->sk_flags & SK_BT_REQBKWD) && ScanDirectionIsBackward(dir))) - continue; - - if (ScanDirectionIsForward(dir)) - first_elem_dir = 0; - else - first_elem_dir = array->num_elems - 1; - - if (array->cur_elem != first_elem_dir) - return false; - } - - return _bt_verify_keys_with_arraykeys(scan); -} - /* * Verify that the scan's "so->keyData[]" scan keys are in agreement with * its array key state @@ -2194,6 +2072,7 @@ _bt_verify_keys_with_arraykeys(IndexScanDesc scan) BTScanOpaque so = (BTScanOpaque) scan->opaque; int last_sk_attno = InvalidAttrNumber, arrayidx = 0; + bool nonrequiredseen = false; if (!so->qual_ok) return false; @@ -2217,8 +2096,16 @@ _bt_verify_keys_with_arraykeys(IndexScanDesc scan) if (array->num_elems != -1 && cur->sk_argument != array->elem_values[array->cur_elem]) return false; - if (last_sk_attno > cur->sk_attno) - return false; + if (cur->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD)) + { + if (last_sk_attno > cur->sk_attno) + return false; + if (nonrequiredseen) + return false; + } + else + nonrequiredseen = true; + last_sk_attno = cur->sk_attno; } @@ -2551,7 +2438,6 @@ _bt_set_startikey(IndexScanDesc scan, BTReadPageState *pstate) if (!(key->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD))) { /* Scan key isn't marked required (corner case) */ - Assert(!(key->sk_flags & SK_ROW_HEADER)); break; /* unsafe */ } if (key->sk_flags & SK_ROW_HEADER) -- 2.50.0