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

Commit9a3ddeb

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 parent4b16049 commit9a3ddeb

File tree

12 files changed

+124
-66
lines changed

12 files changed

+124
-66
lines changed

‎src/backend/commands/explain.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1746,7 +1746,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
17461746
caseT_IndexOnlyScan:
17471747
show_scan_qual(((IndexOnlyScan*)plan)->indexqual,
17481748
"Index Cond",planstate,ancestors,es);
1749-
if (((IndexOnlyScan*)plan)->indexqual)
1749+
if (((IndexOnlyScan*)plan)->recheckqual)
17501750
show_instrumentation_count("Rows Removed by Index Recheck",2,
17511751
planstate,es);
17521752
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
@@ -519,6 +519,7 @@ _copyIndexOnlyScan(const IndexOnlyScan *from)
519519
*/
520520
COPY_SCALAR_FIELD(indexid);
521521
COPY_NODE_FIELD(indexqual);
522+
COPY_NODE_FIELD(recheckqual);
522523
COPY_NODE_FIELD(indexorderby);
523524
COPY_NODE_FIELD(indextlist);
524525
COPY_SCALAR_FIELD(indexorderdir);

‎src/backend/nodes/outfuncs.c

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

581581
WRITE_OID_FIELD(indexid);
582582
WRITE_NODE_FIELD(indexqual);
583+
WRITE_NODE_FIELD(recheckqual);
583584
WRITE_NODE_FIELD(indexorderby);
584585
WRITE_NODE_FIELD(indextlist);
585586
WRITE_ENUM_FIELD(indexorderdir,ScanDirection);

‎src/backend/nodes/readfuncs.c

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

