list partition constraint shape
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]/messages/by-id/7677.1512743642@sss.pgh.pa.us, 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
Attachments:
0001-Emit-list-partition-constraint-as-OR-expression.patchtext/plain; charset=UTF-8; name=0001-Emit-list-partition-constraint-as-OR-expression.patchDownload
From 4afca04d021e6c33b51f7139b79fc33bcabbafc3 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@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
(2017/12/13 16:38), Amit Langote wrote:
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.
+1
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[])
Thanks for the patch! Here are review comments that I have for now:
* I think it's a good idea to generate an OR expression tree for the
case where the type of the partitioning key is an array, but I'm not
sure we should handle other cases the same way because partition
constraints represented by OR-expression trees would not be efficiently
processed by the executor, compared to ScalarArrayOpExpr, when the
number of elements that are ORed together is large. So what about
generating the OR expression tree only if the partitioning-key's type is
an array, instead? That would be more consistent with the handling of
IN-list check constraints in eg, CREATE/ALTER TABLE, which I think is a
good thing.
* I think it'd be better to add a test case where multiple elements are
ORed together as a partition constraint.
Best regards,
Etsuro Fujita
Fujita-san,
Thanks for the review.
On 2018/01/23 20:13, Etsuro Fujita wrote:
(2017/12/13 16:38), Amit Langote wrote:
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[])Thanks for the patch! Here are review comments that I have for now:
* I think it's a good idea to generate an OR expression tree for the case
where the type of the partitioning key is an array, but I'm not sure we
should handle other cases the same way because partition constraints
represented by OR-expression trees would not be efficiently processed by
the executor, compared to ScalarArrayOpExpr, when the number of elements
that are ORed together is large. So what about generating the OR
expression tree only if the partitioning-key's type is an array, instead?
That would be more consistent with the handling of IN-list check
constraints in eg, CREATE/ALTER TABLE, which I think is a good thing.
Agreed, done that way.
So now, make_partition_op_expr() will generate an OR tree if the key is of
an array type, a ScalarArrayOpExpr if the IN-list contains more than one
value, and an OpExpr if the list contains just one value.
* I think it'd be better to add a test case where multiple elements are
ORed together as a partition constraint.
OK, added a test in create_table.sql.
Updated patch attached.
Thanks,
Amit
Attachments:
v2-0001-Emit-list-partition-constraint-as-OR-expression-i.patchtext/plain; charset=UTF-8; name=v2-0001-Emit-list-partition-constraint-as-OR-expression-i.patchDownload
From a83ebfe05d20e6086a9efacba0bcfbbdb7a2e570 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 8 Dec 2017 19:00:46 +0900
Subject: [PATCH v2] Emit list partition constraint as OR expression in some
cases
... instead of key op ANY (<array>) as is done now. If key is
itself of an array type, the expression in its current form
is not accepted by the backend's parser.
---
src/backend/catalog/partition.c | 96 +++++++++++++++++++--------
src/test/regress/expected/create_table.out | 18 ++++-
src/test/regress/expected/foreign_data.out | 6 +-
src/test/regress/expected/partition_prune.out | 8 +--
src/test/regress/sql/create_table.sql | 6 ++
5 files changed, 93 insertions(+), 41 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8adc4ee977..58527357a9 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1625,18 +1625,68 @@ 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;
+
+ if (type_is_array(key->parttypid[keynum]))
+ {
+ 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);
+ }
+ else if (list_length(elems) > 1)
+ {
+ ArrayExpr *arrexpr;
+ ScalarArrayOpExpr *saopexpr;
+
+ /*
+ * Construct an ArrayExpr for the non-null partition
+ * values
+ */
+ arrexpr = makeNode(ArrayExpr);
+ arrexpr->array_typeid =
+ !type_is_array(key->parttypid[0])
+ ? get_array_type(key->parttypid[0])
+ : key->parttypid[0];
+ arrexpr->array_collid = key->parttypcoll[0];
+ arrexpr->element_typeid = key->parttypid[0];
+ arrexpr->elements = elems;
+ arrexpr->multidims = false;
+ arrexpr->location = -1;
+
+ /* 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, arrexpr);
+ saopexpr->location = -1;
+
+ result = (Expr *) saopexpr;
+ }
+ else
+ result = make_opclause(operoid,
+ BOOLOID,
+ false,
+ arg1, linitial(elems),
+ InvalidOid,
+ key->partcollation[keynum]);
break;
}
@@ -1758,11 +1808,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;
/*
@@ -1828,7 +1877,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
false, /* isnull */
key->parttypbyval[0]);
- arrelems = lappend(arrelems, val);
+ elems = lappend(elems, val);
}
}
else
@@ -1843,26 +1892,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..14bcf7eaf3 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)
@@ -863,3 +863,15 @@ Partition key: LIST (a)
Number of partitions: 0
DROP TABLE parted_col_comment;
+-- list partitioning on array type column
+CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+\d+ arrlp12
+ Table "public.arrlp12"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+-----------+-----------+----------+---------+----------+--------------+-------------
+ a | integer[] | | | | extended | |
+Partition of: arrlp FOR VALUES IN ('{1}', '{2}')
+Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray OPERATOR(pg_catalog.=) '{2}'::integer[])))
+
+DROP TABLE arrlp;
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..348719bd62 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1014,23 +1014,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
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 8f9991ef18..f04d6c6114 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -707,3 +707,9 @@ COMMENT ON COLUMN parted_col_comment.a IS 'Partition key';
SELECT obj_description('parted_col_comment'::regclass);
\d+ parted_col_comment
DROP TABLE parted_col_comment;
+
+-- list partitioning on array type column
+CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+\d+ arrlp12
+DROP TABLE arrlp;
--
2.11.0
(2018/01/25 18:44), Amit Langote wrote:
On 2018/01/23 20:13, Etsuro Fujita wrote:
Here are review comments that I have for now:
* I think it's a good idea to generate an OR expression tree for the case
where the type of the partitioning key is an array, but I'm not sure we
should handle other cases the same way because partition constraints
represented by OR-expression trees would not be efficiently processed by
the executor, compared to ScalarArrayOpExpr, when the number of elements
that are ORed together is large. So what about generating the OR
expression tree only if the partitioning-key's type is an array, instead?
That would be more consistent with the handling of IN-list check
constraints in eg, CREATE/ALTER TABLE, which I think is a good thing.Agreed, done that way.
So now, make_partition_op_expr() will generate an OR tree if the key is of
an array type, a ScalarArrayOpExpr if the IN-list contains more than one
value, and an OpExpr if the list contains just one value.
Seems like a good idea.
* I think it'd be better to add a test case where multiple elements are
ORed together as a partition constraint.OK, added a test in create_table.sql.
Thanks.
Updated patch attached.
Thanks for the updated patch! Some minor comments:
+ /*
+ * Construct an ArrayExpr for the non-null partition
+ * values
+ */
+ arrexpr = makeNode(ArrayExpr);
+ arrexpr->array_typeid =
+ !type_is_array(key->parttypid[0])
+ ? get_array_type(key->parttypid[0])
+ : key->parttypid[0];We test the type_is_array() above in this bit, so I don't think we need
to test that again here.
+ arrexpr->array_collid = key->parttypcoll[0];
+ arrexpr->element_typeid = key->parttypid[0];We can assume that keynum=0 here, so it would be okay to specify zero as
the offset. But ISTM that that makes code a bit less readable, so I'd
vote for using keynum as the offset and maybe adding an assertion that
keynum should be zero, somewhere in the PARTITION_STRATEGY_LIST block.
* Both comments in the following in get_qual_for_list needs to be
updated, because the expression made there isn't necessarily = ANY anymore.
if (elems)
{
/* Generate the main expression, i.e., keyCol = ANY (arr) */
opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
keyCol, (Expr *) elems);
}
else
{
/* If there are no partition values, we don't need an = ANY expr */
opexpr = NULL;
}
Other than that, the patch looks good to me.
Best regards,
Etsuro Fujita
Fujita-san,
Thanks for the review.
On 2018/01/25 21:17, Etsuro Fujita wrote:
Thanks for the updated patch! Some minor comments:
+ /* + * Construct an ArrayExpr for the non-null partition + * values + */ + arrexpr = makeNode(ArrayExpr); + arrexpr->array_typeid = + !type_is_array(key->parttypid[0]) + ? get_array_type(key->parttypid[0]) + : key->parttypid[0];We test the type_is_array() above in this bit, so I don't think we need to
test that again here.
Ah, you're right. Fixed.
+ arrexpr->array_collid = key->parttypcoll[0]; + arrexpr->element_typeid = key->parttypid[0];We can assume that keynum=0 here, so it would be okay to specify zero as
the offset. But ISTM that that makes code a bit less readable, so I'd
vote for using keynum as the offset and maybe adding an assertion that
keynum should be zero, somewhere in the PARTITION_STRATEGY_LIST block.
Agreed, done.
* Both comments in the following in get_qual_for_list needs to be updated,
because the expression made there isn't necessarily = ANY anymore.if (elems)
{
/* Generate the main expression, i.e., keyCol = ANY (arr) */
opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
keyCol, (Expr *) elems);
}
else
{
/* If there are no partition values, we don't need an = ANY expr */
opexpr = NULL;
}
Fixed those.
Attached updated patch. Thanks again.
Regards,
Amit
Attachments:
v3-0001-Change-how-list-partition-constraint-is-emitted.patchtext/plain; charset=UTF-8; name=v3-0001-Change-how-list-partition-constraint-is-emitted.patchDownload
From 358b7c0fbbc646939b43b94405aeca96e56ccb9b Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 8 Dec 2017 19:00:46 +0900
Subject: [PATCH v3] Change how list partition constraint is emitted
In its current form (key = ANY (<array-of-values>)), a list partition
constraint expression is not always accepted by the backend's parser.
---
src/backend/catalog/partition.c | 102 ++++++++++++++++++--------
src/test/regress/expected/create_table.out | 18 ++++-
src/test/regress/expected/foreign_data.out | 6 +-
src/test/regress/expected/partition_prune.out | 8 +-
src/test/regress/sql/create_table.sql | 6 ++
5 files changed, 97 insertions(+), 43 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8adc4ee977..c82a52fc1b 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1625,18 +1625,67 @@ 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;
+
+ Assert(keynum == 0);
+ if (type_is_array(key->parttypid[keynum]))
+ {
+ 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);
+ }
+ else if (list_length(elems) > 1)
+ {
+ ArrayExpr *arrexpr;
+ ScalarArrayOpExpr *saopexpr;
+
+ /*
+ * Construct an ArrayExpr for the non-null partition
+ * values
+ */
+ arrexpr = makeNode(ArrayExpr);
+ arrexpr->array_typeid =
+ get_array_type(key->parttypid[keynum]);
+ arrexpr->array_collid = key->parttypcoll[keynum];
+ arrexpr->element_typeid = key->parttypid[keynum];
+ arrexpr->elements = elems;
+ arrexpr->multidims = false;
+ arrexpr->location = -1;
+
+ /* 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, arrexpr);
+ saopexpr->location = -1;
+
+ result = (Expr *) saopexpr;
+ }
+ else
+ result = make_opclause(operoid,
+ BOOLOID,
+ false,
+ arg1, linitial(elems),
+ InvalidOid,
+ key->partcollation[keynum]);
break;
}
@@ -1758,11 +1807,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;
/*
@@ -1828,7 +1876,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
false, /* isnull */
key->parttypbyval[0]);
- arrelems = lappend(arrelems, val);
+ elems = lappend(elems, val);
}
}
else
@@ -1843,30 +1891,22 @@ 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) */
+ /* Generate the operator expression. */
opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
- keyCol, (Expr *) arr);
+ keyCol, (Expr *) elems);
}
else
{
- /* If there are no partition values, we don't need an = ANY expr */
+ /*
+ * If there are no partition values, we don't need an operator
+ * expression.
+ */
opexpr = NULL;
}
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 8e745402ae..14bcf7eaf3 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)
@@ -863,3 +863,15 @@ Partition key: LIST (a)
Number of partitions: 0
DROP TABLE parted_col_comment;
+-- list partitioning on array type column
+CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+\d+ arrlp12
+ Table "public.arrlp12"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+-----------+-----------+----------+---------+----------+--------------+-------------
+ a | integer[] | | | | extended | |
+Partition of: arrlp FOR VALUES IN ('{1}', '{2}')
+Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray OPERATOR(pg_catalog.=) '{2}'::integer[])))
+
+DROP TABLE arrlp;
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..348719bd62 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1014,23 +1014,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
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 8f9991ef18..f04d6c6114 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -707,3 +707,9 @@ COMMENT ON COLUMN parted_col_comment.a IS 'Partition key';
SELECT obj_description('parted_col_comment'::regclass);
\d+ parted_col_comment
DROP TABLE parted_col_comment;
+
+-- list partitioning on array type column
+CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+\d+ arrlp12
+DROP TABLE arrlp;
--
2.11.0
(2018/01/26 10:15), Amit Langote wrote:
On 2018/01/25 21:17, Etsuro Fujita wrote:
Some minor comments:
+ /* + * Construct an ArrayExpr for the non-null partition + * values + */ + arrexpr = makeNode(ArrayExpr); + arrexpr->array_typeid = + !type_is_array(key->parttypid[0]) + ? get_array_type(key->parttypid[0]) + : key->parttypid[0];We test the type_is_array() above in this bit, so I don't think we need to
test that again here.Ah, you're right. Fixed.
Thanks. I think the updated version is fine, but I think we can
simplify the change in this part a bit further, so I modified your
patch. I also adjusted some comments in that change a little bit.
Attached is a modified version of the patch. What do you think about
that? Please let me know. If that is okay, I'll mark this as Ready for
Committer.
Attached updated patch. Thanks again.
Thanks for updating the patch!
Best regards,
Etsuro Fujita
Attachments:
v3-0001-Change-how-list-partition-constraint-is-emitted-efujita.patchtext/x-diff; name=v3-0001-Change-how-list-partition-constraint-is-emitted-efujita.patchDownload
*** a/src/backend/catalog/partition.c
--- b/src/backend/catalog/partition.c
***************
*** 1625,1642 **** 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;
break;
}
--- 1625,1682 ----
{
case PARTITION_STRATEGY_LIST:
{
! List *elems = (List *) arg2;
! int nelems = list_length(elems);
! Assert(keynum == 0);
! if (nelems > 1 &&
! !type_is_array(key->parttypid[keynum]))
! {
! ArrayExpr *arrexpr;
! ScalarArrayOpExpr *saopexpr;
!
! /* Construct an ArrayExpr for the right-hand inputs */
! arrexpr = makeNode(ArrayExpr);
! arrexpr->array_typeid =
! get_array_type(key->parttypid[keynum]);
! arrexpr->array_collid = key->parttypcoll[keynum];
! arrexpr->element_typeid = key->parttypid[keynum];
! arrexpr->elements = elems;
! arrexpr->multidims = false;
! arrexpr->location = -1;
!
! /* 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, arrexpr);
! saopexpr->location = -1;
!
! result = (Expr *) saopexpr;
! }
! else
! {
! List *elemops = NIL;
! ListCell *lc;
!
! 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 = nelems > 1 ? makeBoolExpr(OR_EXPR, elemops, -1) : linitial(elemops);
! }
break;
}
***************
*** 1758,1768 **** 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;
bool list_has_null = false;
/*
--- 1798,1807 ----
PartitionKey key = RelationGetPartitionKey(parent);
List *result;
Expr *keyCol;
Expr *opexpr;
NullTest *nulltest;
ListCell *cell;
! List *elems = NIL;
bool list_has_null = false;
/*
***************
*** 1828,1834 **** get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
false, /* isnull */
key->parttypbyval[0]);
! arrelems = lappend(arrelems, val);
}
}
else
--- 1867,1873 ----
false, /* isnull */
key->parttypbyval[0]);
! elems = lappend(elems, val);
}
}
else
***************
*** 1843,1872 **** get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
if (val->constisnull)
list_has_null = true;
else
! arrelems = lappend(arrelems, copyObject(val));
}
}
! if (arrelems)
{
! /* 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);
}
else
{
! /* If there are no partition values, we don't need an = ANY expr */
opexpr = NULL;
}
--- 1882,1906 ----
if (val->constisnull)
list_has_null = true;
else
! elems = lappend(elems, copyObject(val));
}
}
! if (elems)
{
! /*
! * Generate the operator expression from the non-null partition
! * values.
! */
opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
! keyCol, (Expr *) elems);
}
else
{
! /*
! * If there are no partition values, we don't need an operator
! * expression.
! */
opexpr = NULL;
}
*** a/src/test/regress/expected/create_table.out
--- b/src/test/regress/expected/create_table.out
***************
*** 737,743 **** 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])))
Check constraints:
"check_a" CHECK (length(a) > 0)
"part_b_b_check" CHECK (b >= 0)
--- 737,743 ----
a | text | | | | extended | |
b | integer | | not null | 1 | plain | |
Partition of: parted FOR VALUES IN ('b')
! 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,756 **** 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 key: RANGE (b)
Check constraints:
"check_a" CHECK (length(a) > 0)
--- 750,756 ----
a | text | | | | extended | |
b | integer | | not null | 0 | plain | |
Partition of: parted FOR VALUES IN ('c')
! Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text))
Partition key: RANGE (b)
Check constraints:
"check_a" CHECK (length(a) > 0)
***************
*** 764,770 **** 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))
Check constraints:
"check_a" CHECK (length(a) > 0)
--- 764,770 ----
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 = 'c'::text) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10))
Check constraints:
"check_a" CHECK (length(a) > 0)
***************
*** 863,865 **** Partition key: LIST (a)
--- 863,877 ----
Number of partitions: 0
DROP TABLE parted_col_comment;
+ -- list partitioning on array type column
+ CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+ CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+ \d+ arrlp12
+ Table "public.arrlp12"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+ --------+-----------+-----------+----------+---------+----------+--------------+-------------
+ a | integer[] | | | | extended | |
+ Partition of: arrlp FOR VALUES IN ('{1}', '{2}')
+ Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray OPERATOR(pg_catalog.=) '{2}'::integer[])))
+
+ DROP TABLE arrlp;
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
***************
*** 1863,1869 **** 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])))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
--- 1863,1869 ----
c2 | text | | | | | extended | |
c3 | date | | | | | plain | |
Partition of: pt2 FOR VALUES IN (1)
! Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
***************
*** 1935,1941 **** 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])))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
--- 1935,1941 ----
c2 | text | | | | | extended | |
c3 | date | | | | | plain | |
Partition of: pt2 FOR VALUES IN (1)
! Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
***************
*** 1963,1969 **** 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])))
Check constraints:
"p21chk" CHECK (c2 <> ''::text)
Server: s0
--- 1963,1969 ----
c2 | text | | | | | extended | |
c3 | date | | not null | | | plain | |
Partition of: pt2 FOR VALUES IN (1)
! Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1))
Check constraints:
"p21chk" CHECK (c2 <> ''::text)
Server: s0
*** a/src/test/regress/expected/partition_prune.out
--- b/src/test/regress/expected/partition_prune.out
***************
*** 1014,1036 **** 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)
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)
explain (costs off) select * from boolpart where a is true or a is not true;
QUERY PLAN
--- 1014,1032 ----
Append
-> Seq Scan on boolpart_f
Filter: (NOT a)
-> Seq Scan on boolpart_default
Filter: (NOT a)
! (5 rows)
explain (costs off) select * from boolpart where not a = false;
QUERY PLAN
------------------------------------
Append
-> Seq Scan on boolpart_t
Filter: a
-> Seq Scan on boolpart_default
Filter: a
! (5 rows)
explain (costs off) select * from boolpart where a is true or a is not true;
QUERY PLAN
*** a/src/test/regress/sql/create_table.sql
--- b/src/test/regress/sql/create_table.sql
***************
*** 707,709 **** COMMENT ON COLUMN parted_col_comment.a IS 'Partition key';
--- 707,715 ----
SELECT obj_description('parted_col_comment'::regclass);
\d+ parted_col_comment
DROP TABLE parted_col_comment;
+
+ -- list partitioning on array type column
+ CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+ CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+ \d+ arrlp12
+ DROP TABLE arrlp;
Fujita-san,
On 2018/01/26 21:31, Etsuro Fujita wrote:
(2018/01/26 10:15), Amit Langote wrote:
On 2018/01/25 21:17, Etsuro Fujita wrote:
Some minor comments:
+ /* + * Construct an ArrayExpr for the non-null partition + * values + */ + arrexpr = makeNode(ArrayExpr); + arrexpr->array_typeid = + !type_is_array(key->parttypid[0]) + ? get_array_type(key->parttypid[0]) + : key->parttypid[0];We test the type_is_array() above in this bit, so I don't think we need to
test that again here.Ah, you're right. Fixed.
Thanks. I think the updated version is fine, but I think we can simplify
the change in this part a bit further, so I modified your patch. I also
adjusted some comments in that change a little bit. Attached is a modified
version of the patch. What do you think about that? Please let me know.
If that is okay, I'll mark this as Ready for Committer.
That looks good, thanks.
Regards,
Amit
(2018/01/29 9:50), Amit Langote wrote:
On 2018/01/26 21:31, Etsuro Fujita wrote:
Attached is a modified
version of the patch. What do you think about that? Please let me know.
If that is okay, I'll mark this as Ready for Committer.That looks good, thanks.
Cool! One thing I noticed to revise the patch a bit further is to add
an assertion to make_partition_op_expr that the number of the right-hand
inputs to generate an operator expression from for list-partitioning
should be >=1; in that case we call that function if there is at least
one non-null partition value as the right-hand input, so that should be
asserted successfully there. I think the assertion is a good thing, so
I modified the patch. Updated patch attached. Because I made no
changes other than that, I'll make this as Ready for Committer.
Best regards,
Etsuro Fujita
Attachments:
v3-0001-Change-how-list-partition-constraint-is-emitted-efujita-2.patchtext/x-diff; name=v3-0001-Change-how-list-partition-constraint-is-emitted-efujita-2.patchDownload
*** a/src/backend/catalog/partition.c
--- b/src/backend/catalog/partition.c
***************
*** 1625,1642 **** 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;
break;
}
--- 1625,1684 ----
{
case PARTITION_STRATEGY_LIST:
{
! List *elems = (List *) arg2;
! int nelems = list_length(elems);
! Assert(nelems >= 1);
! Assert(keynum == 0);
! if (nelems > 1 &&
! !type_is_array(key->parttypid[keynum]))
! {
! ArrayExpr *arrexpr;
! ScalarArrayOpExpr *saopexpr;
!
! /* Construct an ArrayExpr for the right-hand inputs */
! arrexpr = makeNode(ArrayExpr);
! arrexpr->array_typeid =
! get_array_type(key->parttypid[keynum]);
! arrexpr->array_collid = key->parttypcoll[keynum];
! arrexpr->element_typeid = key->parttypid[keynum];
! arrexpr->elements = elems;
! arrexpr->multidims = false;
! arrexpr->location = -1;
!
! /* 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, arrexpr);
! saopexpr->location = -1;
!
! result = (Expr *) saopexpr;
! }
! else
! {
! List *elemops = NIL;
! ListCell *lc;
!
! 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 = nelems > 1 ? makeBoolExpr(OR_EXPR, elemops, -1) : linitial(elemops);
! }
break;
}
***************
*** 1758,1768 **** 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;
bool list_has_null = false;
/*
--- 1800,1809 ----
PartitionKey key = RelationGetPartitionKey(parent);
List *result;
Expr *keyCol;
Expr *opexpr;
NullTest *nulltest;
ListCell *cell;
! List *elems = NIL;
bool list_has_null = false;
/*
***************
*** 1828,1834 **** get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
false, /* isnull */
key->parttypbyval[0]);
! arrelems = lappend(arrelems, val);
}
}
else
--- 1869,1875 ----
false, /* isnull */
key->parttypbyval[0]);
! elems = lappend(elems, val);
}
}
else
***************
*** 1843,1872 **** get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
if (val->constisnull)
list_has_null = true;
else
! arrelems = lappend(arrelems, copyObject(val));
}
}
! if (arrelems)
{
! /* 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);
}
else
{
! /* If there are no partition values, we don't need an = ANY expr */
opexpr = NULL;
}
--- 1884,1908 ----
if (val->constisnull)
list_has_null = true;
else
! elems = lappend(elems, copyObject(val));
}
}
! if (elems)
{
! /*
! * Generate the operator expression from the non-null partition
! * values.
! */
opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
! keyCol, (Expr *) elems);
}
else
{
! /*
! * If there are no partition values, we don't need an operator
! * expression.
! */
opexpr = NULL;
}
*** a/src/test/regress/expected/create_table.out
--- b/src/test/regress/expected/create_table.out
***************
*** 737,743 **** 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])))
Check constraints:
"check_a" CHECK (length(a) > 0)
"part_b_b_check" CHECK (b >= 0)
--- 737,743 ----
a | text | | | | extended | |
b | integer | | not null | 1 | plain | |
Partition of: parted FOR VALUES IN ('b')
! 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,756 **** 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 key: RANGE (b)
Check constraints:
"check_a" CHECK (length(a) > 0)
--- 750,756 ----
a | text | | | | extended | |
b | integer | | not null | 0 | plain | |
Partition of: parted FOR VALUES IN ('c')
! Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text))
Partition key: RANGE (b)
Check constraints:
"check_a" CHECK (length(a) > 0)
***************
*** 764,770 **** 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))
Check constraints:
"check_a" CHECK (length(a) > 0)
--- 764,770 ----
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 = 'c'::text) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10))
Check constraints:
"check_a" CHECK (length(a) > 0)
***************
*** 863,865 **** Partition key: LIST (a)
--- 863,877 ----
Number of partitions: 0
DROP TABLE parted_col_comment;
+ -- list partitioning on array type column
+ CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+ CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+ \d+ arrlp12
+ Table "public.arrlp12"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+ --------+-----------+-----------+----------+---------+----------+--------------+-------------
+ a | integer[] | | | | extended | |
+ Partition of: arrlp FOR VALUES IN ('{1}', '{2}')
+ Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray OPERATOR(pg_catalog.=) '{2}'::integer[])))
+
+ DROP TABLE arrlp;
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
***************
*** 1863,1869 **** 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])))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
--- 1863,1869 ----
c2 | text | | | | | extended | |
c3 | date | | | | | plain | |
Partition of: pt2 FOR VALUES IN (1)
! Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
***************
*** 1935,1941 **** 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])))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
--- 1935,1941 ----
c2 | text | | | | | extended | |
c3 | date | | | | | plain | |
Partition of: pt2 FOR VALUES IN (1)
! Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1))
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
***************
*** 1963,1969 **** 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])))
Check constraints:
"p21chk" CHECK (c2 <> ''::text)
Server: s0
--- 1963,1969 ----
c2 | text | | | | | extended | |
c3 | date | | not null | | | plain | |
Partition of: pt2 FOR VALUES IN (1)
! Partition constraint: ((c1 IS NOT NULL) AND (c1 = 1))
Check constraints:
"p21chk" CHECK (c2 <> ''::text)
Server: s0
*** a/src/test/regress/expected/partition_prune.out
--- b/src/test/regress/expected/partition_prune.out
***************
*** 1014,1036 **** 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)
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)
explain (costs off) select * from boolpart where a is true or a is not true;
QUERY PLAN
--- 1014,1032 ----
Append
-> Seq Scan on boolpart_f
Filter: (NOT a)
-> Seq Scan on boolpart_default
Filter: (NOT a)
! (5 rows)
explain (costs off) select * from boolpart where not a = false;
QUERY PLAN
------------------------------------
Append
-> Seq Scan on boolpart_t
Filter: a
-> Seq Scan on boolpart_default
Filter: a
! (5 rows)
explain (costs off) select * from boolpart where a is true or a is not true;
QUERY PLAN
*** a/src/test/regress/sql/create_table.sql
--- b/src/test/regress/sql/create_table.sql
***************
*** 707,709 **** COMMENT ON COLUMN parted_col_comment.a IS 'Partition key';
--- 707,715 ----
SELECT obj_description('parted_col_comment'::regclass);
\d+ parted_col_comment
DROP TABLE parted_col_comment;
+
+ -- list partitioning on array type column
+ CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
+ CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
+ \d+ arrlp12
+ DROP TABLE arrlp;
Fujita-san,
On 2018/01/29 15:15, Etsuro Fujita wrote:
(2018/01/29 9:50), Amit Langote wrote:
On 2018/01/26 21:31, Etsuro Fujita wrote:
Attached is a modified
version of the patch. What do you think about that? Please let me know.
If that is okay, I'll mark this as Ready for Committer.That looks good, thanks.
Cool! One thing I noticed to revise the patch a bit further is to add an
assertion to make_partition_op_expr that the number of the right-hand
inputs to generate an operator expression from for list-partitioning
should be >=1; in that case we call that function if there is at least one
non-null partition value as the right-hand input, so that should be
asserted successfully there. I think the assertion is a good thing, so I
modified the patch.
Ah, good call.
Updated patch attached. Because I made no changes
other than that, I'll make this as Ready for Committer.
Thanks a lot for reviewing.
Regards,
Amit
On Mon, Jan 29, 2018 at 1:22 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
Updated patch attached. Because I made no changes
other than that, I'll make this as Ready for Committer.Thanks a lot for reviewing.
Committed and back-patched to v10.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 2018/02/01 6:08, Robert Haas wrote:
On Mon, Jan 29, 2018 at 1:22 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:Updated patch attached. Because I made no changes
other than that, I'll make this as Ready for Committer.Thanks a lot for reviewing.
Committed and back-patched to v10.
Thank you.
Regards,
Amit