Amit Langote wrote:
> On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
> <alvhe...@alvh.no-ip.org> wrote:
> > Makes sense. Still, I was expecting that pruning of hash partitioning
> > would also work for pseudotypes, yet it doesn't.
>
> It does?
Aha, so it does.
While staring at this new code, I was confused as to why we didn't use
the commutator if the code above had determined one. I was unable to
cause a test to fail, so I put that thought aside.
Some time later, after restructuring the code in a way that seemed to
make more sense to me (and saving one get_op_opfamily_properties call
for the case of the not-equals operator), I realized that with the new
code we can store the opstrategy in the PartClause instead of leaving it
as Invalid and look it up again later, so I did that. And lo and
behold, the tests that used commutators started failing! So I fixed
that one in the obvious way, and the tests work fully again.
Please give this version another look. I also rewrote a couple of
comments.
I now wonder if there's anything else that equivclass.c or indxpath.c
can teach us on this topic.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 340b0b3a97af0fa07562bc4434a4908402c3efbd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Wed, 18 Apr 2018 18:18:51 -0300
Subject: [PATCH v5] Fix pruning code to determine comparison function
correctly
It's unreliable to determine one using the constant expression's
type directly (for example, it doesn't work correctly when the
expression contains an array, enum, etc.). Instead, use righttype
of the operator, the one that supposedly passed the op_in_opfamily
test using the partitioning opfamily.
---
src/backend/partitioning/partprune.c | 101 +++++++++++--------
src/test/regress/expected/partition_prune.out | 138 ++++++++++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 53 ++++++++++
3 files changed, 250 insertions(+), 42 deletions(-)
diff --git a/src/backend/partitioning/partprune.c
b/src/backend/partitioning/partprune.c
index 7666c6c412..b8a006e774 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1426,10 +1426,12 @@ match_clause_to_partition_key(RelOptInfo *rel,
OpExpr *opclause = (OpExpr *) clause;
Expr *leftop,
*rightop;
- Oid commutator = InvalidOid,
+ Oid op_lefttype,
+ op_righttype,
+ commutator = InvalidOid,
negator = InvalidOid;
Oid cmpfn;
- Oid exprtype;
+ int op_strategy;
bool is_opne_listp = false;
PartClauseInfo *partclause;
@@ -1483,33 +1485,39 @@ match_clause_to_partition_key(RelOptInfo *rel,
return PARTCLAUSE_UNSUPPORTED;
/*
- * Normally we only bother with operators that are listed as
being
- * part of the partitioning operator family. But we make an
exception
- * in one case -- operators named '<>' are not listed in any
operator
- * family whatsoever, in which case, we try to perform partition
- * pruning with it only if list partitioning is in use.
+ * Determine the input types of the operator we're considering.
+ *
+ * Normally we only care about operators that are listed as
being part
+ * of the partitioning operator family. But there is one
exception:
+ * the not-equals operators are not listed in any operator
family
+ * whatsoever, but we can find their negator in the opfamily
and it's
+ * an equality op. We can use one of those if we find it, but
they
+ * are only useful for list partitioning.
*/
- if (!op_in_opfamily(opclause->opno, partopfamily))
+ if (op_in_opfamily(opclause->opno, partopfamily))
{
+ Oid oper;
+
+ oper = OidIsValid(commutator) ? commutator :
opclause->opno;
+ get_op_opfamily_properties(oper, partopfamily, false,
+
&op_strategy, &op_lefttype,
+
&op_righttype);
+ }
+ else
+ {
+ /* Not good unless list partitioning */
if (part_scheme->strategy != PARTITION_STRATEGY_LIST)
return PARTCLAUSE_UNSUPPORTED;
- /*
- * To confirm if the operator is really '<>', check if
its negator
- * is a btree equality operator.
- */
+ /* See if the negator is equality */
negator = get_negator(opclause->opno);
if (OidIsValid(negator) && op_in_opfamily(negator,
partopfamily))
{
- Oid lefttype;
- Oid righttype;
- int strategy;
-
get_op_opfamily_properties(negator,
partopfamily, false,
-
&strategy, &lefttype, &righttype);
-
- if (strategy == BTEqualStrategyNumber)
- is_opne_listp = true;
+
&op_strategy, &op_lefttype,
+
&op_righttype);
+ if (op_strategy == BTEqualStrategyNumber)
+ is_opne_listp = true; /* bingo */
}
/* Operator isn't really what we were hoping it'd be. */
@@ -1517,24 +1525,41 @@ match_clause_to_partition_key(RelOptInfo *rel,
return PARTCLAUSE_UNSUPPORTED;
}
- /* Check if we're going to need a cross-type comparison
function. */
- exprtype = exprType((Node *) expr);
- if (exprtype != part_scheme->partopcintype[partkeyidx])
+ /*
+ * Now find the procedure to use, based on the types. If the
clause's
+ * other argument is of the same type as the partitioning
opclass's
+ * declared input type, we can use the procedure cached in
+ * PartitionKey. If not, search for a cross-type one in the
same
+ * opfamily; if one doesn't exist, give up on pruning for this
clause.
+ */
+ if (op_righttype == part_scheme->partopcintype[partkeyidx])
+ cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+ else
{
switch (part_scheme->strategy)
{
+ /*
+ * For range and list partitioning, we need the
ordering
+ * procedure with lefttype being the partition
key's type, and
+ * righttype the clause's operator's right type.
+ */
case PARTITION_STRATEGY_LIST:
case PARTITION_STRATEGY_RANGE:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
part_scheme->partopcintype[partkeyidx],
-
exprtype, BTORDER_PROC);
+
op_righttype, BTORDER_PROC);
break;
+ /*
+ * For hash partitioning, we need the hashing
procedure for
+ * the clause's type.
+ */
case PARTITION_STRATEGY_HASH:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
-
exprtype, exprtype, HASHEXTENDED_PROC);
+
op_righttype, op_righttype,
+
HASHEXTENDED_PROC);
break;
default:
@@ -1547,34 +1572,26 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (!OidIsValid(cmpfn))
return PARTCLAUSE_UNSUPPORTED;
}
- else
- cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+ /*
+ * Build the clause, passing the negator or commutator if
applicable.
+ */
partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
partclause->keyno = partkeyidx;
-
- /* For <> operator clauses, pass on the negator. */
- partclause->op_is_ne = false;
- partclause->op_strategy = InvalidStrategy;
-
if (is_opne_listp)
{
Assert(OidIsValid(negator));
partclause->opno = negator;
partclause->op_is_ne = true;
-
- /*
- * We already know the strategy in this case, so may as
well set
- * it rather than having to look it up later.
- */
partclause->op_strategy = BTEqualStrategyNumber;
}
- /* And if commuted before matching, pass on the commutator */
- else if (OidIsValid(commutator))
- partclause->opno = commutator;
else
- partclause->opno = opclause->opno;
-
+ {
+ partclause->opno = OidIsValid(commutator) ?
+ commutator : opclause->opno;
+ partclause->op_is_ne = false;
+ partclause->op_strategy = op_strategy;
+ }
partclause->expr = expr;
partclause->cmpfn = cmpfn;
diff --git a/src/test/regress/expected/partition_prune.out
b/src/test/regress/expected/partition_prune.out
index 844cfb3e42..9c65ee001d 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2599,3 +2599,141 @@ select * from boolp where a = (select value from
boolvalues where not value);
drop table boolp;
reset enable_indexonlyscan;
+--
+-- check that pruning works properly when the partition key is of a
+-- pseudotype
+--
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4,
5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pp_arrpart where a = '{1, 2}';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
+ QUERY PLAN
+----------------------------------------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+ -> Seq Scan on pp_arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(5 rows)
+
+drop table pp_arrpart;
+-- array type hash partition key
+create table pph_arrpart (a int[]) partition by hash (a);
+create table pph_arrpart1 partition of pph_arrpart for values with (modulus 2,
remainder 0);
+create table pph_arrpart2 partition of pph_arrpart for values with (modulus 2,
remainder 1);
+insert into pph_arrpart values ('{1}'), ('{1, 2}'), ('{4, 5}');
+select tableoid::regclass, * from pph_arrpart order by 1;
+ tableoid | a
+--------------+-------
+ pph_arrpart1 | {1,2}
+ pph_arrpart1 | {4,5}
+ pph_arrpart2 | {1}
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a = '{1}';
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on pph_arrpart2
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on pph_arrpart1
+ Filter: (a = '{1,2}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a in ('{4, 5}', '{1}');
+ QUERY PLAN
+----------------------------------------------------------------------
+ Append
+ -> Seq Scan on pph_arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+ -> Seq Scan on pph_arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(5 rows)
+
+drop table pph_arrpart;
+-- enum type list partition key
+create type pp_colors as enum ('green', 'blue', 'black');
+create table pp_enumpart (a pp_colors) partition by list (a);
+create table pp_enumpart_green partition of pp_enumpart for values in
('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+ QUERY PLAN
+-----------------------------------------
+ Append
+ -> Seq Scan on pp_enumpart_blue
+ Filter: (a = 'blue'::pp_colors)
+(3 rows)
+
+explain (costs off) select * from pp_enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_enumpart;
+drop type pp_colors;
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+ QUERY PLAN
+-------------------------------------------
+ Append
+ -> Seq Scan on pp_recpart_11
+ Filter: (a = '(1,1)'::pp_rectype)
+(3 rows)
+
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_recpart;
+drop type pp_rectype;
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in
('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in
('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on pp_intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql
b/src/test/regress/sql/partition_prune.sql
index e808d1a439..b38b39c71e 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -645,3 +645,56 @@ select * from boolp where a = (select value from
boolvalues where not value);
drop table boolp;
reset enable_indexonlyscan;
+
+--
+-- check that pruning works properly when the partition key is of a
+-- pseudotype
+--
+
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4,
5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+explain (costs off) select * from pp_arrpart where a = '{1, 2}';
+explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
+drop table pp_arrpart;
+
+-- array type hash partition key
+create table pph_arrpart (a int[]) partition by hash (a);
+create table pph_arrpart1 partition of pph_arrpart for values with (modulus 2,
remainder 0);
+create table pph_arrpart2 partition of pph_arrpart for values with (modulus 2,
remainder 1);
+insert into pph_arrpart values ('{1}'), ('{1, 2}'), ('{4, 5}');
+select tableoid::regclass, * from pph_arrpart order by 1;
+explain (costs off) select * from pph_arrpart where a = '{1}';
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+explain (costs off) select * from pph_arrpart where a in ('{4, 5}', '{1}');
+drop table pph_arrpart;
+
+-- enum type list partition key
+create type pp_colors as enum ('green', 'blue', 'black');
+create table pp_enumpart (a pp_colors) partition by list (a);
+create table pp_enumpart_green partition of pp_enumpart for values in
('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+explain (costs off) select * from pp_enumpart where a = 'black';
+drop table pp_enumpart;
+drop type pp_colors;
+
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+drop table pp_recpart;
+drop type pp_rectype;
+
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in
('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in
('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+drop table pp_intrangepart;
--
2.11.0
