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

Commit9c4f389

Browse files
committed
Fix index-only scan plans, take 2.
Commit4ace456 failed to fix the problem fully, because thesame issue of attempting to fetch a non-returnable index columncan occur when rechecking the indexqual after using a lossy indexoperator. Moreover, it broke EXPLAIN for such indexquals (whichindicates a gap in our test cases :-().Revert the code changes of4ace456 in favor of adding a new fieldto struct IndexOnlyScan, containing a version of the indexqual thatcan be executed against the index-returned tuple without using anynon-returnable columns. (The restrictions imposed by check_index_onlyguarantee this is possible, although we may have to recompute indexedexpressions.) Support construction of that during setrefs.cprocessing by marking IndexOnlyScan.indextlist entries as resjunkif they can't be returned, rather than removing them entirely.(We could alternatively require setrefs.c to look up the IndexOptInfoagain, but abusing resjunk this way seems like a reasonably safe wayto avoid needing to do that.)This solution isn't great from an API-stability standpoint: if thereare any extensions out there that build IndexOnlyScan structs directly,they'll be broken in the next minor releases. However, only a veryinvasive extension would be likely to do such a thing. There's nochange in the Path representation, so typical planner extensionsshouldn't have a problem.As before, back-patch to all supported branches.Discussion:https://postgr.es/m/3179992.1641150853@sss.pgh.pa.usDiscussion:https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.org
1 parent580e998 commit9c4f389

File tree

12 files changed

+123
-66
lines changed

12 files changed

+123
-66
lines changed

‎src/backend/commands/explain.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1595,7 +1595,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
15951595
caseT_IndexOnlyScan:
15961596
show_scan_qual(((IndexOnlyScan*)plan)->indexqual,
15971597
"Index Cond",planstate,ancestors,es);
1598-
if (((IndexOnlyScan*)plan)->indexqual)
1598+
if (((IndexOnlyScan*)plan)->recheckqual)
15991599
show_instrumentation_count("Rows Removed by Index Recheck",2,
16001600
planstate,es);
16011601
show_scan_qual(((IndexOnlyScan*)plan)->indexorderby,

‎src/backend/executor/nodeIndexonlyscan.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,11 @@ IndexOnlyNext(IndexOnlyScanState *node)
214214

215215
/*
216216
* If the index was lossy, we have to recheck the index quals.
217-
* (Currently, this can never happen, but we should support the case
218-
* for possible future use, eg with GiST indexes.)
219217
*/
220218
if (scandesc->xs_recheck)
221219
{
222220
econtext->ecxt_scantuple=slot;
223-
if (!ExecQualAndReset(node->indexqual,econtext))
221+
if (!ExecQualAndReset(node->recheckqual,econtext))
224222
{
225223
/* Fails recheck, so drop it and loop back for another */
226224
InstrCountFiltered2(node,1);
@@ -555,8 +553,8 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags)
555553
*/
556554
indexstate->ss.ps.qual=
557555
ExecInitQual(node->scan.plan.qual, (PlanState*)indexstate);
558-
indexstate->indexqual=
559-
ExecInitQual(node->indexqual, (PlanState*)indexstate);
556+
indexstate->recheckqual=
557+
ExecInitQual(node->recheckqual, (PlanState*)indexstate);
560558

561559
/*
562560
* If we are just doing EXPLAIN (ie, aren't going to run the plan), stop

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,7 @@ _copyIndexOnlyScan(const IndexOnlyScan *from)
512512
*/
513513
COPY_SCALAR_FIELD(indexid);
514514
COPY_NODE_FIELD(indexqual);
515+
COPY_NODE_FIELD(recheckqual);
515516
COPY_NODE_FIELD(indexorderby);
516517
COPY_NODE_FIELD(indextlist);
517518
COPY_SCALAR_FIELD(indexorderdir);

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,7 @@ _outIndexOnlyScan(StringInfo str, const IndexOnlyScan *node)
570570

571571
WRITE_OID_FIELD(indexid);
572572
WRITE_NODE_FIELD(indexqual);
573+
WRITE_NODE_FIELD(recheckqual);
573574
WRITE_NODE_FIELD(indexorderby);
574575
WRITE_NODE_FIELD(indextlist);
575576
WRITE_ENUM_FIELD(indexorderdir,ScanDirection);

‎src/backend/nodes/readfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,6 +1803,7 @@ _readIndexOnlyScan(void)
18031803

18041804
READ_OID_FIELD(indexid);
18051805
READ_NODE_FIELD(indexqual);
1806+
READ_NODE_FIELD(recheckqual);
18061807
READ_NODE_FIELD(indexorderby);
18071808
READ_NODE_FIELD(indextlist);
18081809
READ_ENUM_FIELD(indexorderdir,ScanDirection);

‎src/backend/optimizer/plan/createplan.c

Lines changed: 27 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include<math.h>
2121

2222
#include"access/sysattr.h"
23-
#include"catalog/pg_am.h"
2423
#include"catalog/pg_class.h"
2524
#include"foreign/fdwapi.h"
2625
#include"miscadmin.h"
@@ -179,10 +178,10 @@ static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
179178
ScanDirectionindexscandir);
180179
staticIndexOnlyScan*make_indexonlyscan(List*qptlist,List*qpqual,
181180
Indexscanrelid,Oidindexid,
182-
List*indexqual,List*indexorderby,
181+
List*indexqual,List*recheckqual,
182+
List*indexorderby,
183183
List*indextlist,
184184
ScanDirectionindexscandir);
185-
staticList*make_indexonly_tlist(IndexOptInfo*indexinfo);
186185
staticBitmapIndexScan*make_bitmap_indexscan(Indexscanrelid,Oidindexid,
187186
List*indexqual,
188187
List*indexqualorig);
@@ -592,7 +591,7 @@ create_scan_plan(PlannerInfo *root, Path *best_path, int flags)
592591
if (best_path->pathtype==T_IndexOnlyScan)
593592
{
594593
/* For index-only scan, the preferred tlist is the index's */
595-
tlist=copyObject(make_indexonly_tlist(((IndexPath*)best_path)->indexinfo));
594+
tlist=copyObject(((IndexPath*)best_path)->indexinfo->indextlist);
596595

597596
/*
598597
* Transfer sortgroupref data to the replacement tlist, if
@@ -2773,7 +2772,8 @@ create_indexscan_plan(PlannerInfo *root,
27732772
List*indexclauses=best_path->indexclauses;
27742773
List*indexorderbys=best_path->indexorderbys;
27752774
Indexbaserelid=best_path->path.parent->relid;
2776-
Oidindexoid=best_path->indexinfo->indexoid;
2775+
IndexOptInfo*indexinfo=best_path->indexinfo;
2776+
Oidindexoid=indexinfo->indexoid;
27772777
List*qpqual;
27782778
List*stripped_indexquals;
27792779
List*fixed_indexquals;
@@ -2903,15 +2903,34 @@ create_indexscan_plan(PlannerInfo *root,
29032903
}
29042904
}
29052905

2906+
/*
2907+
* For an index-only scan, we must mark indextlist entries as resjunk if
2908+
* they are columns that the index AM can't return; this cues setrefs.c to
2909+
* not generate references to those columns.
2910+
*/
2911+
if (indexonly)
2912+
{
2913+
inti=0;
2914+
2915+
foreach(l,indexinfo->indextlist)
2916+
{
2917+
TargetEntry*indextle= (TargetEntry*)lfirst(l);
2918+
2919+
indextle->resjunk= !indexinfo->canreturn[i];
2920+
i++;
2921+
}
2922+
}
2923+
29062924
/* Finally ready to build the plan node */
29072925
if (indexonly)
29082926
scan_plan= (Scan*)make_indexonlyscan(tlist,
29092927
qpqual,
29102928
baserelid,
29112929
indexoid,
29122930
fixed_indexquals,
2931+
stripped_indexquals,
29132932
fixed_indexorderbys,
2914-
make_indexonly_tlist(best_path->indexinfo),
2933+
indexinfo->indextlist,
29152934
best_path->indexscandir);
29162935
else
29172936
scan_plan= (Scan*)make_indexscan(tlist,
@@ -5213,6 +5232,7 @@ make_indexonlyscan(List *qptlist,
52135232
Indexscanrelid,
52145233
Oidindexid,
52155234
List*indexqual,
5235+
List*recheckqual,
52165236
List*indexorderby,
52175237
List*indextlist,
52185238
ScanDirectionindexscandir)
@@ -5227,60 +5247,14 @@ make_indexonlyscan(List *qptlist,
52275247
node->scan.scanrelid=scanrelid;
52285248
node->indexid=indexid;
52295249
node->indexqual=indexqual;
5250+
node->recheckqual=recheckqual;
52305251
node->indexorderby=indexorderby;
52315252
node->indextlist=indextlist;
52325253
node->indexorderdir=indexscandir;
52335254

52345255
returnnode;
52355256
}
52365257

5237-
/*
5238-
* make_indexonly_tlist
5239-
*
5240-
* Construct the indextlist for an IndexOnlyScan plan node.
5241-
* We must replace any column that can't be returned by the index AM
5242-
* with a null Const of the appropriate datatype. This is necessary
5243-
* to prevent setrefs.c from trying to use the value of such a column,
5244-
* and anyway it makes the indextlist a better representative of what
5245-
* the indexscan will really return. (We do this here, not where the
5246-
* IndexOptInfo is originally constructed, because earlier planner
5247-
* steps need to know what is in such columns.)
5248-
*/
5249-
staticList*
5250-
make_indexonly_tlist(IndexOptInfo*indexinfo)
5251-
{
5252-
List*result;
5253-
inti;
5254-
ListCell*lc;
5255-
5256-
/* We needn't work hard for the common case of btrees. */
5257-
if (indexinfo->relam==BTREE_AM_OID)
5258-
returnindexinfo->indextlist;
5259-
5260-
result=NIL;
5261-
i=0;
5262-
foreach(lc,indexinfo->indextlist)
5263-
{
5264-
TargetEntry*indextle= (TargetEntry*)lfirst(lc);
5265-
5266-
if (indexinfo->canreturn[i])
5267-
result=lappend(result,indextle);
5268-
else
5269-
{
5270-
TargetEntry*newtle=makeNode(TargetEntry);
5271-
Node*texpr= (Node*)indextle->expr;
5272-
5273-
memcpy(newtle,indextle,sizeof(TargetEntry));
5274-
newtle->expr= (Expr*)makeNullConst(exprType(texpr),
5275-
exprTypmod(texpr),
5276-
exprCollation(texpr));
5277-
result=lappend(result,newtle);
5278-
}
5279-
i++;
5280-
}
5281-
returnresult;
5282-
}
5283-
52845258
staticBitmapIndexScan*
52855259
make_bitmap_indexscan(Indexscanrelid,
52865260
Oidindexid,

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -990,8 +990,26 @@ set_indexonlyscan_references(PlannerInfo *root,
990990
intrtoffset)
991991
{
992992
indexed_tlist*index_itlist;
993+
List*stripped_indextlist;
994+
ListCell*lc;
995+
996+
/*
997+
* Vars in the plan node's targetlist, qual, and recheckqual must only
998+
* reference columns that the index AM can actually return. To ensure
999+
* this, remove non-returnable columns (which are marked as resjunk) from
1000+
* the indexed tlist. We can just drop them because the indexed_tlist
1001+
* machinery pays attention to TLE resnos, not physical list position.
1002+
*/
1003+
stripped_indextlist=NIL;
1004+
foreach(lc,plan->indextlist)
1005+
{
1006+
TargetEntry*indextle= (TargetEntry*)lfirst(lc);
9931007

994-
index_itlist=build_tlist_index(plan->indextlist);
1008+
if (!indextle->resjunk)
1009+
stripped_indextlist=lappend(stripped_indextlist,indextle);
1010+
}
1011+
1012+
index_itlist=build_tlist_index(stripped_indextlist);
9951013

9961014
plan->scan.scanrelid+=rtoffset;
9971015
plan->scan.plan.targetlist= (List*)
@@ -1006,6 +1024,12 @@ set_indexonlyscan_references(PlannerInfo *root,
10061024
index_itlist,
10071025
INDEX_VAR,
10081026
rtoffset);
1027+
plan->recheckqual= (List*)
1028+
fix_upper_expr(root,
1029+
(Node*)plan->recheckqual,
1030+
index_itlist,
1031+
INDEX_VAR,
1032+
rtoffset);
10091033
/* indexqual is already transformed to reference index columns */
10101034
plan->indexqual=fix_scan_list(root,plan->indexqual,rtoffset);
10111035
/* indexorderby is already transformed to reference index columns */

‎src/backend/optimizer/plan/subselect.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2306,6 +2306,8 @@ finalize_plan(PlannerInfo *root, Plan *plan,
23062306
caseT_IndexOnlyScan:
23072307
finalize_primnode((Node*) ((IndexOnlyScan*)plan)->indexqual,
23082308
&context);
2309+
finalize_primnode((Node*) ((IndexOnlyScan*)plan)->recheckqual,
2310+
&context);
23092311
finalize_primnode((Node*) ((IndexOnlyScan*)plan)->indexorderby,
23102312
&context);
23112313

‎src/include/nodes/execnodes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,7 +1453,7 @@ typedef struct IndexScanState
14531453
/* ----------------
14541454
* IndexOnlyScanState information
14551455
*
1456-
*indexqual execution state forindexqual expressions
1456+
*recheckqual execution state forrecheckqual expressions
14571457
*ScanKeys Skey structures for index quals
14581458
*NumScanKeys number of ScanKeys
14591459
*OrderByKeys Skey structures for index ordering operators
@@ -1472,7 +1472,7 @@ typedef struct IndexScanState
14721472
typedefstructIndexOnlyScanState
14731473
{
14741474
ScanStatess;/* its first field is NodeTag */
1475-
ExprState*indexqual;
1475+
ExprState*recheckqual;
14761476
structScanKeyData*ioss_ScanKeys;
14771477
intioss_NumScanKeys;
14781478
structScanKeyData*ioss_OrderByKeys;

‎src/include/nodes/plannodes.h

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,15 +414,28 @@ typedef struct IndexScan
414414
* index-only scan, in which the data comes from the index not the heap.
415415
* Because of this, *all* Vars in the plan node's targetlist, qual, and
416416
* index expressions reference index columns and have varno = INDEX_VAR.
417-
* Hence we do not need separate indexqualorig and indexorderbyorig lists,
418-
* since their contents would be equivalent to indexqual and indexorderby.
417+
*
418+
* We could almost use indexqual directly against the index's output tuple
419+
* when rechecking lossy index operators, but that won't work for quals on
420+
* index columns that are not retrievable. Hence, recheckqual is needed
421+
* for rechecks: it expresses the same condition as indexqual, but using
422+
* only index columns that are retrievable. (We will not generate an
423+
* index-only scan if this is not possible. An example is that if an
424+
* index has table column "x" in a retrievable index column "ind1", plus
425+
* an expression f(x) in a non-retrievable column "ind2", an indexable
426+
* query on f(x) will use "ind2" in indexqual and f(ind1) in recheckqual.
427+
* Without the "ind1" column, an index-only scan would be disallowed.)
428+
*
429+
* We don't currently need a recheckable equivalent of indexorderby,
430+
* because we don't support lossy operators in index ORDER BY.
419431
*
420432
* To help EXPLAIN interpret the index Vars for display, we provide
421433
* indextlist, which represents the contents of the index as a targetlist
422434
* with one TLE per index column. Vars appearing in this list reference
423435
* the base table, and this is the only field in the plan node that may
424-
* contain such Vars. Note however that index columns that the AM can't
425-
* reconstruct are replaced by null Consts in indextlist.
436+
* contain such Vars. Also, for the convenience of setrefs.c, TLEs in
437+
* indextlist are marked as resjunk if they correspond to columns that
438+
* the index AM cannot reconstruct.
426439
* ----------------
427440
*/
428441
typedefstructIndexOnlyScan
@@ -433,6 +446,7 @@ typedef struct IndexOnlyScan
433446
List*indexorderby;/* list of index ORDER BY exprs */
434447
List*indextlist;/* TargetEntry list describing index's cols */
435448
ScanDirectionindexorderdir;/* forward or backward or don't care */
449+
List*recheckqual;/* index quals in recheckable form */
436450
}IndexOnlyScan;
437451

438452
/* ----------------

‎src/test/regress/expected/gist.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,37 @@ where p <@ box(point(5, 5), point(5.3, 5.3));
264264
<(5.3,5.3),1>
265265
(7 rows)
266266

267+
-- Similarly, test that index rechecks involving a non-returnable column
268+
-- are done correctly.
269+
explain (verbose, costs off)
270+
select p from gist_tbl where circle(p,1) @> circle(point(0,0),0.95);
271+
QUERY PLAN
272+
---------------------------------------------------------------------------------------
273+
Index Only Scan using gist_tbl_multi_index on public.gist_tbl
274+
Output: p
275+
Index Cond: ((circle(gist_tbl.p, '1'::double precision)) @> '<(0,0),0.95>'::circle)
276+
(3 rows)
277+
278+
select p from gist_tbl where circle(p,1) @> circle(point(0,0),0.95);
279+
p
280+
-------
281+
(0,0)
282+
(1 row)
283+
284+
-- This case isn't supported, but it should at least EXPLAIN correctly.
285+
explain (verbose, costs off)
286+
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
287+
QUERY PLAN
288+
------------------------------------------------------------------------------------
289+
Limit
290+
Output: p, ((circle(p, '1'::double precision) <-> '(0,0)'::point))
291+
-> Index Only Scan using gist_tbl_multi_index on public.gist_tbl
292+
Output: p, (circle(p, '1'::double precision) <-> '(0,0)'::point)
293+
Order By: ((circle(gist_tbl.p, '1'::double precision)) <-> '(0,0)'::point)
294+
(5 rows)
295+
296+
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
297+
ERROR: lossy distance functions are not supported in index-only scans
267298
-- Clean up
268299
reset enable_seqscan;
269300
reset enable_bitmapscan;

‎src/test/regress/sql/gist.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,17 @@ where p <@ box(point(5, 5), point(5.3, 5.3));
137137
selectcircle(p,1)from gist_tbl
138138
where p<@box(point(5,5),point(5.3,5.3));
139139

140+
-- Similarly, test that index rechecks involving a non-returnable column
141+
-- are done correctly.
142+
explain (verbose, costs off)
143+
select pfrom gist_tblwherecircle(p,1) @>circle(point(0,0),0.95);
144+
select pfrom gist_tblwherecircle(p,1) @>circle(point(0,0),0.95);
145+
146+
-- This case isn't supported, but it should at least EXPLAIN correctly.
147+
explain (verbose, costs off)
148+
select pfrom gist_tblorder bycircle(p,1)<->point(0,0)limit1;
149+
select pfrom gist_tblorder bycircle(p,1)<->point(0,0)limit1;
150+
140151
-- Clean up
141152
reset enable_seqscan;
142153
reset enable_bitmapscan;

0 commit comments

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