Repository navigation
fix(arrow-cast): truncate Timestamp to Date64 casts to whole days - #11632
Merged
Jefffrey merged 1 commit intoOct 18, 2026
Merged
Conversation
Jefffrey
approved these changes
Oct 18, 2026
Jefffrey
approved these changes
Oct 18, 2026
This was referenced Oct 23, 2026
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.
Which issue does this PR close?
Rationale for this change
Timestamp -> Date64kept the time of day, so the result could hold values that are not multiples of86_400_000, which theDate64docs say are invalid.Timestamp -> Date32already truncates to the date, so the two casts disagreed.What changes are included in this PR?
The four
Timestamp -> Date64arms now go through a newtimestamp_to_date64helper next totimestamp_to_date32, using the same rules:i64::MINmilliseconds to a day overflows, so that case is null for a safe cast and an error otherwise, the same as the existing seconds overflow.-05:00,1970-01-01T00:00:01Zbecomes1969-12-31. In other words,cast(ts, Date64)now equalscast(cast(ts, Date32), Date64). I went with this because it matchesDate32; if you'd prefer flooring the UTC value for zoned timestamps instead, that's an easy change.Are these changes tested?
Yes.
test_cast_timestamp_to_date64now expects whole days, and a newtest_cast_timestamp_to_date64_whole_dayscovers all four units (including a timestamp just before the epoch), the timezone case checked against theDate32path, and the overflow case.Are there any significant user-facing changes?
Yes, the cast returns different values than before: any
Timestamp -> Date64cast that used to keep a time of day now returns midnight, and zoned timestamps return the local date. Values that were outside chrono's date range in a zoned array now become null (or an error for a non-safe cast), as they already do forDate32.AI disclosure: I used an AI coding assistant to write the change, tests, and this description. I reviewed the diff and the test expectations.