Hi. I recently posted to the list about a couple of problems I saw when using array type column as the partition key. One of them was that the internal partition constraint expression that we generate for list partitions is of a form that the backend would reject if the partition key column is an array instead of a scalar. See for example:
create table p (a int[]) partition by list (a); create table p1 partition of p for values in ('{1}'); create table p2 partition of p for values in ('{2, 3}', '{4, 5}'); insert into p values ('{1}'); INSERT 0 1 insert into p values ('{2, 3}'), ('{4, 5}'); INSERT 0 2 \d+ p1 ... Partition of: p FOR VALUES IN ('{1}') Partition constraint: ((a IS NOT NULL) AND ((a)::anyarray OPERATOR(pg_catalog.=) ANY (ARRAY['{1}'::integer[]]))) \d+ p2 ... Partition of: p FOR VALUES IN ('{2,3}', '{4,5}') Partition constraint: ((a IS NOT NULL) AND ((a)::anyarray OPERATOR(pg_catalog.=) ANY (ARRAY['{2,3}'::integer[], '{4,5}'::integer[]]))) Try copy-pasting the p1's constraint into SQL: In a select query: select tableoid::regclass, (a)::anyarray OPERATOR(pg_catalog.=) ANY (ARRAY['{1}'::integer[]]) from p; ERROR: operator does not exist: integer[] pg_catalog.= integer LINE 1: select tableoid::regclass, (a)::anyarray OPERATOR(pg_catalog... ^ HINT: No operator matches the given name and argument types. You might need to add explicit type casts. Or use in a check constraint: alter table p1 add constraint check_a check ((a)::anyarray OPERATOR(pg_catalog.=) ANY (ARRAY['{1}'::integer[]])); ERROR: operator does not exist: integer[] pg_catalog.= integer HINT: No operator matches the given name and argument types. You might need to add explicit type casts. That's because, as Tom pointed out [1], ANY/ALL expect the LHS to be a scalar, whereas in this case a is an int[]. So, the partitioning code is internally generating an expression that would not get through the parser. I think it's better that we fix that. Attached patch is an attempt at that. With the patch, instead of internally generating an ANY/ALL expression, generate an OR expression instead. So: \d+ p1 ... Partition of: p FOR VALUES IN ('{1}') Partition constraint: ((a IS NOT NULL) AND ((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[])) \d+ p2 ... Partition of: p FOR VALUES IN ('{2,3}', '{4,5}') Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{2,3}'::integer[]) OR ((a)::anyarray OPERATOR(pg_catalog.=) '{4,5}'::integer[]))) The expressions above get through the parser just fine: select tableoid::regclass, (a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[] from p; tableoid | ?column? |---------+---------- p1 | t p2 | f p2 | f (3 rows) alter table p1 add constraint check_a check ((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]); ALTER TABLE \d+ p1 ... Check constraints: "check_a" CHECK (a = '{1}'::integer[]) Will add the patch to the next CF. Thanks, Amit [1] https://www.postgresql.org/message-id/7677.1512743642%40sss.pgh.pa.us
From 4afca04d021e6c33b51f7139b79fc33bcabbafc3 Mon Sep 17 00:00:00 2001 From: amit <amitlangot...@gmail.com> Date: Fri, 8 Dec 2017 19:00:46 +0900 Subject: [PATCH] Emit list partition constraint as OR expression ... instead of key op ANY (<array>) as is done now. If key is itself of an array type, the current representation leads to an expression that backend doesn't reliably accept. --- src/backend/catalog/partition.c | 55 +++++++++++++-------------- src/test/regress/expected/create_table.out | 6 +-- src/test/regress/expected/foreign_data.out | 6 +-- src/test/regress/expected/partition_prune.out | 12 +++--- 4 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index ef156e449e..92089cf781 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -1621,18 +1621,27 @@ make_partition_op_expr(PartitionKey key, int keynum, { case PARTITION_STRATEGY_LIST: { - ScalarArrayOpExpr *saopexpr; - - /* Build leftop = ANY (rightop) */ - saopexpr = makeNode(ScalarArrayOpExpr); - saopexpr->opno = operoid; - saopexpr->opfuncid = get_opcode(operoid); - saopexpr->useOr = true; - saopexpr->inputcollid = key->partcollation[keynum]; - saopexpr->args = list_make2(arg1, arg2); - saopexpr->location = -1; - - result = (Expr *) saopexpr; + ListCell *lc; + List *elems = (List *) arg2, + *elemops = NIL; + + foreach (lc, elems) + { + Expr *elem = lfirst(lc), + *elemop; + + elemop = make_opclause(operoid, + BOOLOID, + false, + arg1, elem, + InvalidOid, + key->partcollation[keynum]); + elemops = lappend(elemops, elemop); + } + + result = list_length(elemops) > 1 + ? makeBoolExpr(OR_EXPR, elemops, -1) + : linitial(elemops); break; } @@ -1754,11 +1763,10 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) PartitionKey key = RelationGetPartitionKey(parent); List *result; Expr *keyCol; - ArrayExpr *arr; Expr *opexpr; NullTest *nulltest; ListCell *cell; - List *arrelems = NIL; + List *elems = NIL; bool list_has_null = false; /* @@ -1824,7 +1832,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) false, /* isnull */ key->parttypbyval[0]); - arrelems = lappend(arrelems, val); + elems = lappend(elems, val); } } else @@ -1839,26 +1847,15 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) if (val->constisnull) list_has_null = true; else - arrelems = lappend(arrelems, copyObject(val)); + elems = lappend(elems, copyObject(val)); } } - if (arrelems) + if (elems) { - /* Construct an ArrayExpr for the non-null partition values */ - arr = makeNode(ArrayExpr); - arr->array_typeid = !type_is_array(key->parttypid[0]) - ? get_array_type(key->parttypid[0]) - : key->parttypid[0]; - arr->array_collid = key->parttypcoll[0]; - arr->element_typeid = key->parttypid[0]; - arr->elements = arrelems; - arr->multidims = false; - arr->location = -1; - /* Generate the main expression, i.e., keyCol = ANY (arr) */ opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber, - keyCol, (Expr *) arr); + keyCol, (Expr *) elems); } else { diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 8e745402ae..353c428068 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -737,7 +737,7 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); a | text | | | | extended | | b | integer | | not null | 1 | plain | | Partition of: parted FOR VALUES IN ('b') -Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['b'::text]))) +Partition constraint: ((a IS NOT NULL) AND (a = 'b'::text)) Check constraints: "check_a" CHECK (length(a) > 0) "part_b_b_check" CHECK (b >= 0) @@ -750,7 +750,7 @@ Check constraints: a | text | | | | extended | | b | integer | | not null | 0 | plain | | Partition of: parted FOR VALUES IN ('c') -Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['c'::text]))) +Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text)) Partition key: RANGE (b) Check constraints: "check_a" CHECK (length(a) > 0) @@ -764,7 +764,7 @@ Partitions: part_c_1_10 FOR VALUES FROM (1) TO (10) a | text | | | | extended | | b | integer | | not null | 0 | plain | | Partition of: part_c FOR VALUES FROM (1) TO (10) -Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['c'::text])) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10)) +Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10)) Check constraints: "check_a" CHECK (length(a) > 0) diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index d2c184f2cf..6a1b278e5a 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -1863,7 +1863,7 @@ Partitions: pt2_1 FOR VALUES IN (1) c2 | text | | | | | extended | | c3 | date | | | | | plain | | Partition of: pt2 FOR VALUES IN (1) -Partition constraint: ((c1 IS NOT NULL) AND (c1 = ANY (ARRAY[1]))) +Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1)) Server: s0 FDW options: (delimiter ',', quote '"', "be quoted" 'value') @@ -1935,7 +1935,7 @@ Partitions: pt2_1 FOR VALUES IN (1) c2 | text | | | | | extended | | c3 | date | | | | | plain | | Partition of: pt2 FOR VALUES IN (1) -Partition constraint: ((c1 IS NOT NULL) AND (c1 = ANY (ARRAY[1]))) +Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1)) Server: s0 FDW options: (delimiter ',', quote '"', "be quoted" 'value') @@ -1963,7 +1963,7 @@ Partitions: pt2_1 FOR VALUES IN (1) c2 | text | | | | | extended | | c3 | date | | not null | | | plain | | Partition of: pt2 FOR VALUES IN (1) -Partition constraint: ((c1 IS NOT NULL) AND (c1 = ANY (ARRAY[1]))) +Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1)) Check constraints: "p21chk" CHECK (c2 <> ''::text) Server: s0 diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index aabb0240a9..385123b3b0 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -1006,7 +1006,9 @@ explain (costs off) select * from boolpart where a in (true, false); Filter: (a = ANY ('{t,f}'::boolean[])) -> Seq Scan on boolpart_t Filter: (a = ANY ('{t,f}'::boolean[])) -(5 rows) + -> Seq Scan on boolpart_default + Filter: (a = ANY ('{t,f}'::boolean[])) +(7 rows) explain (costs off) select * from boolpart where a = false; QUERY PLAN @@ -1014,23 +1016,19 @@ explain (costs off) select * from boolpart where a = false; Append -> Seq Scan on boolpart_f Filter: (NOT a) - -> Seq Scan on boolpart_t - Filter: (NOT a) -> Seq Scan on boolpart_default Filter: (NOT a) -(7 rows) +(5 rows) explain (costs off) select * from boolpart where not a = false; QUERY PLAN ------------------------------------ Append - -> Seq Scan on boolpart_f - Filter: a -> Seq Scan on boolpart_t Filter: a -> Seq Scan on boolpart_default Filter: a -(7 rows) +(5 rows) explain (costs off) select * from boolpart where a is true or a is not true; QUERY PLAN -- 2.11.0