Fix SQLite decimal comparison with turkish culture#37429
Fix SQLite decimal comparison with turkish culture#37429salihozkara wants to merge 7 commits intodotnet:mainfrom
Conversation
[release/10.0] Source code updates from dotnet/dotnet
[release/10.0] Source code updates from dotnet/dotnet
Updated decimal parsing in EF_DECIMAL collation to use CultureInfo.InvariantCulture for consistent ordering across cultures. Added a test to verify correct ordering of decimal values under Turkish culture.
Eliminated the unnecessary 'using System.Globalization;' directive from BuiltInDataTypesSqliteTest.cs to clean up the code.
There was a problem hiding this comment.
Pull request overview
This PR fixes a culture-specific bug in SQLite decimal comparison where using Turkish culture (or any culture with a different decimal separator) would cause incorrect sorting of decimal values.
- Updated the
EF_DECIMALcollation to parse decimal strings usingCultureInfo.InvariantCulture - Added comprehensive test coverage for decimal ordering under Turkish culture (tr-TR)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs |
Updated the EF_DECIMAL collation to use InvariantCulture when parsing decimal strings, fixing culture-sensitive comparison issues |
test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs |
Added test with Turkish culture to verify decimal ordering works correctly regardless of application culture |
src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs
Outdated
Show resolved
Hide resolved
Adjusted the TestDecimal values for test entities in Can_query_OrderBy_decimal_with_Turkish_culture to ensure correct ordering and test coverage.
AndriySvyryd
requested review from
AndriySvyryd and
cincuranet
as code owners
Can you please open an issue? We try to always have an issue - especially when there's a bug - so that it's clear which release contains what, and when a bug was fixed, etc. |
|
roji
left a comment
roji
left a comment
There was a problem hiding this comment.
Thanks for submitting a fix! See below for some comments.
|
|
||
| <PropertyGroup Label="Version settings"> | ||
| <VersionPrefix>10.0.2</VersionPrefix> | ||
| <VersionPrefix>10.0.3</VersionPrefix> |
There was a problem hiding this comment.
Please revert, we handle version bumps separately.
There was a problem hiding this comment.
This was caused by the base branch change. Please rebase on main
src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| [ConditionalFact, UseCulture("tr-TR")] | ||
| public virtual void Can_query_OrderBy_decimal_with_Turkish_culture() |
There was a problem hiding this comment.
Can you please reduce this test to the bare minimum, removing all the unneeded stuff? Check out the minimal repro posted here.
…tion.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@salihozkara there's generally no need to open a new PR, you can always push-force a branch into the PR, replacing its contents. Please avoid doing that, as review history is more difficult to track etc. |
Fixes #37432
Summary of the changes
Details
SqliteConnection.CreateCollationto useCultureInfo.InvariantCulturewhen parsing decimal stringsFixes
This addresses the issue where SQLite decimal comparison would fail when the application was running under a culture that uses a different decimal separator (e.g., Turkish using comma instead of period).