Skip to content

Allow keywords in locales when creating ICU collations (Fixes issue #6903) #6914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 2, 2021

Conversation

tkeinz
Copy link
Contributor

@tkeinz tkeinz commented Jul 31, 2021

This fix adds a fallback check when trying to create an ICU collator and the check against locale base names has failed. With this fallback check, locales with keywords are accepted when creating a collation. In my tests, the collation worked as expected when applied to a table column.

Please let me know in case this is not the right branch against which to submit the change.

@AppVeyorBot
Copy link

Build firebird 1.0.3105 completed (commit a33f0a463f by @)

@tkeinz tkeinz changed the title Allow keywords in locales when creating ICU collations (#6903) Allow keywords in locales when creating ICU collations (Fixes issue #6903) Jul 31, 2021
@asfernandes asfernandes linked an issue Aug 2, 2021 that may be closed by this pull request
continue;

icu->ucolClose(testCollator);
if (U_FAILURE(status) || (status == U_USING_DEFAULT_WARNING))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried create collation unicode_ptbr for utf8 from unicode 'LOCALE=pt_BRx' and it worked, but should not.

Can you change that to if (status != U_ZERO_ERROR) to treat warnings as errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, good catch. Thank you!

While I had tested with completely invalid locale names, I missed testing with partially valid ones. Changed as requested; now your example also fails (and the examples from my use cases continue to work).

@asfernandes
Copy link
Member

For master, I'm going to completely avoid the icu->uloc* functions.

@asfernandes asfernandes merged commit 2fcab58 into FirebirdSQL:v4.0-release Aug 2, 2021
asfernandes pushed a commit that referenced this pull request Aug 4, 2021
…6903) (#6914)

* Allow keywords in locales when creating ICU collations

* Change ICU locale creation check to fail for any warning.

Co-authored-by: Thomas Keinz <[email protected]>
@pavel-zotov
Copy link

::: test details :::
See tests.bugs.gh_6903

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to create ICU-based collation with locale keywords
4 participants