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
