Repository navigation
Translate DateTime.Ticks on SQLite without losing precision - #39598
Open
Abde1rahman1 wants to merge 1 commit into
Open
Abde1rahman1 wants to merge 1 commit into
Abde1rahman1 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A critical active finding shows the new translation breaks DateTime properties mapped to REAL columns by applying text operations to numeric Julian-day values, so the change requires correction and a regression test.
1 open issue
What changed in this PR
This pull request fixes SQLite DateTime.Ticks translation by preserving sub-millisecond precision for compatible text-backed columns and avoiding floating-point precision loss.
Changes:
- Uses integer arithmetic and direct fractional-text extraction for eligible
DateTimecolumns. - Uses rounded millisecond Julian-day arithmetic for computed or unsupported operands.
- Adds SQLite regression tests and updates the Northwind SQL baseline.
- Requires restricting the text-based translation to non-REAL mappings, correcting the fractional tick test value, and covering both synchronous and asynchronous execution.
| File | Description |
|---|---|
| test/EFCore.Sqlite.FunctionalTests/Query/Translations/Temporal/DateTimeTranslationsSqliteTest.cs | Adds regression tests for column, projection, and computed-value DateTime.Ticks translations. |
| test/EFCore.Sqlite.FunctionalTests/Query/NorthwindSelectQuerySqliteTest.cs | Updates the expected SQLite SQL baseline for the Northwind DateTime.Ticks projection. |
| src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteDateTimeMemberTranslator.cs | Implements precise text-based tick translation for eligible columns and millisecond-rounded fallback translation for other date/time expressions. |
Review effort: Lite
|
|
||
| private SqlExpression TranslateTicks(SqlExpression instance) | ||
| { | ||
| if (instance is ColumnExpression { TypeMapping: SqliteDateTimeTypeMapping { Converter: null } }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #39531
DateTime.Tickson SQLite was translated toCAST((julianday(x) - 1721425.5) * 864000000000.0 AS INTEGER), which has two problems:...10.123gave630298926101229952instead of630298926101230000).As a result, a query such as
Where(e => e.DateTime.Ticks == ticks)could miss rows whose value has a fractional second.Changes
DateTimecolumns are now translated using integer arithmetic for the whole seconds, with the fractional digits read directly from the stored text (yyyy-MM-dd HH:mm:ss.FFFFFFF), which is exact to the tick:The fraction is cut off before calling
strftime, so the result doesn't depend on how SQLite rounds it.Any other operand (e.g. the result of another date/time function, which is already limited to milliseconds) is evaluated once, and the Julian day is rounded to whole milliseconds while it's still exactly representable as a double:
The exact translation is only used when the operand is a column mapped with
SqliteDateTimeTypeMappingand no value converter, since it references the operand twice and relies on the text format.Tests
Updated the baseline for
NorthwindSelectQuerySqliteTest.Select_datetime_Ticks_component.Added
Ticks,Ticks_projectionandTicks_on_computed_valuetoDateTimeTranslationsSqliteTest, covering a sub-millisecond match on a column, all seeded values against the client result, and the non-column translation.I've read the guidelines for contributing and seen the walkthrough
I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
The code builds and tests pass locally (also verified by our automated build checks)
Commit messages follow this format:
Tests for the changes have been added (for bug fixes / features)
Code follows the same patterns and style as existing code in this repo