Repository navigation
Fix NodaTime LocalDate subtraction and BCE year extraction - #4109
Merged
Merged
Conversation
Return calendar years, months and days instead of an elapsed-day interval. Cast operands to timestamp to avoid time zone dependence and cover calendar components, negative periods and month-end differences. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused translation and regression coverage match the intended PostgreSQL semantics, with no blocking issues identified in static review.
No open issues 🎉
What changed in this PR
Updates the PostgreSQL provider’s NodaTime LocalDate subtraction to return calendar period components using age(). This partially addresses #4089; the BCE .Year issue remains out of scope.
Changes:
- Replaces day-only interval construction with
age(), casting both operands to timestamps without time zones. - Adds tests for calendar components, negative periods, and the intentional PostgreSQL month-end behavior.
- Updates existing subtraction SQL baselines.
| File | Description |
|---|---|
| test/EFCore.PG.FunctionalTests/Query/Translations/NodaTime/LocalDateTranslationsTest.cs | Adds period regression coverage and updates SQL assertions. |
| src/EFCore.PG/Query/Internal/NpgsqlSqlTranslatingExpressionVisitor.cs | Translates LocalDate subtraction using age() with explicit timestamp casts. |
Review effort: Balanced
This was referenced Oct 25, 2026
Closed
Correct negative PostgreSQL date/time years to NodaTime astronomical numbering without changing Period.Years or LocalDate subtraction. Partially addresses #4089. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
roji
changed the title
Use trunc(year - 0.5) + 1 to adjust BCE years with a single date_part call, and update SQL baselines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
LocalDate - LocalDatecurrently constructs an interval containing only elapsed days, losing the calendar years and months returned by NodaTime. Translate it to PostgreSQLage()instead, casting both operands totimestamp without time zoneto avoid time zone-dependent arithmetic.This changes observable period components: December 31 to January 31 now returns
P1Mrather thanP31D. PostgreSQL's month-end semantics still differ from NodaTime's; for example, January 31 to February 28 in a non-leap year returnsP28Drather thanP1M. This intentionally uses native PostgreSQL semantics instead of complex exact emulation.Adds coverage for calendar components, negative periods, and the accepted month-end difference, and updates existing SQL baselines. All 219 NodaTime translation tests pass.
Fixes #4089