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

Reply via email to