Hello,
At Thu, 01 Oct 2015 01:36:51 +0200, Tomas Vondra <tomas.von...@2ndquadrant.com>
wrote in <560c7213.3010...@2ndquadrant.com>
> > 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.).
Looks good (for me).
> 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),
Although it is not the cause of these errors (or any errors on
regress), build_paths_for_OR (which doesn't seem to be called in
regression tests) doesn't set indexrinfos
properly. match_clauses_to_index can commonize(?) the process in
return for additional list copy and concat. The attached diff is
doing in the latter method. Avoiding the copies will make the
code a bit complicated.
But anyway regtests doesn't say whether it is sane or not:( (TODO)
> 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.
During considerartion on parameterized joins in
get_join_index_paths, caluseset fed to get_index_paths is
generated from given join (j|e)clausesets. So completing the
clauseset with necessary restrictinfo from rcaluseset fixes the
problem.
The attached patch applies on the latest v5 patch and will
address above issues. (And modifies expected files, which are the
manifestation of this improovement).
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index c5c2b6e..105351f 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -662,6 +662,13 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
/* We should have found something, else caller passed silly relids */
Assert(clauseset.nonempty);
+ /*
+ * get_index_paths requires that restririctinfo for the index is stored in
+ * clauseset->indexrinfos. Since it doesn't contain join clauses, just
+ * copying that of rclauseset is enough.
+ */
+ clauseset.indexrinfos = rclauseset->indexrinfos;
+
/* Build index path(s) using the collected set of clauses */
get_index_paths(root, rel, index, &clauseset, bitindexpaths);
@@ -867,7 +874,6 @@ 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;
@@ -1015,16 +1021,13 @@ 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, restrictinfo));
+ check_index_only(rel, index, clauses->indexrinfos));
/*
* 4. Generate an indexscan path if there are relevant restriction clauses
@@ -1038,7 +1041,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
ipath = create_index_path(root, index,
index_clauses,
clause_columns,
- restrictinfo,
+ clauses->indexrinfos,
orderbyclauses,
orderbyclausecols,
useful_pathkeys,
@@ -1065,7 +1068,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
ipath = create_index_path(root, index,
index_clauses,
clause_columns,
- restrictinfo,
+ clauses->indexrinfos,
NIL,
NIL,
useful_pathkeys,
@@ -2121,6 +2124,11 @@ match_clauses_to_index(IndexOptInfo *index,
Assert(IsA(rinfo, RestrictInfo));
match_clause_to_index(index, rinfo, clauseset);
}
+
+ /* copy clauses so that it won't be broken on concatenation afterward */
+ if (index->indpred == NIL)
+ clauseset->indexrinfos =
+ list_concat(clauseset->indexrinfos, list_copy(clauses));
}
/*
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index de826b5..88f6a02 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -780,7 +780,6 @@ explain (costs off)
-> Index Only Scan Backward using minmaxtest2i on minmaxtest2
Index Cond: (f1 IS NOT NULL)
-> Index Only Scan using minmaxtest3i on minmaxtest3
- Index Cond: (f1 IS NOT NULL)
InitPlan 2 (returns $1)
-> Limit
-> Merge Append
@@ -792,8 +791,7 @@ explain (costs off)
-> Index Only Scan using minmaxtest2i on minmaxtest2 minmaxtest2_1
Index Cond: (f1 IS NOT NULL)
-> Index Only Scan Backward using minmaxtest3i on minmaxtest3 minmaxtest3_1
- Index Cond: (f1 IS NOT NULL)
-(25 rows)
+(23 rows)
select min(f1), max(f1) from minmaxtest;
min | max
@@ -819,7 +817,6 @@ explain (costs off)
-> Index Only Scan Backward using minmaxtest2i on minmaxtest2
Index Cond: (f1 IS NOT NULL)
-> Index Only Scan using minmaxtest3i on minmaxtest3
- Index Cond: (f1 IS NOT NULL)
InitPlan 2 (returns $1)
-> Limit
-> Merge Append
@@ -831,9 +828,8 @@ explain (costs off)
-> Index Only Scan using minmaxtest2i on minmaxtest2 minmaxtest2_1
Index Cond: (f1 IS NOT NULL)
-> Index Only Scan Backward using minmaxtest3i on minmaxtest3 minmaxtest3_1
- Index Cond: (f1 IS NOT NULL)
-> Result
-(27 rows)
+(25 rows)
select distinct min(f1), max(f1) from minmaxtest;
min | max
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
