CREATE COLLATION without LOCALE throws error in v15

Started by Kyle Spearrinover 3 years ago8 messagesbugs
Jump to latest
#1Kyle Spearrin
kspearrin@bitwarden.com

Hi,
I wanted to report a bug with the CREATE COLLATION query in Postgres 15.

The following query works in Postgres 12-14:

CREATE COLLATION "postgresIndetermanisticCollation" (LC_COLLATE =
'en-u-ks-primary',
LC_CTYPE = 'en-u-ks-primary',
PROVIDER = icu,
DETERMINISTIC = False
);

However, in Postgres 15 we see the following error:

ERROR: parameter "locale" must be specified

Changing the query to following resolves the issue:

CREATE COLLATION "postgresIndetermanisticCollation" (LOCALE =
'en-u-ks-primary',
PROVIDER = icu,
DETERMINISTIC = False
);

According to the docs here
<https://www.postgresql.org/docs/current/sql-createcollation.html&gt;, we
should be able to execute this query either way (as is evident when testing
in versions 11-14).

This causes problems with queries generated by our Entity Framework Core
ORM tool that uses npgsql. We don't have control to change the way
the COLLATION query is created, therefore preventing us from supporting
Postgres 15.

--
Kyle Spearrin

Follow Bitwarden on social media:
Twitter <https://twitter.com/bitwarden&gt; | Facebook
<https://www.facebook.com/bitwarden&gt; | GitHub <https://github.com/bitwarden&gt;

