Hi Justin,
Attached is a minimal patch that is rebased and removes the
dependency on Konstantin's earlier patch[1], making it enough to remove
the extraneous index scans as Justin brought up. Is this the direction we
want to head in?
Tagging Konstantin in the email to hear his input on his old patch.
Since Justin's attached patch [1] does not include the work that was done
on the operator_predicate_proof() and as discussed here in [2], that
work is important to see real benefits? Just wanted to check before
reviewing [1].
Regards,
Soumyadeep (VMware)
[1]
https://www.postgresql.org/message-id/attachment/112074/0001-Secondary-index-access-optimizations.patch
[2] https://www.postgresql.org/message-id/5A006016.1010009%40postgrespro.ru
From c60d00a64c7b65d785cda1aad7e780388ab32e27 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 29 Sep 2020 17:09:45 -0700
Subject: [PATCH v3 1/1] Avoid index scan inconsistent with partition
constraint
---
src/backend/optimizer/path/allpaths.c | 7 ++++++
src/backend/optimizer/path/indxpath.c | 5 +++++
src/backend/optimizer/util/plancat.c | 7 +-----
src/include/optimizer/plancat.h | 6 ++++++
src/test/regress/expected/create_index.out | 25 ++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 10 +++++++++
6 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index b399592ff815..a6a643b9091f 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1045,6 +1045,13 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
continue;
}
+ /*
+ * Ensure that rel->partition_qual is set, so we can use the information
+ * to eliminate unnecessary index scans.
+ */
+ if(childRTE->relid != InvalidOid)
+ get_relation_constraints(root, childRTE->relid, childrel, false, false, true);
+
/*
* Constraint exclusion failed, so copy the parent's join quals and
* targetlist to the child, with appropriate variable substitutions.
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index bcb1bc6097d0..0532b3ddd0d6 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
Assert(!restriction_is_or_clause(rinfo));
orargs = list_make1(rinfo);
+ /* Avoid scanning indexes using a scan condition which is
+ * inconsistent with the partition constraint */
+ if (predicate_refuted_by(rel->partition_qual, orargs, false))
+ continue;
+
indlist = build_paths_for_OR(root, rel,
orargs,
all_clauses);
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index f9d0d67aa75a..1e0bac471ff3 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -64,11 +64,6 @@ static void get_relation_foreign_keys(PlannerInfo *root, RelOptInfo *rel,
Relation relation, bool inhparent);
static bool infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
List *idxExprs);
-static List *get_relation_constraints(PlannerInfo *root,
- Oid relationObjectId, RelOptInfo *rel,
- bool include_noinherit,
- bool include_notnull,
- bool include_partition);
static List *build_index_tlist(PlannerInfo *root, IndexOptInfo *index,
Relation heapRelation);
static List *get_relation_statistics(RelOptInfo *rel, Relation relation);
@@ -1165,7 +1160,7 @@ get_relation_data_width(Oid relid, int32 *attr_widths)
* run, and in many cases it won't be invoked at all, so there seems no
* point in caching the data in RelOptInfo.
*/
-static List *
+List *
get_relation_constraints(PlannerInfo *root,
Oid relationObjectId, RelOptInfo *rel,
bool include_noinherit,
diff --git a/src/include/optimizer/plancat.h b/src/include/optimizer/plancat.h
index c29a7091ec04..cadfee15a34a 100644
--- a/src/include/optimizer/plancat.h
+++ b/src/include/optimizer/plancat.h
@@ -39,6 +39,12 @@ extern int32 get_relation_data_width(Oid relid, int32 *attr_widths);
extern bool relation_excluded_by_constraints(PlannerInfo *root,
RelOptInfo *rel, RangeTblEntry *rte);
+extern List *get_relation_constraints(PlannerInfo *root,
+ Oid relationObjectId, RelOptInfo *rel,
+ bool include_noinherit,
+ bool include_notnull,
+ bool include_partition);
+
extern List *build_physical_tlist(PlannerInfo *root, RelOptInfo *rel);
extern bool has_unique_index(RelOptInfo *rel, AttrNumber attno);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 6ace7662ee1f..3d67978232d3 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1
10
(1 row)
+-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint
+CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i);
+CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10);
+CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20);
+INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i;
+VACUUM ANALYZE bitmapor;
+CREATE INDEX ON bitmapor(i);
+EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11);
+ QUERY PLAN
+--------------------------------------------------------
+ Append
+ -> Bitmap Heap Scan on bitmapor1 bitmapor_1
+ Recheck Cond: ((i = 1) OR (i = 2))
+ -> BitmapOr
+ -> Bitmap Index Scan on bitmapor1_i_idx
+ Index Cond: (i = 1)
+ -> Bitmap Index Scan on bitmapor1_i_idx
+ Index Cond: (i = 2)
+ -> Bitmap Heap Scan on bitmapor2 bitmapor_2
+ Recheck Cond: (i = 11)
+ -> Bitmap Index Scan on bitmapor2_i_idx
+ Index Cond: (i = 11)
+(12 rows)
+
+DROP TABLE bitmapor;
--
-- Check behavior with duplicate index column contents
--
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 37f7259da9e6..1c306779ba35 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
+-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint
+CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i);
+CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10);
+CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20);
+INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i;
+VACUUM ANALYZE bitmapor;
+CREATE INDEX ON bitmapor(i);
+EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11);
+DROP TABLE bitmapor;
+
--
-- Check behavior with duplicate index column contents
--
--
2.27.0
