lidavidm commented on a change in pull request #12139:
URL: https://github.com/apache/arrow/pull/12139#discussion_r794561816
##########
File path: cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc
##########
@@ -519,6 +521,16 @@ std::shared_ptr<CastFunction> GetDurationCast() {
// Between durations
AddCrossUnitCast<DurationType>(func.get());
+ // time32/64 -> duration
Review comment:
IMO, it doesn't make sense to cast time to duration.
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -2437,6 +2458,30 @@ void RegisterScalarArithmetic(FunctionRegistry*
registry) {
std::move(exec)));
}
+ // Add subtract(duration, duration) -> duration
+ for (auto unit : TimeUnit::values()) {
+ InputType in_type(match::DurationTypeUnit(unit));
+ auto exec = ArithmeticExecFromOp<ScalarBinaryEqualTypes,
Subtract>(Type::DURATION);
+ DCHECK_OK(subtract->AddKernel({in_type, in_type}, duration(unit),
std::move(exec)));
+ }
+
+ // Add subtract(time32, duration) -> duration
Review comment:
This doesn't match the PR title, and time makes more sense than duration
as a result right?
Also, in that case, we need to validate that the result is not <0 or >=24h
(see #12014).
--
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]
