Fix assorted partition pruning bugs

match_clause_to_partition_key failed to consider COERCION_PATH_ARRAYCOERCE
cases in scalar-op-array expressions, so it was possible to crash the
server easily.  To handle this case properly (ie. prune partitions) we
would need to run a bit of executor code during planning.  Maybe it can
be improved, but for now let's just not crash.  Add a test case that
used to trigger the crash.
Author: Michaël Paquier

match_clause_to_partition_key failed to indicate that operators that
don't have a commutator in a btree opclass are unsupported.  It is
possible for this to cause a crash later if such an operator is used in
a scalar-op-array expression.  Add a test case that used to the crash.
Author: Amit Langote

One caller of gen_partprune_steps_internal in
match_clause_to_partition_key was too optimistic about the former never
returning an empty step list.  Rid it of its innocence.  (Having fixed
the bug above, I no longer know how to exploit this, so no test case for
it, but it remained a bug.)  Revise code flow a little bit, for
succintness.
Author: Álvaro Herrera

Reported-by: Marina Polyakova
Reviewed-by: Michaël Paquier
Reviewed-by: Amit Langote
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/ff8f9bfa485ff961d6bb43e54120485b@postgrespro.ru
This commit is contained in:
Alvaro Herrera 2018-05-09 10:51:23 -03:00
parent 35361ee788
commit d758d9702e
3 changed files with 105 additions and 31 deletions

View File

