On 09/29/2015 04:57 PM, Tomas Vondra wrote:
Hello,
On 09/29/2015 12:27 PM, Kyotaro HORIGUCHI wrote:
...
cost_index() seems to need to be fixed. It would count excluded
clauses in estimate.
Hmm, good point. The problem is that extract_nonindex_conditions uses
baserel->baserestrictinfo again, i.e. it does not skip the implied
clauses. So we may either stick the filtered clauses somewhere (for
example in the IndexPath), teach extract_nonindex_conditions to use
predicate_implied_by. I'd say the first option is better. Agreed?
And the attached patch v4 should do the trick - it adds 'indexrinfos' to
IndexPath and uses it in cost_index().
CREATE TABLE t AS SELECT i AS a, i AS b, i AS c
FROM generate_series(1,1000) s(i);
CREATE INDEX idx ON t(a) WHERE b > 1000;
Then
SELECT a FROM t WHERE b > 1000 AND a < 1000; /* size(qpquals) = 0 */
SELECT a FROM t WHERE b > 1000 AND c < 1000; /* size(qpquals) = 1 */
SELECT a FROM t WHERE c > 1000 AND c < 1000; /* size(qpquals) = 2 */
and so on. Which seems correct I believe.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index d107d76..5feb965 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -86,6 +86,7 @@
#include "optimizer/placeholder.h"
#include "optimizer/plancat.h"
#include "optimizer/planmain.h"
+#include "optimizer/predtest.h"
#include "optimizer/restrictinfo.h"
#include "parser/parsetree.h"
#include "utils/lsyscache.h"
@@ -345,7 +346,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
path->path.rows = path->path.param_info->ppi_rows;
/* qpquals come from the rel's restriction clauses and ppi_clauses */
qpquals = list_concat(
- extract_nonindex_conditions(baserel->baserestrictinfo,
+ extract_nonindex_conditions(path->indexrinfos,
path->indexquals),
extract_nonindex_conditions(path->path.param_info->ppi_clauses,
path->indexquals));
@@ -354,7 +355,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count)
{
path->path.rows = baserel->rows;
/* qpquals come from just the rel's restriction clauses */
- qpquals = extract_nonindex_conditions(baserel->baserestrictinfo,
+ qpquals = extract_nonindex_conditions(path->indexrinfos,
path->indexquals);
}
@@ -558,6 +559,7 @@ extract_nonindex_conditions(List *qual_clauses, List *indexquals)
continue; /* simple duplicate */
if (is_redundant_derived_clause(rinfo, indexquals))
continue; /* derived from same EquivalenceClass */
+
/* ... skip the predicate proof attempts createplan.c will try ... */
result = lappend(result, rinfo);
}
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 9da5444..5f30aaa 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -59,6 +59,7 @@ typedef struct
bool nonempty; /* True if lists are not all empty */
/* Lists of RestrictInfos, one per index column */
List *indexclauses[INDEX_MAX_KEYS];
+ List *rclauses; /* clauses not implied by predicate */
} IndexClauseSet;
/* Per-path data used within choose_bitmap_and() */
@@ -129,7 +130,7 @@ static PathClauseUsage *classify_index_clause_usage(Path *path,
static Relids get_bitmap_tree_required_outer(Path *bitmapqual);
static void find_indexpath_quals(Path *bitmapqual, List **quals, List **preds);
static int find_list_position(Node *node, List **nodelist);
-static bool check_index_only(RelOptInfo *rel, IndexOptInfo *index);
+static bool check_index_only(RelOptInfo *rel, IndexOptInfo *index, List *clauses);
static double get_loop_count(PlannerInfo *root, Index cur_relid, Relids outer_relids);
static double adjust_rowcount_for_semijoins(PlannerInfo *root,
Index cur_relid,
@@ -866,6 +867,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
double loop_count;
List *orderbyclauses;
List *orderbyclausecols;
+ List *rinfos;
List *index_pathkeys;
List *useful_pathkeys;
bool found_lower_saop_clause;
@@ -1013,13 +1015,16 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
orderbyclausecols = NIL;
}
+ /* for partial indexes, use the non-implied RestrictInfo nodes */
+ rinfos = (index->indpred != NIL) ? clauses->rclauses : rel->baserestrictinfo;
+
/*
* 3. Check if an index-only scan is possible. If we're not building
* plain indexscans, this isn't relevant since bitmap scans don't support
* index data retrieval anyway.
*/
index_only_scan = (scantype != ST_BITMAPSCAN &&
- check_index_only(rel, index));
+ check_index_only(rel, index, rinfos));
/*
* 4. Generate an indexscan path if there are relevant restriction clauses
@@ -1033,6 +1038,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
ipath = create_index_path(root, index,
index_clauses,
clause_columns,
+ rinfos,
orderbyclauses,
orderbyclausecols,
useful_pathkeys,
@@ -1059,6 +1065,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
ipath = create_index_path(root, index,
index_clauses,
clause_columns,
+ rinfos,
NIL,
NIL,
useful_pathkeys,
@@ -1782,7 +1789,7 @@ find_list_position(Node *node, List **nodelist)
* Determine whether an index-only scan is possible for this index.
*/
static bool
-check_index_only(RelOptInfo *rel, IndexOptInfo *index)
+check_index_only(RelOptInfo *rel, IndexOptInfo *index, List *clauses)
{
bool result;
Bitmapset *attrs_used = NULL;
@@ -1798,13 +1805,13 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
* Check that all needed attributes of the relation are available from the
* index.
*
- * XXX this is overly conservative for partial indexes, since we will
- * consider attributes involved in the index predicate as required even
- * though the predicate won't need to be checked at runtime. (The same is
- * true for attributes used only in index quals, if we are certain that
- * the index is not lossy.) However, it would be quite expensive to
- * determine that accurately at this point, so for now we take the easy
- * way out.
+ * For partial indexes we won't consider attributes involved in clauses
+ * implied by the index predicate, as those won't be needed at runtime.
+ *
+ * XXX The same is true for attributes used only in index quals, if we
+ * are certain that the index is not lossy. However, it would be quite
+ * expensive to determine that accurately at this point, so for now we
+ * take the easy way out.
*/
/*
@@ -1814,8 +1821,11 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
*/
pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used);
- /* Add all the attributes used by restriction clauses. */
- foreach(lc, rel->baserestrictinfo)
+ /*
+ * Add all the attributes used by restriction clauses (only those not
+ * implied by the index predicate for partial indexes).
+ */
+ foreach(lc, clauses)
{
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
@@ -2136,6 +2146,30 @@ match_clause_to_index(IndexOptInfo *index,
{
int indexcol;
+
+ /*
+ * For partial indexes we skip clauses that are implied by the index
+ * predicate. We don't need to re-evaluate those, and the columns
+ * may not even be in the index itself, just in the predicate.
+ *
+ * We also keep clauses not implied by the index predicate so that
+ * check_index_only does not have to call predicate_implied_by again.
+ * For regular indexes we'll use baserestrictinfo list directly.
+ *
+ * XXX We must do this before trying to match the index to index
+ * columns, because the index predicates may use columns not
+ * used in the index itself.
+ */
+ if (index->indpred != NIL)
+ {
+ if (predicate_implied_by(list_make1(rinfo->clause),
+ index->indpred))
+ return; /* ignore clauses implied by index */
+
+ /* track clauses not implied */
+ clauseset->rclauses = lappend(clauseset->rclauses, rinfo);
+ }
+
for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
{
if (match_clause_to_indexcol(index,
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 06be922..7e23dcd 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4686,7 +4686,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid)
/* Estimate the cost of index scan */
indexScanPath = create_index_path(root, indexInfo,
- NIL, NIL, NIL, NIL, NIL,
+ NIL, NIL, NIL, NIL, NIL, NIL,
ForwardScanDirection, false,
NULL, 1.0);
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 935bc2b..f20944c 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -760,6 +760,7 @@ create_index_path(PlannerInfo *root,
IndexOptInfo *index,
List *indexclauses,
List *indexclausecols,
+ List *indexrinfos,
List *indexorderbys,
List *indexorderbycols,
List *pathkeys,
@@ -788,6 +789,7 @@ create_index_path(PlannerInfo *root,
pathnode->indexclauses = indexclauses;
pathnode->indexquals = indexquals;
pathnode->indexqualcols = indexqualcols;
+ pathnode->indexrinfos = indexrinfos;
pathnode->indexorderbys = indexorderbys;
pathnode->indexorderbycols = indexorderbycols;
pathnode->indexscandir = indexscandir;
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 79bed33..bc5f077 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -790,6 +790,10 @@ typedef struct Path
* index column, so 'indexqualcols' must form a nondecreasing sequence.
* (The order of multiple quals for the same index column is unspecified.)
*
+ * 'indexrinfos' is a list of RestrictInfo nodes from the query's WHERE
+ * or JOIN conditions, excluding those implied by the index predicate
+ * (if the index is not partial, the list includes all restriction clauses).
+ *
* 'indexorderbys', if not NIL, is a list of ORDER BY expressions that have
* been found to be usable as ordering operators for an amcanorderbyop index.
* The list must match the path's pathkeys, ie, one expression per pathkey
@@ -824,6 +828,7 @@ typedef struct IndexPath
List *indexclauses;
List *indexquals;
List *indexqualcols;
+ List *indexrinfos;
List *indexorderbys;
List *indexorderbycols;
ScanDirection indexscandir;
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index 161644c..43d97b1 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -38,6 +38,7 @@ extern IndexPath *create_index_path(PlannerInfo *root,
IndexOptInfo *index,
List *indexclauses,
List *indexclausecols,
+ List *indexrinfos,
List *indexorderbys,
List *indexorderbycols,
List *pathkeys,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
