On 2018/02/02 0:20, Robert Haas wrote:
> On Thu, Feb 1, 2018 at 4:42 AM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>>> I hope someone else chimes in as well. :)
>>
>> Bug #15042 [1] seems to be caused by this same problem. There, a
>> RelabelType node is being slapped (by the partitioning code) on a Var node
>> of a partition key on enum.
>>
>> Attached updated patch.
>
> Can I get anyone else to weigh in on whether this is likely to be
> safe? Paging people who understand constraint exclusion...
Added this to CF (actually moved to the September one after first having
added it to the CF that is just getting started).
It seems to me that we don't need to go with my originally proposed
approach to teach predtest.c to strip RelabelType nodes. Apparently, it's
only partition.c that's adding the RelableType node around partition key
nodes in such cases. That is, in the case of having an array, record,
enum, and range type as the partition key. No other part of the system
ends up adding one as far as I can see. Parser's make_op(), for example,
that is used to generate an OpExpr for a qual involving the partition key,
won't put RelabelType around the partition key node if the operator in
question has "pseudo"-types listed as declared types of its left and right
arguments.
So I revised the patch to drop all the predtest.c changes and instead
modify get_partition_operator() to avoid generating RelabelType in such
cases. Please find it attached.
Thanks,
Amit
From b0ebef9526a2abb162e877bc3b73d8194ffb1d2f Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 8 Dec 2017 19:09:31 +0900
Subject: [PATCH v3] In get_partition_operator(), avoid RelabelType in some
cases
We never have pg_am catalog entries with "real" array, enum, record,
range types as leftop and rightop types. Instead, operators manipulating
such types have entries with "pseudo-types" listed as leftop and rightop
types. For example, for enums, all operator entries are marked with
anyenum as their leftop and rightop types. So, if we pass "real" type
OIDs to get_opfamily_member(), we get back an InvalidOid for the
aforementioned type categories. We'd normally ask the caller of
get_partition_operator to add a RelabelType in such case. But we don't
need to in these cases, as the rest of the system doesn't.
This fixes the issue that the constraint exclusion code would reject
matching partition key quals with restrictions due to these extraneous
RelabelType nodes surrounding partition key expressions generated
by partition.c.
Also, ruleutils.c can resolve the operator names properly when deparsing
an internally generated partition qual expressions.
---
src/backend/catalog/partition.c | 12 ++++++-
src/test/regress/expected/create_table.out | 2 +-
src/test/regress/expected/partition_prune.out | 45 +++++++++++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 18 +++++++++++
4 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index fcf7655553..cf71ba174e 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1605,7 +1605,17 @@ get_partition_operator(PartitionKey key, int col,
StrategyNumber strategy,
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
strategy, key->partopcintype[col],
key->partopcintype[col],
key->partopfamily[col]);
- *need_relabel = true;
+
+ /*
+ * For certain type categories, there don't exist pg_amop
entries with
+ * the given type OID as the operator's left and right operand
type.
+ * Instead, the entries have corresponding pseudo-types listed
as the
+ * left and right operand type; for example, anyenum for an
enum type.
+ * For such cases, it's not necessary to add the RelabelType
node,
+ * because other parts of the sytem won't add one either.
+ */
+ if (get_typtype(key->partopcintype[col]) != TYPTYPE_PSEUDO)
+ *need_relabel = true;
}
else
*need_relabel = false;
diff --git a/src/test/regress/expected/create_table.out
b/src/test/regress/expected/create_table.out
index 39a963888d..7764da3152 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -880,6 +880,6 @@ CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN
('{1}', '{2}');
--------+-----------+-----------+----------+---------+----------+--------------+-------------
a | integer[] | | | | extended |
|
Partition of: arrlp FOR VALUES IN ('{1}', '{2}')
-Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray
OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray
OPERATOR(pg_catalog.=) '{2}'::integer[])))
+Partition constraint: ((a IS NOT NULL) AND ((a = '{1}'::integer[]) OR (a =
'{2}'::integer[])))
DROP TABLE arrlp;
diff --git a/src/test/regress/expected/partition_prune.out
b/src/test/regress/expected/partition_prune.out
index 348719bd62..cf25c8ec86 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1088,4 +1088,49 @@ explain (costs off) select * from boolpart where a is
not unknown;
Filter: (a IS NOT UNKNOWN)
(7 rows)
+-- 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');
+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)
+
+drop table enumpart;
+drop type colors;
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart;
diff --git a/src/test/regress/sql/partition_prune.sql
b/src/test/regress/sql/partition_prune.sql
index 514f8e5ce1..abe1b56c80 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -152,4 +152,22 @@ explain (costs off) select * from boolpart where a is not
true and a is not fals
explain (costs off) select * from boolpart where a is unknown;
explain (costs off) select * from boolpart where a is not unknown;
+-- 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');
+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';
+drop table enumpart;
+drop type colors;
+
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart;
--
2.11.0
