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

Commit497bac7

Browse files
committed
Fix long-obsolete code for separating filter conditions in cost_index().
This code relied on pointer equality to identify which restriction clausesalso appear in the indexquals (and, therefore, don't need to be applied assimple filter conditions). That was okay once upon a time, years ago,before we introduced the equivalence-class machinery. Now there's about a50-50 chance that an equality clause appearing in the indexquals will bethe mirror image (commutator) of its mate in the restriction list. Whenthat happens, we'd erroneously think that the clause would be re-evaluatedat each visited row, and therefore inflate the cost estimate for theindexscan by the clause's cost.Add some logic to catch this case. It seems to me that it continues not tobe worthwhile to expend the extra predicate-proof work that createplan.cwill do on the finally-selected plan, but this case is common enough andcheap enough to handle that we should do so.This will make a small difference (about one cpu_operator_cost per row)in simple cases; but in situations where there's an expensive function inthe indexquals, it can make a very large difference, as seen in recentexample from Jeff Janes.This is a long-standing bug, but I'm hesitant to back-patch because of thepossibility of destabilizing plan choices that people may be happy with.
1 parent5223dda commit497bac7

File tree

2 files changed

+61
-20
lines changed

2 files changed

+61
-20
lines changed

‎src/backend/optimizer/path/costsize.c

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ typedef struct
124124
QualCosttotal;
125125
}cost_qual_eval_context;
126126

127+
staticList*extract_nonindex_conditions(List*qual_clauses,List*indexquals);
127128
staticMergeScanSelCache*cached_scansel(PlannerInfo*root,
128129
RestrictInfo*rinfo,
129130
PathKey*pathkey);
@@ -242,7 +243,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
242243
IndexOptInfo*index=path->indexinfo;
243244
RelOptInfo*baserel=index->rel;
244245
boolindexonly= (path->path.pathtype==T_IndexOnlyScan);
245-
List*allclauses;
246+
List*qpquals;
246247
Coststartup_cost=0;
247248
Costrun_cost=0;
248249
CostindexStartupCost;
@@ -265,19 +266,26 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
265266
Assert(baserel->relid>0);
266267
Assert(baserel->rtekind==RTE_RELATION);
267268

268-
/* Mark the path with the correct row estimate */
269+
/*
270+
* Mark the path with the correct row estimate, and identify which quals
271+
* will need to be enforced as qpquals.
272+
*/
269273
if (path->path.param_info)
270274
{
271275
path->path.rows=path->path.param_info->ppi_rows;
272-
/* also get the set of clauses that should be enforced by the scan */
273-
allclauses=list_concat(list_copy(path->path.param_info->ppi_clauses),
274-
baserel->baserestrictinfo);
276+
/* qpquals come from the rel's restriction clauses and ppi_clauses */
277+
qpquals=list_concat(
278+
extract_nonindex_conditions(baserel->baserestrictinfo,
279+
path->indexquals),
280+
extract_nonindex_conditions(path->path.param_info->ppi_clauses,
281+
path->indexquals));
275282
}
276283
else
277284
{
278285
path->path.rows=baserel->rows;
279-
/* allclauses should just be the rel's restriction clauses */
280-
allclauses=baserel->baserestrictinfo;
286+
/* qpquals come from just the rel's restriction clauses */
287+
qpquals=extract_nonindex_conditions(baserel->baserestrictinfo,
288+
path->indexquals);
281289
}
282290

283291
if (!enable_indexscan)
@@ -433,19 +441,9 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
433441
* Estimate CPU costs per tuple.
434442
*
435443
* What we want here is cpu_tuple_cost plus the evaluation costs of any
436-
* qual clauses that we have to evaluate as qpquals. We approximate that
437-
* list as allclauses minus any clauses appearing in indexquals. (We
438-
* assume that pointer equality is enough to recognize duplicate
439-
* RestrictInfos.)This method neglects some considerations such as
440-
* clauses that needn't be checked because they are implied by a partial
441-
* index's predicate. It does not seem worth the cycles to try to factor
442-
* those things in at this stage, even though createplan.c will take pains
443-
* to remove such unnecessary clauses from the qpquals list if this path
444-
* is selected for use.
445-
*/
446-
cost_qual_eval(&qpqual_cost,
447-
list_difference_ptr(allclauses,path->indexquals),
448-
root);
444+
* qual clauses that we have to evaluate as qpquals.
445+
*/
446+
cost_qual_eval(&qpqual_cost,qpquals,root);
449447

450448
startup_cost+=qpqual_cost.startup;
451449
cpu_per_tuple=cpu_tuple_cost+qpqual_cost.per_tuple;
@@ -456,6 +454,46 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
456454
path->path.total_cost=startup_cost+run_cost;
457455
}
458456

457+
/*
458+
* extract_nonindex_conditions
459+
*
460+
* Given a list of quals to be enforced in an indexscan, extract the ones that
461+
* will have to be applied as qpquals (ie, the index machinery won't handle
462+
* them). The actual rules for this appear in create_indexscan_plan() in
463+
* createplan.c, but the full rules are fairly expensive and we don't want to
464+
* go to that much effort for index paths that don't get selected for the
465+
* final plan. So we approximate it as quals that don't appear directly in
466+
* indexquals and also are not redundant children of the same EquivalenceClass
467+
* as some indexqual. This method neglects some infrequently-relevant
468+
* considerations such as clauses that needn't be checked because they are
469+
* implied by a partial index's predicate. It does not seem worth the cycles
470+
* to try to factor those things in at this stage, even though createplan.c
471+
* will take pains to remove such unnecessary clauses from the qpquals list if
472+
* this path is selected for use.
473+
*/
474+
staticList*
475+
extract_nonindex_conditions(List*qual_clauses,List*indexquals)
476+
{
477+
List*result=NIL;
478+
ListCell*lc;
479+
480+
foreach(lc,qual_clauses)
481+
{
482+
RestrictInfo*rinfo= (RestrictInfo*)lfirst(lc);
483+
484+
Assert(IsA(rinfo,RestrictInfo));
485+
if (rinfo->pseudoconstant)
486+
continue;/* we may drop pseudoconstants here */
487+
if (list_member_ptr(indexquals,rinfo))
488+
continue;/* simple duplicate */
489+
if (is_redundant_derived_clause(rinfo,indexquals))
490+
continue;/* derived from same EquivalenceClass */
491+
/* ... skip the predicate proof attempts createplan.c will try ... */
492+
result=lappend(result,rinfo);
493+
}
494+
returnresult;
495+
}
496+
459497
/*
460498
* index_pages_fetched
461499
* Estimate the number of pages actually fetched after accounting for

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,9 @@ create_indexscan_plan(PlannerInfo *root,
12101210
* predicate, but only in a plain SELECT; when scanning a target relation
12111211
* of UPDATE/DELETE/SELECT FOR UPDATE, we must leave such quals in the
12121212
* plan so that they'll be properly rechecked by EvalPlanQual testing.
1213+
*
1214+
* Note: if you change this bit of code you should also look at
1215+
* extract_nonindex_conditions() in costsize.c.
12131216
*/
12141217
qpqual=NIL;
12151218
foreach(l,scan_clauses)

0 commit comments

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