From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | pgsql-hackers(at)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us |
Subject: | Re: using expression syntax for partition bounds |
Date: | 2019-01-15 13:24:33 |
Message-ID: | 04661508-b6f5-177e-6f6b-c4cb8426b9f0@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Antonin Houska | 2019-01-15 14:06:18 | Re: [HACKERS] WIP: Aggregation push-down |
Previous Message | Dave Cramer | 2019-01-15 13:00:24 | Re: Libpq support to connect to standby server as priority |