Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbd3f59f

Browse files
Make row compares robust during nbtree array scans.
Recent nbtree bugfix commit5f4d98d added a special case to the codethat sets up a page-level prefix of keys that are definitely satisfiedby every tuple on the page: whenever _bt_set_startikey reached a rowcompare key, we'd refuse to apply the pstate.forcenonrequired behaviorin scans where that usually happens (scans with a higher-order arraykey). That hack made the scan avoid essentially the same infinitecycling behavior that also affected nbtree scans with redundant keys(keys that preprocessing could not eliminate) prior to commitf09816a.There are now serious doubts about this row compare workaround.Testing has shown that a scan with a row compare key and an array keycould still read the same leaf page twice (without the scan's directionchanging), which isn't supposed to be possible following the SAOPenhancements added by Postgres 17 commit5bf748b. Also, we stillallowed a required row compare key to be used with forcenonrequired modewhen its header key happened to be beyond the pstate.ikey set by_bt_set_startikey, which was complicated and brittle.The underlying problem was that row compares had inconsistent rulesaround how scans start (which keys can be used for initial positioningpurposes) and how scans end (which keys can set continuescan=false).Quals with redundant keys that could not be eliminated by preprocessingalso had that same quality to them prior to today's bugfixf09816a. Itnow seems prudent to bring row compare keys in line with the new charterfor required keys, by making the start and end rules symmetric.This commit fixes two points of disagreement between _bt_first and_bt_check_rowcompare. Firstly, _bt_check_rowcompare was capable ofending the scan at the point where it needed to compare an ISNULL-markedrow compare member that came immediately after a required row comparemember. _bt_first now has symmetric handling for NULL row compares.Secondly, _bt_first had its own ideas about which keys were safe to usefor initial positioning purposes. It could use fewer or more keys than_bt_check_rowcompare. _bt_first now uses the same requiredness markingsas _bt_check_rowcompare for this.Now that _bt_first and _bt_check_rowcompare agree on how to start andend scans, we can get rid of the forcenonrequired special case, withoutany risk of infinite cycling. This approach also makes row compare keysbehave more like regular scalar keys, particularly within _bt_first.Fixing these inconsistencies necessitates dealing with a related issuewith the way that row compares were marked required by preprocessing: wedidn't mark any lower-order row members required following 2016 bugfixcommita298a1e. That approach was over broad. The bug in question wasactually an oversight in how _bt_check_rowcompare dealt with tuple NULLvalues that failed to satisfy a scan key marked required in the oppositescan direction (it was a bug in 2011 commits6980f81 and882368e, nota bug in 2006 commit3a0a16c). Go back to marking row compare membersas required using the original 2006 rules, and fix the 2016 bug in amore principled way: by limiting use of the "set continuescan=false witha key required in the opposite scan direction upon encountering a NULLtuple value" optimization to the first/most significant row member key.While it isn't safe to use an implied IS NOT NULL qualifier to end thescan when it comes from a required lower-order row compare member key,it _is_ generally safe for such a required member key to end the scan --provided the key is marked required in the _current_ scan direction.This fixes what was arguably an oversight in either commit5f4d98d orcommit8a51027. It is a direct follow-up to today's commitf09816a.Author: Peter Geoghegan <pg@bowt.ie>Reviewed-By: Heikki Linnakangas <heikki.linnakangas@iki.fi>Discussion:https://postgr.es/m/CAH2-Wz=pcijHL_mA0_TJ5LiTB28QpQ0cGtT-ccFV=KzuunNDDQ@mail.gmail.comBackpatch-through: 18
1 parentf09816a commitbd3f59f

File tree

5 files changed

+385
-202
lines changed

5 files changed

+385
-202
lines changed

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -792,12 +792,25 @@ _bt_mark_scankey_required(ScanKey skey)
792792
if (skey->sk_flags&SK_ROW_HEADER)
793793
{
794794
ScanKeysubkey= (ScanKey)DatumGetPointer(skey->sk_argument);
795+
AttrNumberattno=skey->sk_attno;
795796

796797
/* First subkey should be same column/operator as the header */
797-
Assert(subkey->sk_flags&SK_ROW_MEMBER);
798-
Assert(subkey->sk_attno==skey->sk_attno);
798+
Assert(subkey->sk_attno==attno);
799799
Assert(subkey->sk_strategy==skey->sk_strategy);
800-
subkey->sk_flags |=addflags;
800+
801+
for (;;)
802+
{
803+
Assert(subkey->sk_flags&SK_ROW_MEMBER);
804+
if (subkey->sk_attno!=attno)
805+
break;/* non-adjacent key, so not required */
806+
if (subkey->sk_strategy!=skey->sk_strategy)
807+
break;/* wrong direction, so not required */
808+
subkey->sk_flags |=addflags;
809+
if (subkey->sk_flags&SK_ROW_END)
810+
break;
811+
subkey++;
812+
attno++;
813+
}
801814
}
802815
}
803816

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

