mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-08-31 12:27:22 +02:00
Clean up planner confusion between ncolumns and nkeycolumns.
We're only going to consider key columns when creating indexquals, so there is no point in having the outer loops in indxpath.c iterate further than nkeycolumns. Doing so in match_pathkeys_to_index() is actually wrong, and would have caused crashes by now, except that we have no index AMs supporting both amcanorderbyop and amcaninclude. It's also wrong in relation_has_unique_index_for(). The effect there is to fail to prove uniqueness even when the index does prove it, if there are extra columns. Also future-proof examine_variable() for the day when extra columns can be expressions, and fix what's either a thinko or just an oversight in btcostestimate(): we should consider the number of key columns, not the total, when deciding whether to derate correlation. None of these things seemed important enough to risk changing in a just-before-wrap patch, but since we're past the release wrap window, time to fix 'em. Discussion: https://postgr.es/m/25526.1549847928@sss.pgh.pa.us
This commit is contained in:
parent
c2e0954be3
commit
364857f738
@ -252,7 +252,7 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
|
|||||||
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
|
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
|
||||||
|
|
||||||
/* Protect limited-size array in IndexClauseSets */
|
/* Protect limited-size array in IndexClauseSets */
|
||||||
Assert(index->ncolumns <= INDEX_MAX_KEYS);
|
Assert(index->nkeycolumns <= INDEX_MAX_KEYS);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Ignore partial indexes that do not match the query.
|
* Ignore partial indexes that do not match the query.
|
||||||
@ -467,7 +467,7 @@ consider_index_join_clauses(PlannerInfo *root, RelOptInfo *rel,
|
|||||||
* relation itself is also included in the relids set. considered_relids
|
* relation itself is also included in the relids set. considered_relids
|
||||||
* lists all relids sets we've already tried.
|
* lists all relids sets we've already tried.
|
||||||
*/
|
*/
|
||||||
for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
|
for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
|
||||||
{
|
{
|
||||||
/* Consider each applicable simple join clause */
|
/* Consider each applicable simple join clause */
|
||||||
considered_clauses += list_length(jclauseset->indexclauses[indexcol]);
|
considered_clauses += list_length(jclauseset->indexclauses[indexcol]);
|
||||||
@ -621,7 +621,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
|
|||||||
/* Identify indexclauses usable with this relids set */
|
/* Identify indexclauses usable with this relids set */
|
||||||
MemSet(&clauseset, 0, sizeof(clauseset));
|
MemSet(&clauseset, 0, sizeof(clauseset));
|
||||||
|
|
||||||
for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
|
for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
|
||||||
{
|
{
|
||||||
ListCell *lc;
|
ListCell *lc;
|
||||||
|
|
||||||
@ -921,7 +921,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
|
|||||||
clause_columns = NIL;
|
clause_columns = NIL;
|
||||||
found_lower_saop_clause = false;
|
found_lower_saop_clause = false;
|
||||||
outer_relids = bms_copy(rel->lateral_relids);
|
outer_relids = bms_copy(rel->lateral_relids);
|
||||||
for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
|
for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
|
||||||
{
|
{
|
||||||
ListCell *lc;
|
ListCell *lc;
|
||||||
|
|
||||||
@ -2649,11 +2649,12 @@ match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys,
|
|||||||
/*
|
/*
|
||||||
* We allow any column of the index to match each pathkey; they
|
* We allow any column of the index to match each pathkey; they
|
||||||
* don't have to match left-to-right as you might expect. This is
|
* don't have to match left-to-right as you might expect. This is
|
||||||
* correct for GiST, which is the sole existing AM supporting
|
* correct for GiST, and it doesn't matter for SP-GiST because
|
||||||
* amcanorderbyop. We might need different logic in future for
|
* that doesn't handle multiple columns anyway, and no other
|
||||||
* other implementations.
|
* existing AMs support amcanorderbyop. We might need different
|
||||||
|
* logic in future for other implementations.
|
||||||
*/
|
*/
|
||||||
for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
|
for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
|
||||||
{
|
{
|
||||||
Expr *expr;
|
Expr *expr;
|
||||||
|
|
||||||
@ -3084,7 +3085,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
|
|||||||
* Try to find each index column in the lists of conditions. This is
|
* Try to find each index column in the lists of conditions. This is
|
||||||
* O(N^2) or worse, but we expect all the lists to be short.
|
* O(N^2) or worse, but we expect all the lists to be short.
|
||||||
*/
|
*/
|
||||||
for (c = 0; c < ind->ncolumns; c++)
|
for (c = 0; c < ind->nkeycolumns; c++)
|
||||||
{
|
{
|
||||||
bool matched = false;
|
bool matched = false;
|
||||||
ListCell *lc;
|
ListCell *lc;
|
||||||
@ -3159,8 +3160,8 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
|
|||||||
break; /* no match; this index doesn't help us */
|
break; /* no match; this index doesn't help us */
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Matched all columns of this index? */
|
/* Matched all key columns of this index? */
|
||||||
if (c == ind->ncolumns)
|
if (c == ind->nkeycolumns)
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -4868,6 +4868,10 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
|
|||||||
* it to expressional index columns, in hopes of finding some
|
* it to expressional index columns, in hopes of finding some
|
||||||
* statistics.
|
* statistics.
|
||||||
*
|
*
|
||||||
|
* Note that we consider all index columns including INCLUDE columns,
|
||||||
|
* since there could be stats for such columns. But the test for
|
||||||
|
* uniqueness needs to be warier.
|
||||||
|
*
|
||||||
* XXX it's conceivable that there are multiple matches with different
|
* XXX it's conceivable that there are multiple matches with different
|
||||||
* index opfamilies; if so, we need to pick one that matches the
|
* index opfamilies; if so, we need to pick one that matches the
|
||||||
* operator we are estimating for. FIXME later.
|
* operator we are estimating for. FIXME later.
|
||||||
@ -4903,6 +4907,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
|
|||||||
*/
|
*/
|
||||||
if (index->unique &&
|
if (index->unique &&
|
||||||
index->nkeycolumns == 1 &&
|
index->nkeycolumns == 1 &&
|
||||||
|
pos == 0 &&
|
||||||
(index->indpred == NIL || index->predOK))
|
(index->indpred == NIL || index->predOK))
|
||||||
vardata->isunique = true;
|
vardata->isunique = true;
|
||||||
|
|
||||||
@ -7213,7 +7218,7 @@ btcostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
|
|||||||
if (index->reverse_sort[0])
|
if (index->reverse_sort[0])
|
||||||
varCorrelation = -varCorrelation;
|
varCorrelation = -varCorrelation;
|
||||||
|
|
||||||
if (index->ncolumns > 1)
|
if (index->nkeycolumns > 1)
|
||||||
costs.indexCorrelation = varCorrelation * 0.75;
|
costs.indexCorrelation = varCorrelation * 0.75;
|
||||||
else
|
else
|
||||||
costs.indexCorrelation = varCorrelation;
|
costs.indexCorrelation = varCorrelation;
|
||||||
|
Loading…
Reference in New Issue
Block a user