#2Jeff Davis
pgsql@j-davis.com
In reply to: Kyle Spearrin (#1)
Re: CREATE COLLATION without LOCALE throws error in v15

On Fri, 2022-12-02 at 14:14 -0500, Kyle Spearrin wrote:

However, in Postgres 15 we see the following error:

ERROR: parameter "locale" must be specified

I agree that this is some kind of bug. The docs seem pretty clear[1]https://www.postgresql.org/docs/devel/sql-createcollation.html:

"locale

This is a shortcut for setting LC_COLLATE and LC_CTYPE
at once. If you specify this, you cannot specify either
of those parameters."

The error appears to come from commit f2553d4306 ("Add option to use
ICU as global locale provider"). My guess is that it was seen as
clearer to write LOCALE, and that the error wasn't expected to cause
anyone a problem. But, given that it's causing a problem for ORMs, we
should fix it.

Simple patch attached. I intend to commit and backpatch to 15 in the
next day or so.

Looking at the docs for CREATE COLLATION[1]https://www.postgresql.org/docs/devel/sql-createcollation.html and CREATE DATABASE[2]https://www.postgresql.org/docs/devel/sql-createdatabase.html, we
could probably do more to make this area consistent. But that's for
another discussion.

[1]: https://www.postgresql.org/docs/devel/sql-createcollation.html
[2]: https://www.postgresql.org/docs/devel/sql-createdatabase.html

--
Jeff Davis
PostgreSQL Contributor Team - AWS

Attachments:

v1-0001-Re-allow-creating-ICU-collation-if-LC_COLLATE-mat.patchtext/x-patch; charset=UTF-8; name=v1-0001-Re-allow-creating-ICU-collation-if-LC_COLLATE-mat.patchDownload+4-1
#3Peter Eisentraut
peter_e@gmx.net
In reply to: Jeff Davis (#2)
Re: CREATE COLLATION without LOCALE throws error in v15

On 07.12.22 00:57, Jeff Davis wrote:

On Fri, 2022-12-02 at 14:14 -0500, Kyle Spearrin wrote:

However, in Postgres 15 we see the following error:

ERROR: parameter "locale" must be specified

I agree that this is some kind of bug. The docs seem pretty clear[1]:

"locale

This is a shortcut for setting LC_COLLATE and LC_CTYPE
at once. If you specify this, you cannot specify either
of those parameters."

The error appears to come from commit f2553d4306 ("Add option to use
ICU as global locale provider"). My guess is that it was seen as
clearer to write LOCALE, and that the error wasn't expected to cause
anyone a problem.

This was an intentional change because of underlying conceptual and
catalog changes. lc_collate, lc_ctype, and iculocale are now tracked
separately in the catalogs. This is also important because for a
database they actually are used separately. So this also keeps the
locale/collation settings for databases and collations consistent.

The "locale" parameter is now basically "default for whatever settings
are appropriate for the locale provider". We should change the
documentation in that direction.

#4Jeff Davis
pgsql@j-davis.com
In reply to: Peter Eisentraut (#3)
Re: CREATE COLLATION without LOCALE throws error in v15

On Wed, 2022-12-07 at 13:04 +0100, Peter Eisentraut wrote:

This was an intentional change because of underlying conceptual and
catalog changes.  lc_collate, lc_ctype, and iculocale are now tracked
separately in the catalogs.  This is also important because for a
database they actually are used separately.  So this also keeps the
locale/collation settings for databases and collations consistent.

That makes sense, but there also doesn't seem to be much harm in
supporting the old behavior where it works as long as ctype==collate.
As in my patch, it accepts either locale or ctype==collate as the icu
locale.

We can discourage relying on that by updating the documentation as you
suggest.

--
Jeff Davis
PostgreSQL Contributor Team - AWS

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Jeff Davis (#4)
Re: CREATE COLLATION without LOCALE throws error in v15

On 07.12.22 18:44, Jeff Davis wrote:

On Wed, 2022-12-07 at 13:04 +0100, Peter Eisentraut wrote:

This was an intentional change because of underlying conceptual and
catalog changes.  lc_collate, lc_ctype, and iculocale are now tracked
separately in the catalogs.  This is also important because for a
database they actually are used separately.  So this also keeps the
locale/collation settings for databases and collations consistent.

That makes sense, but there also doesn't seem to be much harm in
supporting the old behavior where it works as long as ctype==collate.
As in my patch, it accepts either locale or ctype==collate as the icu
locale.

The harm is that in the context of database-level collations, the
analogous syntax means something different. That's why in the context
of adding ICU support on the database level, this syntax for collation
objects was disabled.

#6Jeff Davis
pgsql@j-davis.com
In reply to: Peter Eisentraut (#5)
Re: CREATE COLLATION without LOCALE throws error in v15

On Thu, 2022-12-08 at 19:29 +0100, Peter Eisentraut wrote:

The harm is that in the context of database-level collations, the
analogous syntax means something different.

I see. I certainly agree that we shouldn't document the
lc_ctype=lc_collate hack for CREATE COLLATION for the ICU provider,
because we don't want to encourage it.

Kyle, perhaps after the docs are updated, you can reach out to the
npgsql project to see if they can fix it on their side?

--
Jeff Davis
PostgreSQL Contributor Team - AWS

#7Kyle Spearrin
kspearrin@bitwarden.com
In reply to: Jeff Davis (#6)
Re: CREATE COLLATION without LOCALE throws error in v15

Jeff,
It appears npgsql has already addressed it in an upcoming version, which we
are awaiting:
https://github.com/npgsql/efcore.pg/issues/2527

--
Kyle Spearrin

Follow Bitwarden on social media:
Twitter <https://twitter.com/bitwarden&gt; | Facebook
<https://www.facebook.com/bitwarden&gt; | GitHub <https://github.com/bitwarden&gt;

On Thu, Dec 8, 2022 at 2:59 PM Jeff Davis <pgsql@j-davis.com> wrote:

Show quoted text

On Thu, 2022-12-08 at 19:29 +0100, Peter Eisentraut wrote:

The harm is that in the context of database-level collations, the
analogous syntax means something different.

I see. I certainly agree that we shouldn't document the
lc_ctype=lc_collate hack for CREATE COLLATION for the ICU provider,
because we don't want to encourage it.

Kyle, perhaps after the docs are updated, you can reach out to the
npgsql project to see if they can fix it on their side?

--
Jeff Davis
PostgreSQL Contributor Team - AWS

#8Justin Baur
jbaur@bitwarden.com
In reply to: Jeff Davis (#6)
Re: CREATE COLLATION without LOCALE throws error in v15

Hi All,

After sending in the report we did find an issue that was resolved by them
that fixes it on their side (https://github.com/npgsql/efcore.pg/issues/2527).
They haven't released a version with the fix yet but we can wait, A docs
update would be totally fine with us.

Thanks for all your help.

Justin Baur

On Thu, Dec 8, 2022 at 2:59 PM Jeff Davis <pgsql@j-davis.com> wrote:

Show quoted text

On Thu, 2022-12-08 at 19:29 +0100, Peter Eisentraut wrote:

The harm is that in the context of database-level collations, the
analogous syntax means something different.

I see. I certainly agree that we shouldn't document the
lc_ctype=lc_collate hack for CREATE COLLATION for the ICU provider,
because we don't want to encourage it.

Kyle, perhaps after the docs are updated, you can reach out to the
npgsql project to see if they can fix it on their side?

--
Jeff Davis
PostgreSQL Contributor Team - AWS