Lines changed: 143 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,8 +1016,8 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
10161016
* traversing a lot of null entries at the start of the scan.
10171017
*
10181018
* In this loop, row-comparison keys are treated the same as keys on their
1019-
* first (leftmost) columns. We'll addon lower-order columns of the row
1020-
* comparisonbelow, if possible.
1019+
* first (leftmost) columns. We'll addall lower-order columns of the row
1020+
* comparisonthat were marked required during preprocessing below.
10211021
*
10221022
* _bt_advance_array_keys needs to know exactly how we'll reposition the
10231023
* scan (should it opt to schedule another primitive index scan). It is
@@ -1261,139 +1261,179 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
12611261
Assert(keysz <=INDEX_MAX_KEYS);
12621262
for (inti=0;i<keysz;i++)
12631263
{
1264-
ScanKeycur=startKeys[i];
1264+
ScanKeybkey=startKeys[i];
12651265

1266-
Assert(cur->sk_attno==i+1);
1266+
Assert(bkey->sk_attno==i+1);
12671267

1268-
if (cur->sk_flags&SK_ROW_HEADER)
1268+
if (bkey->sk_flags&SK_ROW_HEADER)
12691269
{
12701270
/*
12711271
* Row comparison header: look to the first row member instead
12721272
*/
1273-
ScanKeysubkey= (ScanKey)DatumGetPointer(cur->sk_argument);
1273+
ScanKeysubkey= (ScanKey)DatumGetPointer(bkey->sk_argument);
1274+
boolloosen_strat= false,
1275+
tighten_strat= false;
12741276

12751277
/*
12761278
* Cannot be a NULL in the first row member: _bt_preprocess_keys
12771279
* would've marked the qual as unsatisfiable, preventing us from
12781280
* ever getting this far
12791281
*/
12801282
Assert(subkey->sk_flags&SK_ROW_MEMBER);
1281-
Assert(subkey->sk_attno==cur->sk_attno);
1283+
Assert(subkey->sk_attno==bkey->sk_attno);
12821284
Assert(!(subkey->sk_flags&SK_ISNULL));
12831285

1286+
/*
1287+
* This is either a > or >= key (during backwards scans it is
1288+
* either < or <=) that was marked required during preprocessing.
1289+
* Later so->keyData[] keys can't have been marked required, so
1290+
* our row compare header key must be the final startKeys[] entry.
1291+
*/
1292+
Assert(subkey->sk_flags& (SK_BT_REQFWD |SK_BT_REQBKWD));
1293+
Assert(i==keysz-1);
1294+
12841295
/*
12851296
* The member scankeys are already in insertion format (ie, they
12861297
* have sk_func = 3-way-comparison function)
12871298
*/
12881299
memcpy(inskey.scankeys+i,subkey,sizeof(ScanKeyData));
12891300

12901301
/*
1291-
* If the row comparison is the last positioning key we accepted,
1292-
* try to add additional keys from the lower-order row members.
1293-
* (If we accepted independent conditions on additional index
1294-
* columns, we use those instead --- doesn't seem worth trying to
1295-
* determine which is more restrictive.) Note that this is OK
1296-
* even if the row comparison is of ">" or "<" type, because the
1297-
* condition applied to all but the last row member is effectively
1298-
* ">=" or "<=", and so the extra keys don't break the positioning
1299-
* scheme. But, by the same token, if we aren't able to use all
1300-
* the row members, then the part of the row comparison that we
1301-
* did use has to be treated as just a ">=" or "<=" condition, and
1302-
* so we'd better adjust strat_total accordingly.
1302+
* Now look to later row compare members.
1303+
*
1304+
* If there's an "index attribute gap" between two row compare
1305+
* members, the second member won't have been marked required, and
1306+
* so can't be used as a starting boundary key here. The part of
1307+
* the row comparison that we do still use has to be treated as a
1308+
* ">=" or "<=" condition. For example, a qual "(a, c) > (1, 42)"
1309+
* with an omitted intervening index attribute "b" will use an
1310+
* insertion scan key "a >= 1". Even the first "a = 1" tuple on
1311+
* the leaf level might satisfy the row compare qual.
1312+
*
1313+
* We're able to use a _more_ restrictive strategy when we reach a
1314+
* NULL row compare member, since they're always unsatisfiable.
1315+
* For example, a qual "(a, b, c) >= (1, NULL, 77)" will use an
1316+
* insertion scan key "a > 1". All tuples where "a = 1" cannot
1317+
* possibly satisfy the row compare qual, so this is safe.
13031318
*/
1304-
if (i==keysz-1)
1319+
Assert(!(subkey->sk_flags&SK_ROW_END));
1320+
for (;;)
13051321
{
1306-
boolused_all_subkeys= false;
1322+
subkey++;
1323+
Assert(subkey->sk_flags&SK_ROW_MEMBER);
13071324

1308-
Assert(!(subkey->sk_flags&SK_ROW_END));
1309-
for (;;)
1325+
if (subkey->sk_flags&SK_ISNULL)
13101326
{
1311-
subkey++;
1312-
Assert(subkey->sk_flags&SK_ROW_MEMBER);
1313-
if (subkey->sk_attno!=keysz+1)
1314-
break;/* out-of-sequence, can't use it */
1315-
if (subkey->sk_strategy!=cur->sk_strategy)
1316-
break;/* wrong direction, can't use it */
1317-
if (subkey->sk_flags&SK_ISNULL)
1318-
break;/* can't use null keys */
1319-
Assert(keysz<INDEX_MAX_KEYS);
1320-
memcpy(inskey.scankeys+keysz,subkey,
1321-
sizeof(ScanKeyData));
1322-
keysz++;
1323-
if (subkey->sk_flags&SK_ROW_END)
1324-
{
1325-
used_all_subkeys= true;
1326-
break;
1327-
}
1327+
/*
1328+
* NULL member key, can only use earlier keys.
1329+
*
1330+
* We deliberately avoid checking if this key is marked
1331+
* required. All earlier keys are required, and this key
1332+
* is unsatisfiable either way, so we can't miss anything.
1333+
*/
1334+
tighten_strat= true;
1335+
break;
13281336
}
1329-
if (!used_all_subkeys)
1337+
1338+
if (!(subkey->sk_flags& (SK_BT_REQFWD |SK_BT_REQBKWD)))
13301339
{
1331-
switch (strat_total)
1332-
{
1333-
caseBTLessStrategyNumber:
1334-
strat_total=BTLessEqualStrategyNumber;
1335-
break;
1336-
caseBTGreaterStrategyNumber:
1337-
strat_total=BTGreaterEqualStrategyNumber;
1338-
break;
1339-
}
1340+
/* nonrequired member key, can only use earlier keys */
1341+
loosen_strat= true;
1342+
break;
13401343
}
1341-
break;/* done with outer loop */
1344+
1345+
Assert(subkey->sk_attno==keysz+1);
1346+
Assert(subkey->sk_strategy==bkey->sk_strategy);
1347+
Assert(keysz<INDEX_MAX_KEYS);
1348+
1349+
memcpy(inskey.scankeys+keysz,subkey,
1350+
sizeof(ScanKeyData));
1351+
keysz++;
1352+
if (subkey->sk_flags&SK_ROW_END)
1353+
break;
13421354
}
1343-
}
1344-
else
1345-
{
1346-
/*
1347-
* Ordinary comparison key. Transform the search-style scan key
1348-
* to an insertion scan key by replacing the sk_func with the
1349-
* appropriate btree comparison function.
1350-
*
1351-
* If scankey operator is not a cross-type comparison, we can use
1352-
* the cached comparison function; otherwise gotta look it up in
1353-
* the catalogs. (That can't lead to infinite recursion, since no
1354-
* indexscan initiated by syscache lookup will use cross-data-type
1355-
* operators.)
1356-
*
1357-
* We support the convention that sk_subtype == InvalidOid means
1358-
* the opclass input type; this is a hack to simplify life for
1359-
* ScanKeyInit().
1360-
*/
1361-
if (cur->sk_subtype==rel->rd_opcintype[i]||
1362-
cur->sk_subtype==InvalidOid)
1355+
Assert(!(loosen_strat&&tighten_strat));
1356+
if (loosen_strat)
13631357
{
1364-
FmgrInfo*procinfo;
1365-
1366-
procinfo=index_getprocinfo(rel,cur->sk_attno,BTORDER_PROC);
1367-
ScanKeyEntryInitializeWithInfo(inskey.scankeys+i,
1368-
cur->sk_flags,
1369-
cur->sk_attno,
1370-
InvalidStrategy,
1371-
cur->sk_subtype,
1372-
cur->sk_collation,
1373-
procinfo,
1374-
cur->sk_argument);
1358+
/* Use less restrictive strategy (and fewer member keys) */
1359+
switch (strat_total)
1360+
{
1361+
caseBTLessStrategyNumber:
1362+
strat_total=BTLessEqualStrategyNumber;
1363+
break;
1364+
caseBTGreaterStrategyNumber:
1365+
strat_total=BTGreaterEqualStrategyNumber;
1366+
break;
1367+
}
13751368
}
1376-
else
1369+
if (tighten_strat)
13771370
{
1378-
RegProcedurecmp_proc;
1379-
1380-
cmp_proc=get_opfamily_proc(rel->rd_opfamily[i],
1381-
rel->rd_opcintype[i],
1382-
cur->sk_subtype,
1383-
BTORDER_PROC);
1384-
if (!RegProcedureIsValid(cmp_proc))
1385-
elog(ERROR,"missing support function %d(%u,%u) for attribute %d of index \"%s\"",
1386-
BTORDER_PROC,rel->rd_opcintype[i],cur->sk_subtype,
1387-
cur->sk_attno,RelationGetRelationName(rel));
1388-
ScanKeyEntryInitialize(inskey.scankeys+i,
1389-
cur->sk_flags,
1390-
cur->sk_attno,
1391-
InvalidStrategy,
1392-
cur->sk_subtype,
1393-
cur->sk_collation,
1394-
cmp_proc,
1395-
cur->sk_argument);
1371+
/* Use more restrictive strategy (and fewer member keys) */
1372+
switch (strat_total)
1373+
{
1374+
caseBTLessEqualStrategyNumber:
1375+
strat_total=BTLessStrategyNumber;
1376+
break;
1377+
caseBTGreaterEqualStrategyNumber:
1378+
strat_total=BTGreaterStrategyNumber;
1379+
break;
1380+
}
13961381
}
1382+
1383+
/* done adding to inskey (row comparison keys always come last) */
1384+
break;
1385+
}
1386+
1387+
/*
1388+
* Ordinary comparison key/search-style key.
1389+
*
1390+
* Transform the search-style scan key to an insertion scan key by
1391+
* replacing the sk_func with the appropriate btree 3-way-comparison
1392+
* function.
1393+
*
1394+
* If scankey operator is not a cross-type comparison, we can use the
1395+
* cached comparison function; otherwise gotta look it up in the
1396+
* catalogs. (That can't lead to infinite recursion, since no
1397+
* indexscan initiated by syscache lookup will use cross-data-type
1398+
* operators.)
1399+
*
1400+
* We support the convention that sk_subtype == InvalidOid means the
1401+
* opclass input type; this hack simplifies life for ScanKeyInit().
1402+
*/
1403+
if (bkey->sk_subtype==rel->rd_opcintype[i]||
1404+
bkey->sk_subtype==InvalidOid)
1405+
{
1406+
FmgrInfo*procinfo;
1407+
1408+
procinfo=index_getprocinfo(rel,bkey->sk_attno,BTORDER_PROC);
1409+
ScanKeyEntryInitializeWithInfo(inskey.scankeys+i,
1410+
bkey->sk_flags,
1411+
bkey->sk_attno,
1412+
InvalidStrategy,
1413+
bkey->sk_subtype,
1414+
bkey->sk_collation,
1415+
procinfo,
1416+
bkey->sk_argument);
1417+
}
1418+
else
1419+
{
1420+
RegProcedurecmp_proc;
1421+
1422+
cmp_proc=get_opfamily_proc(rel->rd_opfamily[i],
1423+
rel->rd_opcintype[i],
1424+
bkey->sk_subtype,BTORDER_PROC);
1425+
if (!RegProcedureIsValid(cmp_proc))
1426+
elog(ERROR,"missing support function %d(%u,%u) for attribute %d of index \"%s\"",
1427+
BTORDER_PROC,rel->rd_opcintype[i],bkey->sk_subtype,
1428+
bkey->sk_attno,RelationGetRelationName(rel));
1429+
ScanKeyEntryInitialize(inskey.scankeys+i,
1430+
bkey->sk_flags,
1431+
bkey->sk_attno,
1432+
InvalidStrategy,
1433+
bkey->sk_subtype,
1434+
bkey->sk_collation,
1435+
cmp_proc,
1436+
bkey->sk_argument);
13971437
}
13981438
}
13991439

@@ -1482,6 +1522,8 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
14821522

14831523
if (!BufferIsValid(so->currPos.buf))
14841524
{
1525+
Assert(!so->needPrimScan);
1526+
14851527
/*
14861528
* We only get here if the index is completely empty. Lock relation
14871529
* because nothing finer to lock exists. Without a buffer lock, it's
@@ -1500,7 +1542,6 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
15001542

15011543
if (!BufferIsValid(so->currPos.buf))
15021544
{
1503-
Assert(!so->needPrimScan);
15041545
_bt_parallel_done(scan);
15051546
return false;
15061547
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp