Re: list partition constraint shape

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