Re: Report error position in partition bound check

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>, Alexandra Wang <alexandra.wanglei@gmail.com>, Daniel Gustafsson <daniel@yesql.se>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>, "Ashwin Agrawal (Pivotal)" <aagrawal@pivotal.io>
Date: 2020-09-23T22:19:09Z
Lists: pgsql-hackers
I looked this over and pushed it with some minor adjustments.

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.

2. Nothing is doing assign_expr_collations() on the partition expression.
This can trivially be shown to cause problems:

regression=# create table p (f1 bool) partition by list(f1);
CREATE TABLE
regression=# create table cf partition of p for values in ('a' < 'b');
ERROR:  could not determine which collation to use for string comparison
HINT:  Use the COLLATE clause to set the collation explicitly.


If we want to rip out the collation mismatch error altogether, then
fixing #2 would just require inserting assign_expr_collations() before
the expression_planner() call.  The other direction that would make
sense to me is to perform assign_expr_collations() after
coerce_to_target_type(), and then to complain if exprCollation()
isn't default and doesn't match the partition collation.  In any
case a specific test for a CollateExpr seems quite wrong.

			regards, tom lane



Commits

  1. Assign collations in partition bound expressions.

  2. Remove complaints about COLLATE clauses in partition bound values.

  3. Improve error cursor positions for problems with partition bounds.