From d6652eb932a5e1cf6b63fa1dd09e4dd752267dbc Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 18 Apr 2018 10:40:14 +0900 Subject: [PATCH v4] 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 | 45 +++++++-- src/test/regress/expected/partition_prune.out | 138 ++++++++++++++++++++++++++ src/test/regress/sql/partition_prune.sql | 53 ++++++++++ 3 files changed, 227 insertions(+), 9 deletions(-) diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index 7666c6c412..6ab1066dea 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,24 +1519,51 @@ 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 can perform pruning if the clause's other argument is + * not of the same type as the partitioning opclass's declared input + * type. If it *is* of the same type, we can simply use the cached + * procedure, that is, the one in PartitionKey. If not, we must find + * the correct procedure in pg_amproc, which if it doesn't exist, we + * have to give up on pruning using this clause. + */ + get_op_opfamily_properties(OidIsValid(negator) ? + negator : opclause->opno, + partopfamily, false, + &op_strategy, &op_lefttype, &op_righttype); + + if (op_righttype == part_scheme->partopcintype[partkeyidx]) + cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid; + else { switch (part_scheme->strategy) { + /* + * RANGE and LIST partitioning use a btree operator family, + * wherein, we can expect to find a procedure in pg_amproc + * whose righttype matches the clause operator's righttype. + */ 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; + /* + * HASH partitioning uses a hash operator family, wherein the + * support function only ever processes *one* input value that + * is to be hashed. So, it doesn't make sense to look up + * using two distinct type OIDs. Instead, pass the desired + * type's OID as both lefttype and righttype when searching in + * pg_amproc. + */ 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 +1576,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 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