On 09/02/2017 06:44 AM, Thomas Munro wrote:
On Wed, Aug 16, 2017 at 9:23 PM, Konstantin Knizhnik
<k.knizh...@postgrespro.ru> wrote:
postgres=# explain select * from bt where k between 1 and 20000 and v = 100;
QUERY PLAN
----------------------------------------------------------------------
Append (cost=0.29..15.63 rows=2 width=8)
-> Index Scan using dti1 on dt1 (cost=0.29..8.30 rows=1 width=8)
Index Cond: (v = 100)
-> Index Scan using dti2 on dt2 (cost=0.29..7.33 rows=1 width=8)
Index Cond: (v = 100)
Filter: (k <= 20000)
(6 rows)
+1
This seems like a good feature to me: filtering stuff that is
obviously true is a waste of CPU cycles and may even require people to
add redundant stuff to indexes. I was pondering something related to
this over in the partition-wise join thread (join quals that are
implied by partition constraints and should be discarded).
It'd be interesting to get Amit Langote's feedback, so I CC'd him.
I'd be surprised if he and others haven't got a plan or a patch for
this down the back of the sofa.
I might be missing some higher level architectural problems with the
patch, but for what it's worth here's some feedback after a first read
through:
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1441,6 +1441,20 @@ relation_excluded_by_constraints(PlannerInfo *root,
if (predicate_refuted_by(safe_constraints, rel->baserestrictinfo, false))
return true;
+ /*
+ * Remove from restrictions list items implied by table constraints
+ */
+ safe_restrictions = NULL;
+ foreach(lc, rel->baserestrictinfo)
+ {
+ RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
I think the new way to write this is "RestrictInfo *rinfo =
lfirst_node(RestrictInfo, lc)".
+ if (!predicate_implied_by(list_make1(rinfo->clause),
safe_constraints, false)) {
+ safe_restrictions = lappend(safe_restrictions, rinfo);
+ }
+ }
+ rel->baserestrictinfo = safe_restrictions;
It feels wrong to modify rel->baserestrictinfo in
relation_excluded_by_constraints(). I think there should probably be
a function with a name that more clearly indicates that it mutates its
input, and you should call that separately after
relation_excluded_by_constraints(). Something like
remove_restrictions_implied_by_constraints()?
It is because operator_predicate_proof is not able to understand that k <
20001 and k <= 20000 are equivalent for integer type.
[...]
/*
* operator_predicate_proof
if (clause_const->constisnull)
return false;
+ if (!refute_it
+ && ((pred_op == Int4LessOrEqualOperator && clause_op ==
Int4LessOperator)
+ || (pred_op == Int8LessOrEqualOperator && clause_op ==
Int8LessOperator)
+ || (pred_op == Int2LessOrEqualOperator && clause_op ==
Int2LessOperator))
+ && pred_const->constbyval && clause_const->constbyval
+ && pred_const->constvalue + 1 == clause_const->constvalue)
+ {
+ return true;
+ }
+
I'm less sure about this part. It seems like a slippery slope.
A couple of regression test failures:
inherit ... FAILED
rowsecurity ... FAILED
========================
2 of 179 tests failed.
========================
I didn't try to understand the rowsecurity one, but at first glance
all the differences reported in the inherit test are in fact cases
where your patch is doing the right thing and removing redundant
filters from scans. Nice!
Thank you for review.
I attached new version of the patch with
remove_restrictions_implied_by_constraints() function.
Concerning failed tests - this is actually result of this optimization: extra
filter conditions are removed from query plans.
Sorry, I have not included updated version of expected test output files to the
patch.
Now I did it.
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 2d7e1d8..5de67ce 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -344,6 +344,7 @@ set_rel_size(PlannerInfo *root, RelOptInfo *rel,
switch (rel->rtekind)
{
case RTE_RELATION:
+ remove_restrictions_implied_by_constraints(root, rel, rte);
if (rte->relkind == RELKIND_FOREIGN_TABLE)
{
/* Foreign table */
@@ -1047,6 +1048,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
set_dummy_rel_pathlist(childrel);
continue;
}
+ remove_restrictions_implied_by_constraints(root, childrel, childRTE);
/*
* CE failed, so finish copying/modifying targetlist and join quals.
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index a1ebd4a..fc663d6 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1444,6 +1444,51 @@ relation_excluded_by_constraints(PlannerInfo *root,
return false;
}
+/*
+ * Remove from restrictions list items implied by table constraints
+ */
+void remove_restrictions_implied_by_constraints(PlannerInfo *root,
+ RelOptInfo *rel, RangeTblEntry *rte)
+{
+ List *constraint_pred;
+ List *safe_constraints = NIL;
+ List *safe_restrictions = NIL;
+ ListCell *lc;
+
+ if (rte->rtekind != RTE_RELATION || rte->inh)
+ return;
+
+ /*
+ * OK to fetch the constraint expressions. Include "col IS NOT NULL"
+ * expressions for attnotnull columns, in case we can refute those.
+ */
+ constraint_pred = get_relation_constraints(root, rte->relid, rel, true);
+
+ /*
+ * We do not currently enforce that CHECK constraints contain only
+ * immutable functions, so it's necessary to check here. We daren't draw
+ * conclusions from plan-time evaluation of non-immutable functions. Since
+ * they're ANDed, we can just ignore any mutable constraints in the list,
+ * and reason about the rest.
+ */
+ foreach(lc, constraint_pred)
+ {
+ Node *pred = (Node*) lfirst(lc);
+
+ if (!contain_mutable_functions(pred))
+ safe_constraints = lappend(safe_constraints, pred);
+ }
+
+ foreach(lc, rel->baserestrictinfo)
+ {
+ RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
+ if (!predicate_implied_by(list_make1(rinfo->clause), safe_constraints, false)) {
+ safe_restrictions = lappend(safe_restrictions, rinfo);
+ }
+ }
+ rel->baserestrictinfo = safe_restrictions;
+}
+
/*
* build_physical_tlist
diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer/util/predtest.c
index 536d24b..54efbde 100644
--- a/src/backend/optimizer/util/predtest.c
+++ b/src/backend/optimizer/util/predtest.c
@@ -17,6 +17,7 @@
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+#include "catalog/pg_operator.h"
#include "executor/executor.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
@@ -1407,6 +1408,11 @@ static const StrategyNumber BT_refute_table[6][6] = {
{none, none, BTEQ, none, none, none} /* NE */
};
+#define Int2LessOperator 95
+#define Int2LessOrEqualOperator 522
+#define Int4LessOrEqualOperator 523
+#define Int8LessOrEqualOperator 414
+
/*
* operator_predicate_proof
@@ -1600,6 +1606,16 @@ operator_predicate_proof(Expr *predicate, Node *clause, bool refute_it)
if (clause_const->constisnull)
return false;
+ if (!refute_it
+ && ((pred_op == Int4LessOrEqualOperator && clause_op == Int4LessOperator)
+ || (pred_op == Int8LessOrEqualOperator && clause_op == Int8LessOperator)
+ || (pred_op == Int2LessOrEqualOperator && clause_op == Int2LessOperator))
+ && pred_const->constbyval && clause_const->constbyval
+ && pred_const->constvalue + 1 == clause_const->constvalue)
+ {
+ return true;
+ }
+
/*
* Lookup the constant-comparison operator using the system catalogs and
* the operator implication tables.
diff --git a/src/include/optimizer/plancat.h b/src/include/optimizer/plancat.h
index 71f0faf..09e8927 100644
--- a/src/include/optimizer/plancat.h
+++ b/src/include/optimizer/plancat.h
@@ -38,6 +38,9 @@ extern int32 get_relation_data_width(Oid relid, int32 *attr_widths);
extern bool relation_excluded_by_constraints(PlannerInfo *root,
RelOptInfo *rel, RangeTblEntry *rte);
+extern void remove_restrictions_implied_by_constraints(PlannerInfo *root,
+ RelOptInfo *rel, RangeTblEntry *rte);
+
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/inherit.out b/src/test/regress/expected/inherit.out
index 1fa9650..5610a4d 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1668,34 +1668,30 @@ explain (costs off) select * from list_parted where a is not null;
---------------------------------
Append
-> Seq Scan on part_ab_cd
- Filter: (a IS NOT NULL)
-> Seq Scan on part_ef_gh
- Filter: (a IS NOT NULL)
-> Seq Scan on part_null_xy
Filter: (a IS NOT NULL)
-(7 rows)
+(5 rows)
explain (costs off) select * from list_parted where a in ('ab', 'cd', 'ef');
QUERY PLAN
----------------------------------------------------------
Append
-> Seq Scan on part_ab_cd
- Filter: ((a)::text = ANY ('{ab,cd,ef}'::text[]))
-> Seq Scan on part_ef_gh
Filter: ((a)::text = ANY ('{ab,cd,ef}'::text[]))
-(5 rows)
+(4 rows)
explain (costs off) select * from list_parted where a = 'ab' or a in (null, 'cd');
QUERY PLAN
---------------------------------------------------------------------------------------
Append
-> Seq Scan on part_ab_cd
- Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY ('{NULL,cd}'::text[])))
-> Seq Scan on part_ef_gh
Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY ('{NULL,cd}'::text[])))
-> Seq Scan on part_null_xy
Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY ('{NULL,cd}'::text[])))
-(7 rows)
+(6 rows)
explain (costs off) select * from list_parted where a = 'ab';
QUERY PLAN
@@ -1748,30 +1744,25 @@ explain (costs off) select * from range_list_parted where a = 5;
(5 rows)
explain (costs off) select * from range_list_parted where b = 'ab';
- QUERY PLAN
-------------------------------------
+ QUERY PLAN
+----------------------------------
Append
-> Seq Scan on part_1_10_ab
- Filter: (b = 'ab'::bpchar)
-> Seq Scan on part_10_20_ab
- Filter: (b = 'ab'::bpchar)
-> Seq Scan on part_21_30_ab
- Filter: (b = 'ab'::bpchar)
-> Seq Scan on part_40_inf_ab
- Filter: (b = 'ab'::bpchar)
-(9 rows)
+(5 rows)
explain (costs off) select * from range_list_parted where a between 3 and 23 and b in ('ab');
- QUERY PLAN
------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------
Append
-> Seq Scan on part_1_10_ab
- Filter: ((a >= 3) AND (a <= 23) AND (b = 'ab'::bpchar))
+ Filter: (a >= 3)
-> Seq Scan on part_10_20_ab
- Filter: ((a >= 3) AND (a <= 23) AND (b = 'ab'::bpchar))
-> Seq Scan on part_21_30_ab
- Filter: ((a >= 3) AND (a <= 23) AND (b = 'ab'::bpchar))
-(7 rows)
+ Filter: (a <= 23)
+(6 rows)
/* Should select no rows because range partition key cannot be null */
explain (costs off) select * from range_list_parted where a is null;
@@ -1787,44 +1778,34 @@ explain (costs off) select * from range_list_parted where b is null;
------------------------------------
Append
-> Seq Scan on part_40_inf_null
- Filter: (b IS NULL)
-(3 rows)
+(2 rows)
explain (costs off) select * from range_list_parted where a is not null and a < 67;
- QUERY PLAN
-------------------------------------------------
+ QUERY PLAN
+------------------------------------
Append
-> Seq Scan on part_1_10_ab
- Filter: ((a IS NOT NULL) AND (a < 67))
-> Seq Scan on part_1_10_cd
- Filter: ((a IS NOT NULL) AND (a < 67))
-> Seq Scan on part_10_20_ab
- Filter: ((a IS NOT NULL) AND (a < 67))
-> Seq Scan on part_10_20_cd
- Filter: ((a IS NOT NULL) AND (a < 67))
-> Seq Scan on part_21_30_ab
- Filter: ((a IS NOT NULL) AND (a < 67))
-> Seq Scan on part_21_30_cd
- Filter: ((a IS NOT NULL) AND (a < 67))
-> Seq Scan on part_40_inf_ab
- Filter: ((a IS NOT NULL) AND (a < 67))
+ Filter: (a < 67)
-> Seq Scan on part_40_inf_cd
- Filter: ((a IS NOT NULL) AND (a < 67))
+ Filter: (a < 67)
-> Seq Scan on part_40_inf_null
- Filter: ((a IS NOT NULL) AND (a < 67))
-(19 rows)
+ Filter: (a < 67)
+(13 rows)
explain (costs off) select * from range_list_parted where a >= 30;
QUERY PLAN
------------------------------------
Append
-> Seq Scan on part_40_inf_ab
- Filter: (a >= 30)
-> Seq Scan on part_40_inf_cd
- Filter: (a >= 30)
-> Seq Scan on part_40_inf_null
- Filter: (a >= 30)
-(7 rows)
+(4 rows)
drop table list_parted;
drop table range_list_parted;
@@ -1860,7 +1841,7 @@ explain (costs off) select * from mcrparted where a = 10 and abs(b) = 5; -- scan
-> Seq Scan on mcrparted1
Filter: ((a = 10) AND (abs(b) = 5))
-> Seq Scan on mcrparted2
- Filter: ((a = 10) AND (abs(b) = 5))
+ Filter: (abs(b) = 5)
(5 rows)
explain (costs off) select * from mcrparted where abs(b) = 5; -- scans all partitions
@@ -1886,23 +1867,18 @@ explain (costs off) select * from mcrparted where a > -1; -- scans all partition
-> Seq Scan on mcrparted0
Filter: (a > '-1'::integer)
-> Seq Scan on mcrparted1
- Filter: (a > '-1'::integer)
-> Seq Scan on mcrparted2
- Filter: (a > '-1'::integer)
-> Seq Scan on mcrparted3
- Filter: (a > '-1'::integer)
-> Seq Scan on mcrparted4
- Filter: (a > '-1'::integer)
-> Seq Scan on mcrparted5
- Filter: (a > '-1'::integer)
-(13 rows)
+(8 rows)
explain (costs off) select * from mcrparted where a = 20 and abs(b) = 10 and c > 10; -- scans mcrparted4
- QUERY PLAN
------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------
Append
-> Seq Scan on mcrparted4
- Filter: ((c > 10) AND (a = 20) AND (abs(b) = 10))
+ Filter: ((c > 10) AND (abs(b) = 10))
(3 rows)
explain (costs off) select * from mcrparted where a = 20 and c > 20; -- scans mcrparted3, mcrparte4, mcrparte5
@@ -1912,7 +1888,7 @@ explain (costs off) select * from mcrparted where a = 20 and c > 20; -- scans mc
-> Seq Scan on mcrparted3
Filter: ((c > 20) AND (a = 20))
-> Seq Scan on mcrparted4
- Filter: ((c > 20) AND (a = 20))
+ Filter: (c > 20)
-> Seq Scan on mcrparted5
Filter: ((c > 20) AND (a = 20))
(7 rows)
@@ -1933,13 +1909,13 @@ explain (costs off) select min(a), max(a) from parted_minmax where b = '12345';
-> Merge Append
Sort Key: parted_minmax1.a
-> Index Only Scan using parted_minmax1i on parted_minmax1
- Index Cond: ((a IS NOT NULL) AND (b = '12345'::text))
+ Index Cond: (b = '12345'::text)
InitPlan 2 (returns $1)
-> Limit
-> Merge Append
Sort Key: parted_minmax1_1.a DESC
-> Index Only Scan Backward using parted_minmax1i on parted_minmax1 parted_minmax1_1
- Index Cond: ((a IS NOT NULL) AND (b = '12345'::text))
+ Index Cond: (b = '12345'::text)
(13 rows)
select min(a), max(a) from parted_minmax where b = '12345';
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index de2ee4d..dee7443 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1057,14 +1057,14 @@ NOTICE: f_leak => awesome science fiction
(4 rows)
EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle);
- QUERY PLAN
---------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------
Append
InitPlan 1 (returns $0)
-> Index Scan using uaccount_pkey on uaccount
Index Cond: (pguser = CURRENT_USER)
-> Seq Scan on part_document_fiction
- Filter: ((cid < 55) AND (dlevel <= $0) AND f_leak(dtitle))
+ Filter: ((dlevel <= $0) AND f_leak(dtitle))
(6 rows)
-- pp1 ERROR
@@ -1136,14 +1136,14 @@ NOTICE: f_leak => awesome science fiction
(4 rows)
EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle);
- QUERY PLAN
---------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------
Append
InitPlan 1 (returns $0)
-> Index Scan using uaccount_pkey on uaccount
Index Cond: (pguser = CURRENT_USER)
-> Seq Scan on part_document_fiction
- Filter: ((cid < 55) AND (dlevel <= $0) AND f_leak(dtitle))
+ Filter: ((dlevel <= $0) AND f_leak(dtitle))
(6 rows)
-- viewpoint from regress_rls_carol
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
