Re: Report error position in partition bound check

[ cc'ing Peter, since his opinion seems to have got us here in the first place ]

Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> On Thu, Sep 24, 2020 at 7:19 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> However, while I was looking at it I couldn't help noticing that
>> transformPartitionBoundValue's handling of collation concerns seems
>> less than sane. There are two things bugging me:
>>
>> 1. Why does it care about the expression's collation only when there's
>> a top-level CollateExpr? For example, that means we get an error for
>>
>> regression=# create table p (f1 text collate "C") partition by list(f1);
>> CREATE TABLE
>> regression=# create table c1 partition of p for values in ('a' collate "POSIX");
>> ERROR: collation of partition bound value for column "f1" does not match partition key collation "C"
>>
>> but not this:
>>
>> regression=# create table c2 partition of p for values in ('a' || 'b' collate "POSIX");
>> CREATE TABLE
>>
>> Given that we will override the expression's collation with the partition
>> column's collation anyway, I don't see why we have this check at all,
>> so my preference is to just rip out the entire stanza beginning with
>> "if (IsA(value, CollateExpr))". If we keep it, though, I think it needs
>> to do something else that is more general.

> I dug up the discussion which resulted in this test being added and
> found that Peter E had opined that this failure should not occur [1].

Well, I agree with Peter to that extent, but my opinion is that *none*
of these cases ought to be errors. What we're doing here is performing
an implicit assignment-level coercion of the expression to the type of
the column, and changing the collation is allowed as part of that:

regression=# create table foo (f1 text collate "C");
CREATE TABLE
regression=# insert into foo values ('a' COLLATE "POSIX");
INSERT 0 1
regression=# update foo set f1 = 'b' COLLATE "POSIX";
UPDATE 1

So I find it completely inconsistent that the partitioning logic
complains about equivalent cases. I think we should just rip the
whole thing out, as per the attached draft. This causes several
regression test results to change, but AFAICS those are only there
to exercise the error tests that I think we should get rid of.

regards, tom lane