On 09/30/2015 12:55 PM, Tomas Vondra wrote:
Hello!
On 09/30/2015 10:29 AM, Kyotaro HORIGUCHI wrote:
By the way your comment for indexrinfos is as following,
* '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).
But the v4 patch instead leaves it empty for non-partial
indexes:) I prefer to follow this comment because looking the
condition (index->indpred != NIL) for such purpose in
build_index_paths is somewhat uneasy for me. But I don't insist
on that if you choose to avoid useless memory and clock
consumption to construct a list which is not so meaningful for
non-partial indexes (it is almost all cases).
Good point. I think we may simply point indexrinfos to the existing list
of restrictions in that case - we don't need to copy it. So no
additional memory / CPU consumption, and it should make the code working
with it a bit simpler.
Attached is v5 of the patch improving the comments (rephrasing, moving
before function etc.).
I also attempted to fix the issue with empty list for non-partial
indexes by simply setting it to rel->baserestrictinfo in
match_restriction_clauses_to_index (for non-partial indexes), but that
results in a bunch of
ERROR: variable not found in subplan target list
during "make installcheck". I can't quite wrap my head around why.
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..c5c2b6e 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 *indexrinfos; /* 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 *restrictinfo;
List *index_pathkeys;
List *useful_pathkeys;
bool found_lower_saop_clause;
@@ -1013,13 +1015,16 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
orderbyclausecols = NIL;
}
+ restrictinfo
+ = (index->indpred != NIL) ? clauses->indexrinfos : 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, restrictinfo));
/*
* 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,
+ restrictinfo,
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,
+ restrictinfo,
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);
@@ -2120,6 +2130,14 @@ match_clauses_to_index(IndexOptInfo *index,
* If the clause is usable, add it to the appropriate list in *clauseset.
* *clauseset must be initialized to zeroes before first call.
*
+ * For partial indexes we ignore clauses that are implied by the index
+ * predicate - no need to to re-evaluate those, and the columns may not
+ * even be included in the index itself.
+ *
+ * We also build a list of clauses that are not implied by the index
+ * predicate so that we don't need calling predicate_implied_by again
+ * (e.g. in check_index_only).
+ *
* Note: in some circumstances we may find the same RestrictInfos coming from
* multiple places. Defend against redundant outputs by refusing to add a
* clause twice (pointer equality should be a good enough check for this).
@@ -2136,6 +2154,16 @@ match_clause_to_index(IndexOptInfo *index,
{
int indexcol;
+ if (index->indpred != NIL)
+ {
+ if (predicate_implied_by(list_make1(rinfo->clause),
+ index->indpred))
+ return;
+
+ /* track non-implied restriction clauses */
+ clauseset->indexrinfos = lappend(clauseset->indexrinfos, 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 e1ee67c..8562c31 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4687,7 +4687,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 4336ca1..80bcc54 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 961b5d1..415585b 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -792,6 +792,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
@@ -826,6 +830,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
