From 1ec0c064adc34c12ae5615f0f2bca27a9c61c30f Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 8 May 2018 14:31:37 +0900 Subject: [PATCH v4] Fix how get_partition_operator looks up the operator Instead of looking up using the underlying partition key's type as the operator's lefttype and righttype, use the partition operator class declared input type, which works reliably even in the cases where pseudo-types are involved. Also, make its decision whether a RelableType is needed depend on a different criteria than what's currently there. That is, only add a RelabelType if the partition key's type is different from the selected operator's input types. --- src/backend/catalog/partition.c | 47 ++++++-------- src/test/regress/expected/create_table.out | 2 +- src/test/regress/expected/inherit.out | 98 ++++++++++++++++++++++++++++++ src/test/regress/sql/inherit.sql | 43 +++++++++++++ 4 files changed, 161 insertions(+), 29 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 17704f36b9..1f50b29a3f 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -1176,40 +1176,31 @@ get_partition_operator(PartitionKey key, int col, StrategyNumber strategy, Oid operoid; /* - * First check if there exists an operator of the given strategy, with - * this column's type as both its lefttype and righttype, in the - * partitioning operator family specified for the column. + * Get the operator in the partitioning operator family using the operator + * class declared input type as both its lefttype and righttype. */ operoid = get_opfamily_member(key->partopfamily[col], - key->parttypid[col], - key->parttypid[col], + key->partopcintype[col], + key->partopcintype[col], strategy); + if (!OidIsValid(operoid)) + elog(ERROR, "missing operator %d(%u,%u) in partition opfamily %u", + strategy, key->partopcintype[col], key->partopcintype[col], + key->partopfamily[col]); /* - * If one doesn't exist, we must resort to using an operator in the same - * operator family but with the operator class declared input type. It is - * OK to do so, because the column's type is known to be binary-coercible - * with the operator class input type (otherwise, the operator class in - * question would not have been accepted as the partitioning operator - * class). We must however inform the caller to wrap the non-Const - * expression with a RelabelType node to denote the implicit coercion. It - * ensures that the resulting expression structurally matches similarly - * processed expressions within the optimizer. + * If the partition key column is not of the same type as what the + * operator expects, we'll need to tell the caller to wrap the non-Const + * expression with a RelableType node in most cases, because that's what + * the parser would do for similar expressions (such as WHERE clauses + * involving this column and operator.) The intention is to make the + * expression that we generate here structurally match with expression + * generated elsewhere. We don't need the RelableType node for certain + * types though, because parse_coerce.c won't emit one either. */ - if (!OidIsValid(operoid)) - { - operoid = get_opfamily_member(key->partopfamily[col], - key->partopcintype[col], - key->partopcintype[col], - strategy); - if (!OidIsValid(operoid)) - elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", - strategy, key->partopcintype[col], key->partopcintype[col], - key->partopfamily[col]); - *need_relabel = true; - } - else - *need_relabel = false; + *need_relabel = (key->parttypid[col] != key->partopcintype[col] && + key->partopcintype[col] != RECORDOID && + !IsPolymorphicType(key->partopcintype[col])); return operoid; } diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index ab1d4ecaee..0026aa11dd 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -808,7 +808,7 @@ 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; -- partition on boolean column diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 2efae8038f..26fdac0542 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1973,3 +1973,101 @@ select min(a), max(a) from parted_minmax where b = '12345'; (1 row) drop table parted_minmax; +-- +-- 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; +-- 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/inherit.sql b/src/test/regress/sql/inherit.sql index 55770f5681..d157055d2b 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -683,3 +683,46 @@ insert into parted_minmax values (1,'12345'); explain (costs off) select min(a), max(a) from parted_minmax where b = '12345'; select min(a), max(a) from parted_minmax where b = '12345'; drop table parted_minmax; + + +-- +-- 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; + +-- 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