Thanks for the comment.
On 2018/04/09 23:22, Tom Lane wrote:
> Amit Langote <langote_amit...@lab.ntt.co.jp> writes:
>> I noticed that the newly added pruning does not work if the partition key
>> is of one of the types that have a corresponding pseudo-type.
>
> While I don't recall the details due to acute caffeine shortage,
> there are specific coding patterns that are used in the planner
> (e.g. in indxpath.c) to ensure that the code works with pseudotype
> opclasses as well as simple ones. The existence of this bug
> indicates that those conventions were not followed in the pruning
> code. I wonder whether this patch makes the pruning code look
> more like the pre-existing code, or even less like it.
Ah, I think I got it after staring at the (btree) index code for a bit.
What pruning code got wrong is that it's comparing the expression type
(type of the constant arg that will be compared with partition bound
datums when pruning) with the partopcintype to determine if we should look
up the cross-type comparison/hashing procedure, whereas what the latter
should be compare with is the clause operator's oprighttype. ISTM, if
op_in_opfamily() passed for the operator, that's the correct thing to do.
Once I changed the code to do it that way, no special considerations are
needed to handle pseudo-type type partition key.
Attached please find the updated patch.
Thanks,
Amit
From 1c136a321153cca04c0842807c2cd166e79f2556 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 8 Dec 2017 19:09:31 +0900
Subject: [PATCH v2] 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 | 29 +++++---
src/test/regress/expected/partition_prune.out | 98 +++++++++++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 44 +++++++++++-
3 files changed, 161 insertions(+), 10 deletions(-)
diff --git a/src/backend/partitioning/partprune.c
b/src/backend/partitioning/partprune.c
index 7666c6c412..2655d2caa2 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;
@@ -1517,10 +1519,20 @@ 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])
+ /*
+ * Check if we're going to need a cross-type comparison
function to
+ * use during pruning.
+ */
+ get_op_opfamily_properties(OidIsValid(negator)
+ ?
negator : opclause->opno,
+
partopfamily, false,
+
&op_strategy, &op_lefttype, &op_righttype);
+ /* Use the cached one if no cross-type comparison. */
+ if (op_righttype == part_scheme->partopcintype[partkeyidx])
+ cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+ else
{
+ /* Otherwise, look the correct one up in the catalog. */
switch (part_scheme->strategy)
{
case PARTITION_STRATEGY_LIST:
@@ -1528,13 +1540,14 @@ match_clause_to_partition_key(RelOptInfo *rel,
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
part_scheme->partopcintype[partkeyidx],
-
exprtype, BTORDER_PROC);
+
op_righttype, BTORDER_PROC);
break;
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,8 +1560,6 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (!OidIsValid(cmpfn))
return PARTCLAUSE_UNSUPPORTED;
}
- else
- cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
partclause->keyno = partkeyidx;
diff --git a/src/test/regress/expected/partition_prune.out
b/src/test/regress/expected/partition_prune.out
index df3fca025e..8874c192ee 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2399,3 +2399,101 @@ 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 array,
+-- enum, record, or range type
+--
+-- array type list partition key
+create table arrpart (a int[]) partition by list (a);
+create table arrpart1 partition of arrpart for values in ('{1}');
+create table arrpart2 partition of arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from arrpart where a = '{1}';
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on arrpart1
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from arrpart where a = '{1, 2}';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from arrpart where a in ('{4, 5}', '{1}');
+ QUERY PLAN
+----------------------------------------------------------------------
+ Append
+ -> Seq Scan on arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+ -> Seq Scan on arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(5 rows)
+
+drop table arrpart;
+-- enum type list partition key
+create type colors as enum ('green', 'blue', 'black');
+create table enumpart (a colors) partition by list (a);
+create table enumpart_green partition of enumpart for values in ('green');
+create table enumpart_blue partition of enumpart for values in ('blue');
+explain (costs off) select * from enumpart where a = 'blue';
+ QUERY PLAN
+--------------------------------------
+ Append
+ -> Seq Scan on enumpart_blue
+ Filter: (a = 'blue'::colors)
+(3 rows)
+
+explain (costs off) select * from enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table enumpart;
+drop type colors;
+-- record type as partition key
+create type rectype as (a int, b int);
+create table recpart (a rectype) partition by list (a);
+create table recpart_11 partition of recpart for values in ('(1,1)');
+create table recpart_23 partition of recpart for values in ('(2,3)');
+explain (costs off) select * from recpart where a = '(1,1)'::rectype;
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on recpart_11
+ Filter: (a = '(1,1)'::rectype)
+(3 rows)
+
+explain (costs off) select * from recpart where a = '(1,2)'::rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table recpart;
+drop type rectype;
+-- range type partition key
+create table intrangepart (a int4range) partition by list (a);
+create table intrangepart12 partition of intrangepart for values in ('[1,2]');
+create table intrangepart2inf partition of intrangepart for values in ('[2,)');
+explain (costs off) select * from intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql
b/src/test/regress/sql/partition_prune.sql
index 7fe93bbc04..7e4a69e5a1 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -587,4 +587,46 @@ select * from boolp where a = (select value from
boolvalues where not value);
drop table boolp;
-reset enable_indexonlyscan;
\ No newline at end of file
+reset enable_indexonlyscan;
+
+--
+-- check that pruning works properly when the partition key is of a array,
+-- enum, record, or range type
+--
+
+-- array type list partition key
+create table arrpart (a int[]) partition by list (a);
+create table arrpart1 partition of arrpart for values in ('{1}');
+create table arrpart2 partition of arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from arrpart where a = '{1}';
+explain (costs off) select * from arrpart where a = '{1, 2}';
+explain (costs off) select * from arrpart where a in ('{4, 5}', '{1}');
+drop table arrpart;
+
+-- enum type list partition key
+create type colors as enum ('green', 'blue', 'black');
+create table enumpart (a colors) partition by list (a);
+create table enumpart_green partition of enumpart for values in ('green');
+create table enumpart_blue partition of enumpart for values in ('blue');
+explain (costs off) select * from enumpart where a = 'blue';
+explain (costs off) select * from enumpart where a = 'black';
+drop table enumpart;
+drop type colors;
+
+-- record type as partition key
+create type rectype as (a int, b int);
+create table recpart (a rectype) partition by list (a);
+create table recpart_11 partition of recpart for values in ('(1,1)');
+create table recpart_23 partition of recpart for values in ('(2,3)');
+explain (costs off) select * from recpart where a = '(1,1)'::rectype;
+explain (costs off) select * from recpart where a = '(1,2)'::rectype;
+drop table recpart;
+drop type rectype;
+
+-- range type partition key
+create table intrangepart (a int4range) partition by list (a);
+create table intrangepart12 partition of intrangepart for values in ('[1,2]');
+create table intrangepart2inf partition of intrangepart for values in ('[2,)');
+explain (costs off) select * from intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from intrangepart where a = '(1,2)'::int4range;
+drop table intrangepart;
--
2.11.0
