(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
*** 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;
