mbutrovich commented on code in PR #11199:
URL: https://github.com/apache/arrow-rs/pull/11199#discussion_r4107202344
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -674,6 +765,9 @@ fn timestamp_to_date32<T: ArrowTimestampType>(
/// * `Date32` and `Date64`: precision lost when going to higher interval
/// * `Time32` and `Time64`: precision lost when going to higher interval
/// * `Timestamp` and `Date{32|64}`: precision lost when going to higher
interval
+/// * `Timestamp` without a timezone to `Date32`, `Time32`, or `Time64`:
supports
+/// timestamps outside Chrono's date range. A `Date32` day count that does
not fit
+/// in `i32` returns an error, regardless of [`CastOptions::safe`].
Review Comment:
You're right, and I'll withdraw that suggestion.
[`CONTRIBUTING.md`](https://github.com/apache/arrow-rs/blob/c258274aef28810584f1ef356fe145348a075d59/CONTRIBUTING.md#L196-L201)
says an `api-change` PR waits until development opens for the next major
release, so the label would hold this PR until 61.0.0 for no reason. The "Are
there any user-facing changes?" section of the description already records the
behavior change for the changelog. I'm fine keeping the new semantics. Without
a timezone the result is plain integer arithmetic and is well defined for every
input, so tying it to Chrono's calendar range would only add errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
