8000 Improve _bt_first row compare NULL member handling. · petergeoghegan/postgres@8af5e96 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8af5e96

Browse files
petergeogheganpetergeoghegan
committed
Improve _bt_first row compare NULL member handling.
Improve _bt_first's handling of NULL row comparison members in such a way as to make it consistent with _bt_check_rowcompare's approach. In other words, make sure that code that starts primitive index scans that involve row compares fully agrees with code that ends primitive index scans -- even in the presence of a NULL row compare member. This makes row comparisons more similar to scalar inequality keys, obviating the need for _bt_set_startikey to consider row comparisons directly. Follow-up to bugfix commit 5f4d98d, which taught _bt_set_startikey to avoid the use of forcenonrequired mode iff it reached a row compare key. Now _bt_set_startikey can treat row comparison keys/inequalities just like scalar inequalities. This makes _bt_first avoid uselessly scanning earlier index tuples that cannot possibly contain matching tuples due to the use of a NULL row compare member. The goal wasn't to improve performance, but the only other way to make this approach work is to remove _bt_check_rowcompare's long standing ability to end the scan when it compares a NULL row compare member that happens to be the second row compare member (of a row compare whose first member could be marked required).
1 parent 982b47c commit 8af5e96

File tree

2 files changed

+87
-52
lines changed

2 files changed

+87
-52
lines changed

‎src/backend/access/nbtree/nbtsearch.c

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,14 +1288,23 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
12881288
* even if the row comparison is of ">" or "<" type, because the
12891289
* condition applied to all but the last row member is effectively
12901290
* ">=" or "<=", and so the extra keys don't break the positioning
1291-
* scheme. But, by the same token, if we aren't able to use all
1292-
* the row members, then the part of the row comparison that we
1291+
* scheme. But, by the same token, if a "column gap" appears
1292+
* between members, then the part of the row comparison that we
12931293
* did use has to be treated as just a ">=" or "<=" condition, and
12941294
* so we'd better adjust strat_total accordingly.
1295+
*
1296+
* We're able to use a _more_ restrictive strategy when we
1297+
* encounter a NULL row compare member (which is unsatisfiable).
1298+
* For example, a qual "(a, b, c) >= (1, NULL, 77)" will use an
1299+
* insertion scan key "a > 1". All rows where "a = 1" have to
1300+
* perform a NULL row member comparison (or would, if we didn't
1301+
* use the more restrictive ">" strategy), which is guranteed to
1302+
* return false/NULL.
12951303
*/
12961304
if (i == keysz - 1)
12971305
{
1298-
bool used_all_subkeys = false;
1306+
bool keep_strat_total = false;
1307+
bool tighten_strat_total = false;
12991308

13001309
Assert(!(subkey->sk_flags & SK_ROW_END));
13011310
for (;;)
@@ -1307,19 +1316,26 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
13071316
if (subkey->sk_strategy != cur->sk_strategy)
13081317
break; /* wrong direction, can't use it */
13091318
if (subkey->sk_flags & SK_ISNULL)
1310-
break; /* can't use null keys */
1319+
{
1320+
/* Unsatisfiable NULL row member */
1321+
keep_strat_total = true;
1322+
tighten_strat_total = true;
1323+
break;
1324+
}
13111325
Assert(keysz < INDEX_MAX_KEYS);
13121326
memcpy(inskey.scankeys + keysz, subkey,
13131327
sizeof(ScanKeyData));
13141328
keysz++;
13151329
if (subkey->sk_flags & SK_ROW_END)
13161330
{
1317-
used_all_subkeys = true;
1331+
keep_strat_total = true;
13181332
break;
13191333
}
13201334
}
1321-
if (!used_all_subkeys)
1335+
if (!keep_strat_total)
13221336
{
1337+
/* Use less restrictive strategy (and fewer keys) */
1338+
Assert(!tighten_strat_total);
13231339
switch (strat_total)
13241340
{
13251341
case BTLessStrategyNumber:
@@ -1330,6 +1346,19 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
13301346
break;
13311347
}
13321348
}
1349+
if (tighten_strat_total)
1350+
{
1351+
/* Use more restrictive strategy (and fewer keys) */
1352+
switch (strat_total)
1353+
{
1354+
case BTLessEqualStrategyNumber:
1355+
strat_total = BTLessStrategyNumber;
1356+
break;
1357+
case BTGreaterEqualStrategyNumber:
1358+
strat_total = BTGreaterStrategyNumber;
1359+
break;
1360+
}
1361+
}
13331362
break; /* done with outer loop */
13341363
}
13351364
}

