alexandreyc commented on code in PR #4546:
URL: https://github.com/apache/arrow-rs/pull/4546#discussion_r1272311550
##########
arrow-array/src/types.rs:
##########
@@ -454,25 +469,13 @@ impl TimestampSecondType {
/// * `timestamp` - The date on which to perform the operation
/// * `delta` - The interval to add
pub fn subtract_day_time(
- timestamp: <TimestampSecondType as ArrowPrimitiveType>::Native,
+ timestamp: <Self as ArrowPrimitiveType>::Native,
delta: <IntervalDayTimeType as ArrowPrimitiveType>::Native,
- ) -> Result<<TimestampSecondType as ArrowPrimitiveType>::Native,
ArrowError> {
+ tz: Tz,
+ ) -> Result<<Self as ArrowPrimitiveType>::Native, ArrowError> {
let (days, ms) = IntervalDayTimeType::to_parts(delta);
- let res = NaiveDateTime::from_timestamp_opt(timestamp,
0).ok_or_else(|| {
- ArrowError::ComputeError("Timestamp out of range".to_string())
- })?;
- let res = res
- .checked_sub_signed(Duration::days(days as i64))
- .ok_or_else(|| {
- ArrowError::ComputeError("Timestamp out of range".to_string())
- })?;
- let res = res
- .checked_sub_signed(Duration::milliseconds(ms as i64))
- .ok_or_else(|| {
- ArrowError::ComputeError("Timestamp out of range".to_string())
- })?;
- TimestampSecondType::make_value(res)
- .ok_or_else(|| ArrowError::ComputeError("Timestamp out of
range".to_string()))
+ let delta = IntervalDayTimeType::make_value(-days, -ms);
Review Comment:
I think that ultimately we cannot avoid a checked negation because chrono's
`Months::new` accepts only an `u32` and arrow's months are `i32`. Same thing
for days.
I've added checked negation everywhere it's needed.
I may have missed a simple solution without checked ops... Feel free to
suggest any idea!
--
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]
