Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit eef8c00

Browse files
committed
Clean up bugs in clause_selectivity() cleanup.
Commit ac2b095 was not terribly carefully reviewed. Band-aid it to not fail on non-RestrictInfo input, per report from Andreas Seltenreich. Also make it do something more reasonable with variable-free clauses, and improve nearby comments. Discussion: https://postgr.es/m/87inmf5rdx.fsf@credativ.de
1 parent aba696d commit eef8c00

File tree

1 file changed

+42
-39
lines changed

1 file changed

+42
-39
lines changed

src/backend/optimizer/path/clausesel.c

+42-39
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ typedef struct RangeQueryClause
4141

4242
static void addRangeClause(RangeQueryClause **rqlist, Node *clause,
4343
bool varonleft, bool isLTsel, Selectivity s2);
44-
static RelOptInfo *find_relation_from_clauses(PlannerInfo *root,
45-
List *clauses);
44+
static RelOptInfo *find_single_rel_for_clauses(PlannerInfo *root,
45+
List *clauses);
4646

4747
/****************************************************************************
4848
* ROUTINES TO COMPUTE SELECTIVITIES
@@ -63,10 +63,8 @@ static RelOptInfo *find_relation_from_clauses(PlannerInfo *root,
6363
* probabilities, and in reality they are often NOT independent. So,
6464
* we want to be smarter where we can.
6565
*
66-
* When 'rel' is not null and rtekind = RTE_RELATION, we'll try to apply
67-
* selectivity estimates using any extended statistcs on 'rel'.
68-
*
69-
* If we identify such extended statistics exist, we try to apply them.
66+
* If the clauses taken together refer to just one relation, we'll try to
67+
* apply selectivity estimates using any extended statistics for that rel.
7068
* Currently we only have (soft) functional dependencies, so apply these in as
7169
* many cases as possible, and fall back on normal estimates for remaining
7270
* clauses.
@@ -105,41 +103,36 @@ clauselist_selectivity(PlannerInfo *root,
105103
SpecialJoinInfo *sjinfo)
106104
{
107105
Selectivity s1 = 1.0;
106+
RelOptInfo *rel;
107+
Bitmapset *estimatedclauses = NULL;
108108
RangeQueryClause *rqlist = NULL;
109109
ListCell *l;
110-
Bitmapset *estimatedclauses = NULL;
111110
int listidx;
112-
RelOptInfo *rel;
113111

114112
/*
115-
* If there's exactly one clause, then extended statistics is futile at
116-
* this level (we might be able to apply them later if it's AND/OR
117-
* clause). So just go directly to clause_selectivity().
113+
* If there's exactly one clause, just go directly to
114+
* clause_selectivity(). None of what we might do below is relevant.
118115
*/
119116
if (list_length(clauses) == 1)
120117
return clause_selectivity(root, (Node *) linitial(clauses),
121118
varRelid, jointype, sjinfo);
122119

123120
/*
124-
* Determine if these clauses reference a single relation. If so we might
125-
* like to try applying any extended statistics which exist on it.
126-
* Called many time during joins, so must return NULL quickly if not.
127-
*/
128-
rel = find_relation_from_clauses(root, clauses);
129-
130-
/*
131-
* When a relation of RTE_RELATION is given as 'rel', we'll try to
132-
* perform selectivity estimation using extended statistics.
121+
* Determine if these clauses reference a single relation. If so, and if
122+
* it has extended statistics, try to apply those.
133123
*/
124+
rel = find_single_rel_for_clauses(root, clauses);
134125
if (rel && rel->rtekind == RTE_RELATION && rel->statlist != NIL)
135126
{
136127
/*
137128
* Perform selectivity estimations on any clauses found applicable by
138-
* dependencies_clauselist_selectivity. The 0-based list position of
139-
* estimated clauses will be populated in 'estimatedclauses'.
129+
* dependencies_clauselist_selectivity. 'estimatedclauses' will be
130+
* filled with the 0-based list positions of clauses used that way, so
131+
* that we can ignore them below.
140132
*/
141133
s1 *= dependencies_clauselist_selectivity(root, clauses, varRelid,
142-
jointype, sjinfo, rel, &estimatedclauses);
134+
jointype, sjinfo, rel,
135+
&estimatedclauses);
143136

144137
/*
145138
* This would be the place to apply any other types of extended
@@ -426,36 +419,46 @@ addRangeClause(RangeQueryClause **rqlist, Node *clause,
426419
}
427420

428421
/*
429-
* find_relation_from_clauses
430-
* Process each clause in 'clauses' and determine if all clauses
431-
* reference only a single relation. If so return that relation,
422+
* find_single_rel_for_clauses
423+
* Examine each clause in 'clauses' and determine if all clauses
424+
* reference only a single relation. If so return that relation,
432425
* otherwise return NULL.
433426
*/
434427
static RelOptInfo *
435-
find_relation_from_clauses(PlannerInfo *root, List *clauses)
428+
find_single_rel_for_clauses(PlannerInfo *root, List *clauses)
436429
{
437-
ListCell *l;
438-
int relid;
439-
int lastrelid = 0;
430+
int lastrelid = 0;
431+
ListCell *l;
440432

441433
foreach(l, clauses)
442434
{
443435
RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
436+
int relid;
444437

445-
if (bms_get_singleton_member(rinfo->clause_relids, &relid))
446-
{
447-
if (lastrelid != 0 && relid != lastrelid)
448-
return NULL; /* relation not the same as last one */
449-
lastrelid = relid;
450-
}
451-
else
452-
return NULL; /* multiple relations in clause */
438+
/*
439+
* If we have a list of bare clauses rather than RestrictInfos, we
440+
* could pull out their relids the hard way with pull_varnos().
441+
* However, currently the extended-stats machinery won't do anything
442+
* with non-RestrictInfo clauses anyway, so there's no point in
443+
* spending extra cycles; just fail if that's what we have.
444+
*/
445+
if (!IsA(rinfo, RestrictInfo))
446+
return NULL;
447+
448+
if (bms_is_empty(rinfo->clause_relids))
449+
continue; /* we can ignore variable-free clauses */
450+
if (!bms_get_singleton_member(rinfo->clause_relids, &relid))
451+
return NULL; /* multiple relations in this clause */
452+
if (lastrelid == 0)
453+
lastrelid = relid; /* first clause referencing a relation */
454+
else if (relid != lastrelid)
455+
return NULL; /* relation not same as last one */
453456
}
454457

455458
if (lastrelid != 0)
456459
return find_base_rel(root, lastrelid);
457460

458-
return NULL; /* no clauses */
461+
return NULL; /* no clauses */
459462
}
460463

461464
/*

0 commit comments

Comments
 (0)