Align JSON and M.D.Sqlite representations#36951
Align JSON and M.D.Sqlite representations#36951krisbiradar wants to merge 8 commits intodotnet:mainfrom
Conversation
|
@dotnet-policy-service agree |
|
Hi @roji , can we get this merged anytime soon ? |
249ae47 to
6b86657
Compare
|
Hi @roji would like to see my changes going live any luck with that...? |
|
@krisbiradar we're unfortunately very backed up with processing community PRs... I promise to do my very best to get this in for EF 11, but it may still take some time. |
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #36749 by aligning the JSON and Microsoft.Data.Sqlite textual representations for TimeOnly and byte[] types in SQLite. Previously, there were discrepancies where EF Core's JSON representation would preserve trailing zeros for TimeOnly values (e.g., 12:30:45.0000000), while Microsoft.Data.Sqlite would not (e.g., 12:30:45), causing comparison failures.
Changes:
- Created
SqliteJsonTimeOnlyReaderWriterto format TimeOnly values consistently with M.D.Sqlite (omitting trailing zeros when seconds are whole) - Modified
SqliteByteArrayTypeMappingto detect JSON columns and configure parameters to use text binding for hex-encoded strings - Enhanced
SqliteValueBinderto convert byte[] to hex strings when binding to text columns (JSON columns)
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/EFCore.Sqlite.Core/Storage/Json/Internal/SqliteJsonTimeOnlyReaderWriter.cs |
New SQLite-specific JSON reader/writer for TimeOnly that matches M.D.Sqlite's formatting |
src/EFCore.Sqlite.Core/Storage/Internal/SqliteByteArrayTypeMapping.cs |
Added JSON column detection and parameter configuration to use text binding |
src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs |
Override FindMapping to return JSON-column-aware byte array mappings |
src/EFCore.Sqlite.Core/Storage/Internal/SqliteTimeOnlyTypeMapping.cs |
Updated to use SqliteJsonTimeOnlyReaderWriter instead of generic JsonTimeOnlyReaderWriter |
src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs |
Added logic to convert byte[] to hex strings when binding to text columns (for JSON) |
test/EFCore.Sqlite.FunctionalTests/Update/JsonUpdateSqliteTest.cs |
Updated baselines to reflect new TimeOnly formatting without trailing zeros |
test/EFCore.Sqlite.FunctionalTests/Types/SqliteTemporalTypeTest.cs |
Re-enabled Query_property_within_json test for TimeOnly |
test/EFCore.Sqlite.FunctionalTests/Types/SqliteMiscellaneousTypeTest.cs |
Re-enabled Query_property_within_json test for byte[] |
test/EFCore.Sqlite.FunctionalTests/JsonTypesSqliteTest.cs |
Added test overrides to verify correct TimeOnly JSON formatting |
test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel*/ManyTypesEntityType.cs |
Updated scaffolding baselines to use SqliteJsonTimeOnlyReaderWriter |
src/EFCore.Sqlite.Core/Storage/Json/Internal/SqliteJsonTimeOnlyReaderWriter.cs
Outdated
Show resolved
Hide resolved
|
|
||
|
|
There was a problem hiding this comment.
Extra blank lines should be removed. There should only be one blank line between the XML documentation comment and the next member declaration.
test/EFCore.Sqlite.FunctionalTests/Types/SqliteMiscellaneousTypeTest.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs
Outdated
Show resolved
Hide resolved
…peTest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…yReaderWriter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…e.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Thanks for the update @roji! |
Fixes #36749