bjchambers commented on a change in pull request #651:
URL: https://github.com/apache/arrow-rs/pull/651#discussion_r686455865



##########
File path: arrow/src/compute/kernels/temporal.rs
##########
@@ -166,8 +168,62 @@ where
     Ok(b.finish())
 }
 
+/// Add the given `time_delta` to each time in the `date_time` array.
+///
+/// # Errors
+/// If the arrays of different lengths.
+pub fn add_time<DateTime, TimeDelta>(
+    date_time: &PrimitiveArray<DateTime>,
+    time_delta: &PrimitiveArray<TimeDelta>,
+) -> Result<PrimitiveArray<DateTime>>
+where
+    DateTime: ArrowTimestampType,

Review comment:
       One potential issue there is that you can't necessarily add months to a 
`Time32`, since the latter doesn't have a date component. It would be possible 
to express that as a trait, but it would be more complex. For instance:
   
   ```
   trait ArrowTimeDeltaType<T: ArrowTemporalType> {
     fn add_time(temporal: T::Native, delta: Self::Native) -> T::Native;
   }
   
   impl ArrowTimeDeltaType<Date32> for IntervalYearMonthType {
     ...
   }
   
   ...
   ```
   
   Specifically, instead of having the trait allow adding the time delta to 
*anything* that is a `DateTime`, it could have an additional parameter 
indicating what it can be added to. This would require more implementation 
blocks, would express what kinds of arithmetic were supported.
   
   Eg., we could have `impl ArrowTimeDeltaType<Time32> for DurationSecondType { 
... }` but we wouldn't have one for `IntervalYearMonthType`.
   
   Does that level of encoding the supported operations in the trait make 
sense? The alternative would be to have fewer limitations at the type level and 
instead make it a runtime error to pass an invalid time type. In that world, 
the delta could have two functions -- one for adding to a `DateTime` and one 
for adding to a `Time`, and could choose to throw an exception if it wasn't 
supported on `Time`.




-- 
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]


Reply via email to