‎src/backend/access/nbtree/nbtutils.c

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2568,23 +2568,7 @@ _bt_set_startikey(IndexScanDesc scan, BTReadPageState *pstate)
25682568
* whether or not every tuple on the page satisfies a RowCompare
25692569
* key based only on firsttup and lasttup -- so we just give up.
25702570
*/
2571-
if (!start_past_saop_eq && !so->skipScan)
2572-
break; /* unsafe to go further */
2573-
2574-
/*
2575-
* We have to be even more careful with RowCompares that come
2576-
* after an array: we assume it's unsafe to even bypass the array.
2577-
* Calling _bt_start_array_keys to recover the scan's arrays
2578-
* following use of forcenonrequired mode isn't compatible with
2579-
* _bt_check_rowcompare's continuescan=false behavior with NULL
2580-
* row compare members. _bt_advance_array_keys must not make a
2581-
* decision on the basis of a key not being satisfied in the
2582-
* opposite-to-scan direction until the scan reaches a leaf page
2583-
* where the same key begins to be satisfied in scan direction.
2584-
* The _bt_first !used_all_subkeys behavior makes this limitation
2585-
* hard to work around some other way.
2586-
*/
2587-
return; /* completely unsafe to set pstate.startikey */
2571+
break; /* unsafe */
25882572
}
25892573
if (key->sk_strategy != BTEqualStrategyNumber)
25902574
{
@@ -3081,6 +3065,56 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts,
30813065

30823066
Assert(subkey->sk_flags & SK_ROW_MEMBER);
30833067

3068+
/*
3069+
* Unlike the simple-scankey case, NULL row members aren't disallowed
3070+
* (except when it's the first row element that has the NULL arg,
3071+
* where preprocessing recognizes the scan's qual as unsatisfiable).
3072+
* But it can never match any rows.
3073+
*
3074+
* If the first row member's column is marked required, and this row
3075+
* comparison is on the very next column, we can stop the scan; there
3076+
* can't be another tuple that will succeed.
3077+
*/
3078+
if (subkey->sk_flags & SK_ISNULL)
3079+
{
3080+
AttrNumber isnull_attno = subkey->sk_attno;
3081+
3082+
/* can't be the first row member (preprocessing catches this) */
3083+
Assert(subkey != (ScanKey) DatumGetPointer(skey->sk_argument));
3084+
3085+
subkey--;
3086+
if (forcenonrequired)
3087+
{
3088+
/* treating scan's keys as non-required */
3089+
}
3090+
else if (subkey->sk_attno != isnull_attno - 1)
3091+
{
3092+
/*
3093+
* There's an index column gap between the NULL row member,
3094+
* and the next most significant row member. Cannot end scan.
3095+
*
3096+
* The presence of this gap doesn't actually imply that there
3097+
* really can be another tuple that will succeed; there can't.
3098+
* This isn't really about the _current_ primitive index scan.
3099+
*
3100+
* When we set continuescan=false, it had better be safe for a
3101+
* scan with a more-significant-than-rowcompare array key to
3102+
* reposition itself to some later leaf page, in the usual way
3103+
* (by starting the next primitive index scan). But there'd
3104+
* be no way for _bt_first to actually locate some later leaf
3105+
* page if we were to set continuescan=false here -- we'd run
3106+
* the risk of getting stuck in an unending cycle.
3107+
*/
3108+
}
3109+
else if ((subkey->sk_flags & SK_BT_REQFWD) &&
3110+
ScanDirectionIsForward(dir))
3111+
*continuescan = false;
3112+
else if ((subkey->sk_flags & SK_BT_REQBKWD) &&
3113+
ScanDirectionIsBackward(dir))
3114+
*continuescan = false;
3115+
return false;
3116+
}
3117+
30843118
if (subkey->sk_attno > tupnatts)
30853119
{
30863120
/*
@@ -3090,11 +3124,7 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts,
30903124
* attribute passes the qual.
30913125
*/
30923126
Assert(BTreeTupleIsPivot(tuple));
3093-
cmpresult = 0;
3094-
if (subkey->sk_flags & SK_ROW_END)
3095-
break;
3096-
subkey++;
3097-
continue;
3127+
return true;
30983128
}
30993129

31003130
datum = index_getattr(tuple,
@@ -3151,30 +3181,6 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts,
31513181
return false;
31523182
}
31533183

3154-
if (subkey->sk_flags & SK_ISNULL)
3155-
{
3156-
/*
3157-
* Unlike the simple-scankey case, this isn't a disallowed case
3158-
* (except when it's the first row element that has the NULL arg).
3159-
* But it can never match. If all the earlier row comparison
3160-
* columns are required for the scan direction, we can stop the
3161-
* scan, because there can't be another tuple that will succeed.
3162-
*/
3163-
Assert(subkey != (ScanKey) DatumGetPointer(skey->sk_argument));
3164-
subkey--;
3165-
if (forcenonrequired)
3166-
{
3167-
/* treating scan's keys as non-required */
3168-
}
3169-
else if ((subkey->sk_flags & SK_BT_REQFWD) &&
3170-
ScanDirectionIsForward(dir))
3171-
*continuescan = false;
3172-
else if ((subkey->sk_flags & SK_BT_REQBKWD) &&
3173-
ScanDirectionIsBackward(dir))
3174-
*continuescan = false;
3175-
return false;
3176-
}
3177-
31783184
/* Perform the test --- three-way comparison not bool operator */
31793185
cmpresult = DatumGetInt32(FunctionCall2Coll(&subkey->sk_func,
31803186
subkey->sk_collation,

0 commit comments

Comments
 (0)
0