18851885
READ_OID_FIELD(indexid);
18861886
READ_NODE_FIELD(indexqual);
1887+
READ_NODE_FIELD(recheckqual);
18871888
READ_NODE_FIELD(indexorderby);
18881889
READ_NODE_FIELD(indextlist);
18891890
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"
@@ -189,10 +188,10 @@ static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
189188
ScanDirectionindexscandir);
190189
staticIndexOnlyScan*make_indexonlyscan(List*qptlist,List*qpqual,
191190
Indexscanrelid,Oidindexid,
192-
List*indexqual,List*indexorderby,
191+
List*indexqual,List*recheckqual,
192+
List*indexorderby,
193193
List*indextlist,
194194
ScanDirectionindexscandir);
195-
staticList*make_indexonly_tlist(IndexOptInfo*indexinfo);
196195
staticBitmapIndexScan*make_bitmap_indexscan(Indexscanrelid,Oidindexid,
197196
List*indexqual,
198197
List*indexqualorig);
@@ -623,7 +622,7 @@ create_scan_plan(PlannerInfo *root, Path *best_path, int flags)
623622
if (best_path->pathtype==T_IndexOnlyScan)
624623
{
625624
/* For index-only scan, the preferred tlist is the index's */
626-
tlist=copyObject(make_indexonly_tlist(((IndexPath*)best_path)->indexinfo));
625+
tlist=copyObject(((IndexPath*)best_path)->indexinfo->indextlist);
627626

628627
/*
629628
* Transfer sortgroupref data to the replacement tlist, if
@@ -2934,7 +2933,8 @@ create_indexscan_plan(PlannerInfo *root,
29342933
List*indexclauses=best_path->indexclauses;
29352934
List*indexorderbys=best_path->indexorderbys;
29362935
Indexbaserelid=best_path->path.parent->relid;
2937-
Oidindexoid=best_path->indexinfo->indexoid;
2936+
IndexOptInfo*indexinfo=best_path->indexinfo;
2937+
Oidindexoid=indexinfo->indexoid;
29382938
List*qpqual;
29392939
List*stripped_indexquals;
29402940
List*fixed_indexquals;
@@ -3064,15 +3064,34 @@ create_indexscan_plan(PlannerInfo *root,
30643064
}
30653065
}
30663066

3067+
/*
3068+
* For an index-only scan, we must mark indextlist entries as resjunk if
3069+
* they are columns that the index AM can't return; this cues setrefs.c to
3070+
* not generate references to those columns.
3071+
*/
3072+
if (indexonly)
3073+
{
3074+
inti=0;
3075+
3076+
foreach(l,indexinfo->indextlist)
3077+
{
3078+
TargetEntry*indextle= (TargetEntry*)lfirst(l);
3079+
3080+
indextle->resjunk= !indexinfo->canreturn[i];
3081+
i++;
3082+
}
3083+
}
3084+
30673085
/* Finally ready to build the plan node */
30683086
if (indexonly)
30693087
scan_plan= (Scan*)make_indexonlyscan(tlist,
30703088
qpqual,
30713089
baserelid,
30723090
indexoid,
30733091
fixed_indexquals,
3092+
stripped_indexquals,
30743093
fixed_indexorderbys,
3075-
make_indexonly_tlist(best_path->indexinfo),
3094+
indexinfo->indextlist,
30763095
best_path->indexscandir);
30773096
else
30783097
scan_plan= (Scan*)make_indexscan(tlist,
@@ -5444,6 +5463,7 @@ make_indexonlyscan(List *qptlist,
54445463
Indexscanrelid,
54455464
Oidindexid,
54465465
List*indexqual,
5466+
List*recheckqual,
54475467
List*indexorderby,
54485468
List*indextlist,
54495469
ScanDirectionindexscandir)
@@ -5458,60 +5478,14 @@ make_indexonlyscan(List *qptlist,
54585478
node->scan.scanrelid=scanrelid;
54595479
node->indexid=indexid;
54605480
node->indexqual=indexqual;
5481+
node->recheckqual=recheckqual;
54615482
node->indexorderby=indexorderby;
54625483
node->indextlist=indextlist;
54635484
node->indexorderdir=indexscandir;
54645485

54655486
returnnode;
54665487
}
54675488

5468-
/*
5469-
* make_indexonly_tlist
5470-
*
5471-
* Construct the indextlist for an IndexOnlyScan plan node.
5472-
* We must replace any column that can't be returned by the index AM
5473-
* with a null Const of the appropriate datatype. This is necessary
5474-
* to prevent setrefs.c from trying to use the value of such a column,
5475-
* and anyway it makes the indextlist a better representative of what
5476-
* the indexscan will really return. (We do this here, not where the
5477-
* IndexOptInfo is originally constructed, because earlier planner
5478-
* steps need to know what is in such columns.)
5479-
*/
5480-
staticList*
5481-
make_indexonly_tlist(IndexOptInfo*indexinfo)
5482-
{
5483-
List*result;
5484-
inti;
5485-
ListCell*lc;
5486-
5487-
/* We needn't work hard for the common case of btrees. */
5488-
if (indexinfo->relam==BTREE_AM_OID)
5489-
returnindexinfo->indextlist;
5490-
5491-
result=NIL;
5492-
i=0;
5493-
foreach(lc,indexinfo->indextlist)
5494-
{
5495-
TargetEntry*indextle= (TargetEntry*)lfirst(lc);
5496-
5497-
if (indexinfo->canreturn[i])
5498-
result=lappend(result,indextle);
5499-
else
5500-
{
5501-
TargetEntry*newtle=makeNode(TargetEntry);
5502-
Node*texpr= (Node*)indextle->expr;
5503-
5504-
memcpy(newtle,indextle,sizeof(TargetEntry));
5505-
newtle->expr= (Expr*)makeNullConst(exprType(texpr),
5506-
exprTypmod(texpr),
5507-
exprCollation(texpr));
5508-
result=lappend(result,newtle);
5509-
}
5510-
i++;
5511-
}
5512-
returnresult;
5513-
}
5514-
55155489
staticBitmapIndexScan*
55165490
make_bitmap_indexscan(Indexscanrelid,
55175491
Oidindexid,

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1142,8 +1142,26 @@ set_indexonlyscan_references(PlannerInfo *root,
11421142
intrtoffset)
11431143
{
11441144
indexed_tlist*index_itlist;
1145+
List*stripped_indextlist;
1146+
ListCell*lc;
1147+
1148+
/*
1149+
* Vars in the plan node's targetlist, qual, and recheckqual must only
1150+
* reference columns that the index AM can actually return. To ensure
1151+
* this, remove non-returnable columns (which are marked as resjunk) from
1152+
* the indexed tlist. We can just drop them because the indexed_tlist
1153+
* machinery pays attention to TLE resnos, not physical list position.
1154+
*/
1155+
stripped_indextlist=NIL;
1156+
foreach(lc,plan->indextlist)
1157+
{
1158+
TargetEntry*indextle= (TargetEntry*)lfirst(lc);
1159+
1160+
if (!indextle->resjunk)
1161+
stripped_indextlist=lappend(stripped_indextlist,indextle);
1162+
}
11451163

1146-
index_itlist=build_tlist_index(plan->indextlist);
1164+
index_itlist=build_tlist_index(stripped_indextlist);
11471165

11481166
plan->scan.scanrelid+=rtoffset;
11491167
plan->scan.plan.targetlist= (List*)
@@ -1160,6 +1178,13 @@ set_indexonlyscan_references(PlannerInfo *root,
11601178
INDEX_VAR,
11611179
rtoffset,
11621180
NUM_EXEC_QUAL((Plan*)plan));
1181+
plan->recheckqual= (List*)
1182+
fix_upper_expr(root,
1183+
(Node*)plan->recheckqual,
1184+
index_itlist,
1185+
INDEX_VAR,
1186+
rtoffset,
1187+
NUM_EXEC_QUAL((Plan*)plan));
11631188
/* indexqual is already transformed to reference index columns */
11641189
plan->indexqual=fix_scan_list(root,plan->indexqual,
11651190
rtoffset,1);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2336,6 +2336,8 @@ finalize_plan(PlannerInfo *root, Plan *plan,
23362336
caseT_IndexOnlyScan:
23372337
finalize_primnode((Node*) ((IndexOnlyScan*)plan)->indexqual,
23382338
&context);
2339+
finalize_primnode((Node*) ((IndexOnlyScan*)plan)->recheckqual,
2340+
&context);
23392341
finalize_primnode((Node*) ((IndexOnlyScan*)plan)->indexorderby,
23402342
&context);
23412343

‎src/include/nodes/execnodes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,7 +1490,7 @@ typedef struct IndexScanState
14901490
/* ----------------
14911491
* IndexOnlyScanState information
14921492
*
1493-
*indexqual execution state forindexqual expressions
1493+
*recheckqual execution state forrecheckqual expressions
14941494
*ScanKeys Skey structures for index quals
14951495
*NumScanKeys number of ScanKeys
14961496
*OrderByKeys Skey structures for index ordering operators
@@ -1509,7 +1509,7 @@ typedef struct IndexScanState
15091509
typedefstructIndexOnlyScanState
15101510
{
15111511
ScanStatess;/* its first field is NodeTag */
1512-
ExprState*indexqual;
1512+
ExprState*recheckqual;
15131513
structScanKeyData*ioss_ScanKeys;
15141514
intioss_NumScanKeys;
15151515
structScanKeyData*ioss_OrderByKeys;

‎src/include/nodes/plannodes.h

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -420,22 +420,36 @@ typedef struct IndexScan
420420
* index-only scan, in which the data comes from the index not the heap.
421421
* Because of this, *all* Vars in the plan node's targetlist, qual, and
422422
* index expressions reference index columns and have varno = INDEX_VAR.
423-
* Hence we do not need separate indexqualorig and indexorderbyorig lists,
424-
* since their contents would be equivalent to indexqual and indexorderby.
423+
*
424+
* We could almost use indexqual directly against the index's output tuple
425+
* when rechecking lossy index operators, but that won't work for quals on
426+
* index columns that are not retrievable. Hence, recheckqual is needed
427+
* for rechecks: it expresses the same condition as indexqual, but using
428+
* only index columns that are retrievable. (We will not generate an
429+
* index-only scan if this is not possible. An example is that if an
430+
* index has table column "x" in a retrievable index column "ind1", plus
431+
* an expression f(x) in a non-retrievable column "ind2", an indexable
432+
* query on f(x) will use "ind2" in indexqual and f(ind1) in recheckqual.
433+
* Without the "ind1" column, an index-only scan would be disallowed.)
434+
*
435+
* We don't currently need a recheckable equivalent of indexorderby,
436+
* because we don't support lossy operators in index ORDER BY.
425437
*
426438
* To help EXPLAIN interpret the index Vars for display, we provide
427439
* indextlist, which represents the contents of the index as a targetlist
428440
* with one TLE per index column. Vars appearing in this list reference
429441
* the base table, and this is the only field in the plan node that may
430-
* contain such Vars. Note however that index columns that the AM can't
431-
* reconstruct are replaced by null Consts in indextlist.
442+
* contain such Vars. Also, for the convenience of setrefs.c, TLEs in
443+
* indextlist are marked as resjunk if they correspond to columns that
444+
* the index AM cannot reconstruct.
432445
* ----------------
433446
*/
434447
typedefstructIndexOnlyScan
435448
{
436449
Scanscan;
437450
Oidindexid;/* OID of index to scan */
438451
List*indexqual;/* list of index quals (usually OpExprs) */
452+
List*recheckqual;/* index quals in recheckable form */
439453
List*indexorderby;/* list of index ORDER BY exprs */
440454
List*indextlist;/* TargetEntry list describing index's cols */
441455
ScanDirectionindexorderdir;/* forward or backward or don't care */

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

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

343+
-- Similarly, test that index rechecks involving a non-returnable column
344+
-- are done correctly.
345+
explain (verbose, costs off)
346+
select p from gist_tbl where circle(p,1) @> circle(point(0,0),0.95);
347+
QUERY PLAN
348+
---------------------------------------------------------------------------------------
349+
Index Only Scan using gist_tbl_multi_index on public.gist_tbl
350+
Output: p
351+
Index Cond: ((circle(gist_tbl.p, '1'::double precision)) @> '<(0,0),0.95>'::circle)
352+
(3 rows)
353+
354+
select p from gist_tbl where circle(p,1) @> circle(point(0,0),0.95);
355+
p
356+
-------
357+
(0,0)
358+
(1 row)
359+
360+
-- This case isn't supported, but it should at least EXPLAIN correctly.
361+
explain (verbose, costs off)
362+
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
363+
QUERY PLAN
364+
------------------------------------------------------------------------------------
365+
Limit
366+
Output: p, ((circle(p, '1'::double precision) <-> '(0,0)'::point))
367+
-> Index Only Scan using gist_tbl_multi_index on public.gist_tbl
368+
Output: p, (circle(p, '1'::double precision) <-> '(0,0)'::point)
369+
Order By: ((circle(gist_tbl.p, '1'::double precision)) <-> '(0,0)'::point)
370+
(5 rows)
371+
372+
select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
373+
ERROR: lossy distance functions are not supported in index-only scans
343374
-- Clean up
344375
reset enable_seqscan;
345376
reset enable_bitmapscan;

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

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

156+
-- Similarly, test that index rechecks involving a non-returnable column
157+
-- are done correctly.
158+
explain (verbose, costs off)
159+
select pfrom gist_tblwherecircle(p,1) @>circle(point(0,0),0.95);
160+
select pfrom gist_tblwherecircle(p,1) @>circle(point(0,0),0.95);
161+
162+
-- This case isn't supported, but it should at least EXPLAIN correctly.
163+
explain (verbose, costs off)
164+
select pfrom gist_tblorder bycircle(p,1)<->point(0,0)limit1;
165+
select pfrom gist_tblorder bycircle(p,1)<->point(0,0)limit1;
166+
156167
-- Clean up
157168
reset enable_seqscan;
158169
reset enable_bitmapscan;

0 commit comments

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