On Mon, May 07, 2018 at 10:37:10AM +0900, Michael Paquier wrote: > On Fri, May 04, 2018 at 12:32:23PM +0300, Marina Polyakova wrote: > > I got a similar server crash as in [1] on the master branch since the commit > > 9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails because > > the second argument ScalarArrayOpExpr is not a Const or an ArrayExpr, but is > > an ArrayCoerceExpr (see [2]): > > Indeed, I can see the crash. I have been playing with this stuff and I > am in the middle of writing the patch, but let's track this properly for > now.
So the problem appears when an expression needs to use COERCION_PATH_ARRAYCOERCE for a type coercion from one type to another, which requires an execution state to be able to build the list of elements. The clause matching happens at planning state, so while there are surely cases where this could be improved depending on the expression type, I propose to just discard all clauses which do not match OpExpr and ArrayExpr for now, as per the attached. It would be definitely a good practice to have a default code path returning PARTCLAUSE_UNSUPPORTED where the element list cannot be built. Thoughts? -- Michael
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index f954b92a6b..2d2f88e880 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1690,7 +1690,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);
@@ -1704,6 +1704,16 @@ match_clause_to_partition_key(RelOptInfo *rel,
elem_exprs = arrexpr->elements;
}
+ else
+ {
+ /*
+ * Ignore all other clause types. It could be possible here
+ * to reach this code path with a type coercion from an
+ * array type to another with ArrayCoerceExpr which depends on
+ * per-element execution for the conversion.
+ */
+ return PARTCLAUSE_UNSUPPORTED;
+ }
/*
* Now generate a list of clauses, one for each array element, of the
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index e0cc5f3393..86dcd62d55 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1073,6 +1073,48 @@ explain (costs off) select * from boolpart where a is not unknown;
Filter: (a IS NOT UNKNOWN)
(7 rows)
+-- type coercion from one array type to another, no pruning
+create table coercepart (a varchar) partition by list (a);
+create table coercepart_ab partition of coercepart for values in ('ab');
+create table coercepart_cd partition of coercepart for values in ('cd');
+create table coercepart_ef_gh partition of coercepart for values in ('ef', 'gh');
+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_cd
+ Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+ -> Seq Scan on coercepart_ef_gh
+ 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 in ('ab', NULL, to_char(125, '999'));
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------------------------------------------------------
+ Append
+ -> Seq Scan on coercepart_ab
+ Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, NULL::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+ -> Seq Scan on coercepart_cd
+ Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, NULL::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+ -> Seq Scan on coercepart_ef_gh
+ Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, NULL::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+(7 rows)
+
+explain (costs off) select * from coercepart where a in ('ef', 'gh', to_char(125, '999'));
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------------------------------------------------------
+ Append
+ -> Seq Scan on coercepart_ab
+ Filter: ((a)::text = ANY ((ARRAY['ef'::character varying, 'gh'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+ -> Seq Scan on coercepart_cd
+ Filter: ((a)::text = ANY ((ARRAY['ef'::character varying, 'gh'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+ -> Seq Scan on coercepart_ef_gh
+ Filter: ((a)::text = ANY ((ARRAY['ef'::character varying, 'gh'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+(7 rows)
+
+drop table coercepart;
--
-- some more cases
--
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 6b7f57ab41..267b7a3545 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -152,6 +152,16 @@ 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;
+-- type coercion from one array type to another, no pruning
+create table coercepart (a varchar) partition by list (a);
+create table coercepart_ab partition of coercepart for values in ('ab');
+create table coercepart_cd partition of coercepart for values in ('cd');
+create table coercepart_ef_gh partition of coercepart for values in ('ef', 'gh');
+explain (costs off) select * from coercepart where a in ('ab', to_char(125, '999'));
+explain (costs off) select * from coercepart where a in ('ab', NULL, to_char(125, '999'));
+explain (costs off) select * from coercepart where a in ('ef', 'gh', to_char(125, '999'));
+drop table coercepart;
+
--
-- some more cases
--
signature.asc
Description: PGP signature
