From 340b0b3a97af0fa07562bc4434a4908402c3efbd Mon Sep 17 00:00:00 2001 From: Alvaro Herrera 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