Re: using expression syntax for partition bounds

On 15/01/2019 07:31, Amit Langote wrote:
>> Is "partition bound" the right term? For list partitioning, it's not
>> really a bound. Maybe it's OK.
>
> Hmm, maybe "partition bound value"? Just want to stress that the
> expression specifies "bounding" value of a partition.

I was more concerned about the term "bound", which it is not range
partitioning. But I can't think of a better term.

I wouldn't change expr to value as you have done between v7 and v8,
since the point of this patch is to make expressions valid where
previously only values were.

>> I don't see any treatment of non-immutable expressions. There is a test
>> case with a non-immutable cast that was removed, but that's all. There
>> was considerable discussion earlier in the thread on whether
>> non-immutable expressions should be allowed. I'm not sure what the
>> resolution was, but in any case there should be documentation and tests
>> of the outcome.
>
> The partition bound expression is evaluated only once, that is, when the
> partition creation command is executed, and what gets stored in the
> catalog is a Const expression that contains the value that was computed,
> not the original non-immutable expression. So, we don't need to do
> anything special in terms of code to handle users possibly specifying a
> non-immutable expression as partition bound. Tom said something in that
> thread which seems to support such a position:
>
> https://www.postgresql.org/message-id/22534.1523374457%40sss.pgh.pa.us

OK, if we are going with that approach, then it needs to be documented
and there should be test cases.

>> The collation handling might need another look. The following is
>> allowed without any diagnostic:
>>
>> CREATE TABLE t2 (
>> a text COLLATE "POSIX"
>> ) PARTITION BY RANGE (a);
>> CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM ('foo' COLLATE "C") TO
>> ('xyz');
>>
>> I think the correct behavior would be that an explicit collation in the
>> partition bound expression is an error.

> But that's essentially ignoring the collation specified by the user for
> the partition bound value without providing any information about that to
> the user. I've fixed that by explicitly checking if the collate clause in
> the partition bound value expression contains the same collation as the
> parent partition key collation and give an error otherwise.

I think that needs more refinement. In v8, the following errors

CREATE TABLE t2 ( a name COLLATE "POSIX" ) PARTITION BY RANGE (a);
CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM (name 'foo') TO (name
'xyz');
ERROR: collation of partition bound value for column "a" does not match
partition key collation "POSIX"

The problem here is that the "name" type has a collation that is neither
the one of the column nor the default collation. We can allow that.
What we don't want is someone writing an explicit COLLATE clause. I
think we just need to check that there is no top-level COLLATE clause.
This would then sort of match the logic parse_collate.c for combining
collations.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services