Skip to content
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

Fix lower/upper-case not to depend on JVM locale #10521

Conversation

findepi
Copy link
Member

@findepi findepi commented Jun 17, 2024

Found by #10518

@findepi findepi force-pushed the findepi/fix-lowercase-not-to-depend-on-jvm-locale-3870f6 branch from 340b000 to fa87bb5 Compare June 17, 2024 15:51
@findepi findepi changed the title Fix lowercase not to depend on JVM locale Fix lower/upper-case not to depend on JVM locale Jun 17, 2024
@ajantha-bhat
Copy link
Member

I think this PR also fixes #9276

@findepi
Copy link
Member Author

findepi commented Jun 18, 2024

@ajantha-bhat thanks for the link!
I am not sure this is a full fix though. There may be other Locale-dependent APIs besides toLowerCase & toUpperCase.
Let's keep the issue open for now.

@findepi
Copy link
Member Author

findepi commented Jun 18, 2024

there is a conflict (core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java), rebasing

@findepi findepi force-pushed the findepi/fix-lowercase-not-to-depend-on-jvm-locale-3870f6 branch from fa87bb5 to 1045926 Compare June 18, 2024 11:01
@Fokko
Copy link
Contributor

Fokko commented Jun 18, 2024

Thanks @findepi for the PR, and thanks @ajantha-bhat, @nastra and @dimas-b for the review 🙌

@Fokko Fokko merged commit 7911406 into apache:main Jun 18, 2024
42 checks passed
@findepi findepi deleted the findepi/fix-lowercase-not-to-depend-on-jvm-locale-3870f6 branch June 18, 2024 18:00
@findepi
Copy link
Member Author

findepi commented Jun 18, 2024

thank you @ajantha-bhat @nastra @dimas-b @Fokko for your reviews!

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.

None yet

5 participants