@ -573,8 +573,9 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps)
* For BoolExpr clauses, we recursively generate steps for each argument, and
* return a PartitionPruneStepCombine of their results.
*
* The generated steps are added to the context's steps list. Each step is
* assigned a step identifier, unique even across recursive calls.
* The return value is a list of the steps generated, which are also added to
* the context's steps list. Each step is assigned a step identifier, unique
* even across recursive calls.
*
* If we find clauses that are mutually contradictory, or a pseudoconstant
* clause that contains false, we set *contradictory to true and return NIL
@ -1601,6 +1602,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
List *elem_exprs,
*elem_clauses;
ListCell *lc1;
bool contradictory;
if (IsA(leftop, RelabelType))
leftop = ((RelabelType *) leftop)->arg;
@ -1619,7 +1621,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
* Only allow strict operators. This will guarantee nulls are
* filtered.
*/
if (!op_strict(saop->opno))
if (!op_strict(saop_op))
return PARTCLAUSE_UNSUPPORTED;
/* Useless if the array has any volatile functions. */
@ -1652,6 +1654,8 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (strategy != BTEqualStrategyNumber)
return PARTCLAUSE_UNSUPPORTED;
}
else
return PARTCLAUSE_UNSUPPORTED; /* no useful negator */
}
/*
@ -1692,7 +1696,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
elem_exprs = lappend(elem_exprs, elem_expr);
}
}
else
else if (IsA(rightop, ArrayExpr))
{
ArrayExpr *arrexpr = castNode(ArrayExpr, rightop);
@ -1706,6 +1710,11 @@ match_clause_to_partition_key(RelOptInfo *rel,
elem_exprs = arrexpr->elements;
}
else
{
/* Give up on any other clause types. */
return PARTCLAUSE_UNSUPPORTED;
}
/*
* Now generate a list of clauses, one for each array element, of the
@ -1724,36 +1733,21 @@ match_clause_to_partition_key(RelOptInfo *rel,
}
/*
* Build a combine step as if for an OR clause or add the clauses to
* the end of the list that's being processed currently.
* If we have an ANY clause and multiple elements, first turn the list
* of clauses into an OR expression.
*/
if (saop->useOr && list_length(elem_clauses) > 1)
{
Expr *orexpr;
bool contradictory;
elem_clauses = list_make1(makeBoolExpr(OR_EXPR, elem_clauses, -1));
orexpr = makeBoolExpr(OR_EXPR, elem_clauses, -1);
*clause_steps =
gen_partprune_steps_internal(context, rel, list_make1(orexpr),
&contradictory);
if (contradictory)
return PARTCLAUSE_MATCH_CONTRADICT;
Assert(list_length(*clause_steps) == 1);
return PARTCLAUSE_MATCH_STEPS;
}
else
{
bool contradictory;
*clause_steps =
gen_partprune_steps_internal(context, rel, elem_clauses,
&contradictory);
if (contradictory)
return PARTCLAUSE_MATCH_CONTRADICT;
Assert(list_length(*clause_steps) >= 1);
return PARTCLAUSE_MATCH_STEPS;
}
/* Finally, generate steps */
*clause_steps =
gen_partprune_steps_internal(context, rel, elem_clauses,
&contradictory);
if (contradictory)
return PARTCLAUSE_MATCH_CONTRADICT;
else if (*clause_steps == NIL)
return PARTCLAUSE_UNSUPPORTED; /* step generation failed */
return PARTCLAUSE_MATCH_STEPS;
}
else if (IsA(clause, NullTest))
{

View File

@ -1073,6 +1073,72 @@ explain (costs off) select * from boolpart where a is not unknown;
Filter: (a IS NOT UNKNOWN)
(7 rows)
-- test scalar-to-array operators
create table coercepart (a varchar) partition by list (a);
create table coercepart_ab partition of coercepart for values in ('ab');
create table coercepart_bc partition of coercepart for values in ('bc');
create table coercepart_cd partition of coercepart for values in ('cd');
explain (costs off) select * from coercepart where a in ('ab', to_char(125, '999'));
QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------
Append
-> Seq Scan on coercepart_ab
Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
-> Seq Scan on coercepart_bc
Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
-> Seq Scan on coercepart_cd
Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
(7 rows)
explain (costs off) select * from coercepart where a ~ any ('{ab}');
QUERY PLAN
----------------------------------------------------
Append
-> Seq Scan on coercepart_ab
Filter: ((a)::text ~ ANY ('{ab}'::text[]))
-> Seq Scan on coercepart_bc
Filter: ((a)::text ~ ANY ('{ab}'::text[]))
-> Seq Scan on coercepart_cd
Filter: ((a)::text ~ ANY ('{ab}'::text[]))
(7 rows)
explain (costs off) select * from coercepart where a !~ all ('{ab}');
QUERY PLAN
-----------------------------------------------------
Append
-> Seq Scan on coercepart_ab
Filter: ((a)::text !~ ALL ('{ab}'::text[]))
-> Seq Scan on coercepart_bc
Filter: ((a)::text !~ ALL ('{ab}'::text[]))
-> Seq Scan on coercepart_cd
Filter: ((a)::text !~ ALL ('{ab}'::text[]))
(7 rows)
explain (costs off) select * from coercepart where a ~ any ('{ab,bc}');
QUERY PLAN
-------------------------------------------------------
Append
-> Seq Scan on coercepart_ab
Filter: ((a)::text ~ ANY ('{ab,bc}'::text[]))
-> Seq Scan on coercepart_bc
Filter: ((a)::text ~ ANY ('{ab,bc}'::text[]))
-> Seq Scan on coercepart_cd
Filter: ((a)::text ~ ANY ('{ab,bc}'::text[]))
(7 rows)
explain (costs off) select * from coercepart where a !~ all ('{ab,bc}');
QUERY PLAN
--------------------------------------------------------
Append
-> Seq Scan on coercepart_ab
Filter: ((a)::text !~ ALL ('{ab,bc}'::text[]))
-> Seq Scan on coercepart_bc
Filter: ((a)::text !~ ALL ('{ab,bc}'::text[]))
-> Seq Scan on coercepart_cd
Filter: ((a)::text !~ ALL ('{ab,bc}'::text[]))
(7 rows)
drop table coercepart;
--
-- some more cases
--

View File

@ -152,6 +152,20 @@ 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;
-- test scalar-to-array operators
create table coercepart (a varchar) partition by list (a);
create table coercepart_ab partition of coercepart for values in ('ab');
create table coercepart_bc partition of coercepart for values in ('bc');
create table coercepart_cd partition of coercepart for values in ('cd');
explain (costs off) select * from coercepart where a in ('ab', to_char(125, '999'));
explain (costs off) select * from coercepart where a ~ any ('{ab}');
explain (costs off) select * from coercepart where a !~ all ('{ab}');
explain (costs off) select * from coercepart where a ~ any ('{ab,bc}');
explain (costs off) select * from coercepart where a !~ all ('{ab,bc}');
drop table coercepart;
--
-- some more cases
--