> On Wed, Apr 01, 2020 at 04:08:42PM +0900, Michael Paquier wrote:
> > On Sat, Mar 28, 2020 at 01:53:03PM +0100, Dmitry Dolgov wrote:
> > Thanks for reporting! Looks like transformPartitionBoundValue needs to
> > check that the source collumn is collatable, then we get more expected
> > result:
> >
> > =# create table part_coll partition of parted for values in ('1' collate "POSIX");
> > ERROR: 42804: collations are not supported by type integer
>
> We have here a side effect of 7c079d74, that has given the possibility
> to pass down general expressions for partition bounds.
Yep. That's why I've placed a test for this fix close to those
introduced in this commit.
> > + if (type_is_collatable(colType))
> > + {
> > + if (!OidIsValid(exprCollOid))
> > + ereport(ERROR,
> > + (errcode(ERRCODE_INDETERMINATE_COLLATION),
> > + errmsg("could not determine which collation to use for partition expression"),
> > + errhint("Use the COLLATE clause to set the collation explicitly.")));
> > + }
> > + else
> > + {
> > + if (OidIsValid(exprCollOid))
> > + ereport(ERROR,
> > + (errcode(ERRCODE_DATATYPE_MISMATCH),
> > + errmsg("collations are not supported by type %s",
> > + format_type_be(colType))));
> > + }
> > +
>
> It seems to me that the first error cannot happen, and should not
> happen. When the expression is transformed, there is a lookup at the
> collation used via transformCollateClause() -> LookupCollation() ->
> get_collation_oid() which would never return an invalid OID if the
> collation exists, throwing an error instead. The reason why a similar
> check exists in tablecmds.c is that there can be a collation override,
> no?
Good point, I believe you're right. This means that the fix can be
reduced as in attachments.
