From eb673a49c0cd41165c1e98e6d113c229493c6152 Mon Sep 17 00:00:00 2001 From: Rok Date: Sat, 8 Jan 2022 18:20:32 +0100 Subject: [PATCH 1/6] Subtract for time32/64. --- .../compute/kernels/scalar_arithmetic.cc | 17 ++++++ .../compute/kernels/scalar_temporal_test.cc | 52 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index b8dae54b67a..182ea7ba74b 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1515,6 +1515,10 @@ ArrayKernelExec ArithmeticExecFromOp(detail::GetTypeId get_id) { case Type::INT64: case Type::TIMESTAMP: return KernelGenerator::Exec; + case Type::TIME32: + return KernelGenerator::Exec; + case Type::TIME64: + return KernelGenerator::Exec; case Type::UINT64: return KernelGenerator::Exec; case Type::FLOAT: @@ -2479,6 +2483,19 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto exec = ScalarBinary::Exec; DCHECK_OK(subtract->AddKernel({in_type, duration(unit)}, OutputType(FirstType), std::move(exec))); + + // Add subtract(time32, time32) -> time32 + for (auto unit : {TimeUnit::SECOND, TimeUnit::MILLI}) { + InputType in_type(match::Time32TypeUnit(unit)); + auto exec = ArithmeticExecFromOp(Type::TIME32); + DCHECK_OK(subtract->AddKernel({in_type, in_type}, time32(unit), std::move(exec))); + } + + // Add subtract(time64, time64) -> time64 + for (auto unit : {TimeUnit::MICRO, TimeUnit::NANO}) { + InputType in_type(match::Time64TypeUnit(unit)); + auto exec = ArithmeticExecFromOp(Type::TIME64); + DCHECK_OK(subtract->AddKernel({in_type, in_type}, time64(unit), std::move(exec))); } // Add subtract(date32, date32) -> duration(TimeUnit::SECOND) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 67a5a8b57f0..f670ced7328 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1117,6 +1117,58 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractTimestamp) { } } +TEST_F(ScalarTemporalTest, TestTemporalSubtractTime) { + std::string op = "subtract"; + auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); + auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); + auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); + auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); + auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); + auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); + auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); + auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); + + CheckScalarBinary(op, arr_s2, arr_s, + ArrayFromJSON(time32(TimeUnit::SECOND), seconds_between_time)); + CheckScalarBinary(op, arr_ms2, arr_ms, + ArrayFromJSON(time32(TimeUnit::MILLI), milliseconds_between_time)); + CheckScalarBinary(op, arr_us2, arr_us, + ArrayFromJSON(time64(TimeUnit::MICRO), microseconds_between_time)); + CheckScalarBinary(op, arr_ns2, arr_ns, + ArrayFromJSON(time64(TimeUnit::NANO), nanoseconds_between_time)); + + CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(time32(TimeUnit::SECOND), zeros)); + CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(time32(TimeUnit::MILLI), zeros)); + CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(time64(TimeUnit::MICRO), zeros)); + CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(time64(TimeUnit::NANO), zeros)); +} + +TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { + std::string op = "subtract_checked"; + auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); + auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); + auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); + auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); + auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); + auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); + auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); + auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); + + CheckScalarBinary(op, arr_s2, arr_s, + ArrayFromJSON(time32(TimeUnit::SECOND), seconds_between_time)); + CheckScalarBinary(op, arr_ms2, arr_ms, + ArrayFromJSON(time32(TimeUnit::MILLI), milliseconds_between_time)); + CheckScalarBinary(op, arr_us2, arr_us, + ArrayFromJSON(time64(TimeUnit::MICRO), microseconds_between_time)); + CheckScalarBinary(op, arr_ns2, arr_ns, + ArrayFromJSON(time64(TimeUnit::NANO), nanoseconds_between_time)); + + CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(time32(TimeUnit::SECOND), zeros)); + CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(time32(TimeUnit::MILLI), zeros)); + CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(time64(TimeUnit::MICRO), zeros)); + CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(time64(TimeUnit::NANO), zeros)); +} + TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) { auto raw_days = ArrayFromJSON(timestamp(TimeUnit::SECOND), R"([ "2021-08-09", "2021-08-10", "2021-08-11", "2021-08-12", "2021-08-13", "2021-08-14", "2021-08-15", From c5481342b3ac8a607baa56aa2006235d39ed143d Mon Sep 17 00:00:00 2001 From: Rok Date: Tue, 11 Jan 2022 12:33:57 +0100 Subject: [PATCH 2/6] Review feedback. --- .../arrow/compute/kernels/scalar_arithmetic.cc | 16 ++++++---------- .../compute/kernels/scalar_temporal_test.cc | 16 ++++++++-------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 182ea7ba74b..e117aa56c92 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1515,10 +1515,6 @@ ArrayKernelExec ArithmeticExecFromOp(detail::GetTypeId get_id) { case Type::INT64: case Type::TIMESTAMP: return KernelGenerator::Exec; - case Type::TIME32: - return KernelGenerator::Exec; - case Type::TIME64: - return KernelGenerator::Exec; case Type::UINT64: return KernelGenerator::Exec; case Type::FLOAT: @@ -2484,18 +2480,18 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { DCHECK_OK(subtract->AddKernel({in_type, duration(unit)}, OutputType(FirstType), std::move(exec))); - // Add subtract(time32, time32) -> time32 + // Add subtract(time32, time32) -> duration for (auto unit : {TimeUnit::SECOND, TimeUnit::MILLI}) { InputType in_type(match::Time32TypeUnit(unit)); - auto exec = ArithmeticExecFromOp(Type::TIME32); - DCHECK_OK(subtract->AddKernel({in_type, in_type}, time32(unit), std::move(exec))); + auto exec = ScalarBinaryEqualTypes::Exec; + DCHECK_OK(subtract->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); } - // Add subtract(time64, time64) -> time64 + // Add subtract(time64, time64) -> duration for (auto unit : {TimeUnit::MICRO, TimeUnit::NANO}) { InputType in_type(match::Time64TypeUnit(unit)); - auto exec = ArithmeticExecFromOp(Type::TIME64); - DCHECK_OK(subtract->AddKernel({in_type, in_type}, time64(unit), std::move(exec))); + auto exec = ScalarBinaryEqualTypes::Exec; + DCHECK_OK(subtract->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); } // Add subtract(date32, date32) -> duration(TimeUnit::SECOND) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index f670ced7328..0847374fa41 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1155,18 +1155,18 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); CheckScalarBinary(op, arr_s2, arr_s, - ArrayFromJSON(time32(TimeUnit::SECOND), seconds_between_time)); + ArrayFromJSON(duration(TimeUnit::SECOND), seconds_between_time)); CheckScalarBinary(op, arr_ms2, arr_ms, - ArrayFromJSON(time32(TimeUnit::MILLI), milliseconds_between_time)); + ArrayFromJSON(duration(TimeUnit::MILLI), milliseconds_between_time)); CheckScalarBinary(op, arr_us2, arr_us, - ArrayFromJSON(time64(TimeUnit::MICRO), microseconds_between_time)); + ArrayFromJSON(duration(TimeUnit::MICRO), microseconds_between_time)); CheckScalarBinary(op, arr_ns2, arr_ns, - ArrayFromJSON(time64(TimeUnit::NANO), nanoseconds_between_time)); + ArrayFromJSON(duration(TimeUnit::NANO), nanoseconds_between_time)); - CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(time32(TimeUnit::SECOND), zeros)); - CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(time32(TimeUnit::MILLI), zeros)); - CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(time64(TimeUnit::MICRO), zeros)); - CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(time64(TimeUnit::NANO), zeros)); + CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(duration(TimeUnit::SECOND), zeros)); + CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); + CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); + CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); } TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) { From b4cbf48abdf41edb7623f129b24ff0366925c5d0 Mon Sep 17 00:00:00 2001 From: Rok Date: Tue, 18 Jan 2022 18:40:31 +0100 Subject: [PATCH 3/6] Review feedback. --- .../compute/kernels/scalar_arithmetic.cc | 14 ++++++++++ .../compute/kernels/scalar_temporal_test.cc | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index e117aa56c92..147f614f56f 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -2546,6 +2546,20 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { duration(TimeUnit::MILLI), std::move(exec_date_64_checked))); + // Add subtract_checked(time32, time32) -> duration + for (auto unit : {TimeUnit::SECOND, TimeUnit::MILLI}) { + InputType in_type(match::Time32TypeUnit(unit)); + auto exec = ScalarBinaryEqualTypes::Exec; + DCHECK_OK(subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); + } + + // Add subtract_checked(time64, time64) -> duration + for (auto unit : {TimeUnit::MICRO, TimeUnit::NANO}) { + InputType in_type(match::Time64TypeUnit(unit)); + auto exec = ScalarBinaryEqualTypes::Exec; + DCHECK_OK(subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); + } + DCHECK_OK(registry->AddFunction(std::move(subtract_checked))); // ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 0847374fa41..3cfc78e7342 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1169,6 +1169,32 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); } +TEST_F(ScalarTemporalTest, TestTemporalSubtractChecked) { + std::string op = "subtract_checked"; + auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); + auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); + auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); + auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); + auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); + auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); + auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); + auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); + + CheckScalarBinary(op, arr_s2, arr_s, + ArrayFromJSON(duration(TimeUnit::SECOND), seconds_between_time)); + CheckScalarBinary(op, arr_ms2, arr_ms, + ArrayFromJSON(duration(TimeUnit::MILLI), milliseconds_between_time)); + CheckScalarBinary(op, arr_us2, arr_us, + ArrayFromJSON(duration(TimeUnit::MICRO), microseconds_between_time)); + CheckScalarBinary(op, arr_ns2, arr_ns, + ArrayFromJSON(duration(TimeUnit::NANO), nanoseconds_between_time)); + + CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(duration(TimeUnit::SECOND), zeros)); + CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); + CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); + CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); +} + TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) { auto raw_days = ArrayFromJSON(timestamp(TimeUnit::SECOND), R"([ "2021-08-09", "2021-08-10", "2021-08-11", "2021-08-12", "2021-08-13", "2021-08-14", "2021-08-15", From a6e5e6480cf9991e20ee3c86d627c7ee9893a0dc Mon Sep 17 00:00:00 2001 From: Rok Date: Wed, 19 Jan 2022 13:27:40 +0100 Subject: [PATCH 4/6] Add ReplaceTemporalTypes --- .../compute/kernels/scalar_arithmetic.cc | 24 +++++++++- .../compute/kernels/scalar_cast_temporal.cc | 12 ++++- .../compute/kernels/scalar_temporal_test.cc | 44 +++++++------------ 3 files changed, 49 insertions(+), 31 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 147f614f56f..774efbc30ed 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1513,6 +1513,7 @@ ArrayKernelExec ArithmeticExecFromOp(detail::GetTypeId get_id) { case Type::UINT32: return KernelGenerator::Exec; case Type::INT64: + case Type::DURATION: case Type::TIMESTAMP: return KernelGenerator::Exec; case Type::UINT64: @@ -2473,12 +2474,20 @@ 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(Type::DURATION); + DCHECK_OK(subtract->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); + } + // Add subtract(timestamp, duration) -> timestamp for (auto unit : TimeUnit::values()) { InputType in_type(match::TimestampTypeUnit(unit)); auto exec = ScalarBinary::Exec; DCHECK_OK(subtract->AddKernel({in_type, duration(unit)}, OutputType(FirstType), std::move(exec))); + } // Add subtract(time32, time32) -> duration for (auto unit : {TimeUnit::SECOND, TimeUnit::MILLI}) { @@ -2523,6 +2532,15 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { std::move(exec))); } + // Add subtract_checked(duration, duration) -> duration + for (auto unit : TimeUnit::values()) { + InputType in_type(match::DurationTypeUnit(unit)); + auto exec = + ArithmeticExecFromOp(Type::DURATION); + DCHECK_OK( + subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); + } + // Add subtract_checked(timestamp, duration) -> timestamp for (auto unit : TimeUnit::values()) { InputType in_type(match::TimestampTypeUnit(unit)); @@ -2550,14 +2568,16 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { for (auto unit : {TimeUnit::SECOND, TimeUnit::MILLI}) { InputType in_type(match::Time32TypeUnit(unit)); auto exec = ScalarBinaryEqualTypes::Exec; - DCHECK_OK(subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); + DCHECK_OK( + subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); } // Add subtract_checked(time64, time64) -> duration for (auto unit : {TimeUnit::MICRO, TimeUnit::NANO}) { InputType in_type(match::Time64TypeUnit(unit)); auto exec = ScalarBinaryEqualTypes::Exec; - DCHECK_OK(subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); + DCHECK_OK( + subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); } DCHECK_OK(registry->AddFunction(std::move(subtract_checked))); diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc b/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc index 74274e963a1..5e6983ffd88 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc @@ -338,10 +338,12 @@ struct CastFunctor { }; // ---------------------------------------------------------------------- -// From one time32 or time64 to another +// From one time32 or time64 to another or to duration template -struct CastFunctor::value && is_time_type::value>> { +struct CastFunctor::value && is_time_type::value) || + (is_time_type::value && is_duration_type::value)>> { using in_t = typename I::c_type; using out_t = typename O::c_type; @@ -519,6 +521,12 @@ std::shared_ptr GetDurationCast() { // Between durations AddCrossUnitCast(func.get()); + // time32/64 -> duration + AddSimpleCast(InputType(Type::TIME64), kOutputTargetType, + func.get()); + AddSimpleCast(InputType(Type::TIME32), kOutputTargetType, + func.get()); + return func; } diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 3cfc78e7342..a2785219a14 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1128,32 +1128,6 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractTime) { auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); - CheckScalarBinary(op, arr_s2, arr_s, - ArrayFromJSON(time32(TimeUnit::SECOND), seconds_between_time)); - CheckScalarBinary(op, arr_ms2, arr_ms, - ArrayFromJSON(time32(TimeUnit::MILLI), milliseconds_between_time)); - CheckScalarBinary(op, arr_us2, arr_us, - ArrayFromJSON(time64(TimeUnit::MICRO), microseconds_between_time)); - CheckScalarBinary(op, arr_ns2, arr_ns, - ArrayFromJSON(time64(TimeUnit::NANO), nanoseconds_between_time)); - - CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(time32(TimeUnit::SECOND), zeros)); - CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(time32(TimeUnit::MILLI), zeros)); - CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(time64(TimeUnit::MICRO), zeros)); - CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(time64(TimeUnit::NANO), zeros)); -} - -TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { - std::string op = "subtract_checked"; - auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); - auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); - auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); - auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); - auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); - auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); - auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); - auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); - CheckScalarBinary(op, arr_s2, arr_s, ArrayFromJSON(duration(TimeUnit::SECOND), seconds_between_time)); CheckScalarBinary(op, arr_ms2, arr_ms, @@ -1167,9 +1141,17 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); + + auto seconds_3 = ArrayFromJSON(time32(TimeUnit::SECOND), R"([3, null])"); + auto milliseconds_2k = ArrayFromJSON(time32(TimeUnit::MILLI), R"([1999, null])"); + auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1001, null])"); + auto microseconds_2M = ArrayFromJSON(time64(TimeUnit::MICRO), R"([1999999, null])"); + auto microseconds_1M = ArrayFromJSON(duration(TimeUnit::MICRO), R"([1000001, null])"); + CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); + CheckScalarBinary(op, seconds_3, microseconds_2M, microseconds_1M); } -TEST_F(ScalarTemporalTest, TestTemporalSubtractChecked) { +TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { std::string op = "subtract_checked"; auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); @@ -1193,6 +1175,14 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractChecked) { CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); + + auto seconds_3 = ArrayFromJSON(time32(TimeUnit::SECOND), R"([3, null])"); + auto milliseconds_2k = ArrayFromJSON(time32(TimeUnit::MILLI), R"([1999, null])"); + auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1001, null])"); + auto microseconds_2M = ArrayFromJSON(time64(TimeUnit::MICRO), R"([1999999, null])"); + auto microseconds_1M = ArrayFromJSON(duration(TimeUnit::MICRO), R"([1000001, null])"); + CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); + CheckScalarBinary(op, seconds_3, microseconds_2M, microseconds_1M); } TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) { From 1ae8bd528fd7ae45694215bc56f06f11de694258 Mon Sep 17 00:00:00 2001 From: Rok Date: Tue, 1 Feb 2022 13:03:19 +0100 Subject: [PATCH 5/6] Fixing casting issue --- cpp/src/arrow/compute/kernels/codegen_internal.cc | 9 ++++++++- cpp/src/arrow/compute/kernels/codegen_internal_test.cc | 8 ++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.cc b/cpp/src/arrow/compute/kernels/codegen_internal.cc index 1e06a364509..26c12ebf591 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.cc +++ b/cpp/src/arrow/compute/kernels/codegen_internal.cc @@ -120,7 +120,14 @@ void ReplaceTemporalTypes(const TimeUnit::type unit, std::vector* de continue; } case Type::TIME32: - case Type::TIME64: + case Type::TIME64: { + if (unit > TimeUnit::MILLI) { + it->type = time64(unit); + } else { + it->type = time32(unit); + } + continue; + } case Type::DURATION: { it->type = duration(unit); continue; diff --git a/cpp/src/arrow/compute/kernels/codegen_internal_test.cc b/cpp/src/arrow/compute/kernels/codegen_internal_test.cc index 6b68632ceb3..c2e17e377f2 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal_test.cc +++ b/cpp/src/arrow/compute/kernels/codegen_internal_test.cc @@ -198,6 +198,10 @@ TEST(TestDispatchBest, CommonTemporalResolution) { ASSERT_EQ(TimeUnit::MILLI, CommonTemporalResolution(args.data(), args.size())); args = {timestamp(TimeUnit::SECOND, "UTC"), timestamp(TimeUnit::SECOND, tz)}; ASSERT_EQ(TimeUnit::SECOND, CommonTemporalResolution(args.data(), args.size())); + args = {time32(TimeUnit::MILLI), duration(TimeUnit::SECOND)}; + ASSERT_EQ(TimeUnit::MILLI, CommonTemporalResolution(args.data(), args.size())); + args = {time64(TimeUnit::MICRO), duration(TimeUnit::NANO)}; + ASSERT_EQ(TimeUnit::NANO, CommonTemporalResolution(args.data(), args.size())); } TEST(TestDispatchBest, ReplaceTemporalTypes) { @@ -215,7 +219,7 @@ TEST(TestDispatchBest, ReplaceTemporalTypes) { ty = CommonTemporalResolution(args.data(), args.size()); ReplaceTemporalTypes(ty, &args); AssertTypeEqual(args[0].type, timestamp(TimeUnit::MILLI)); - AssertTypeEqual(args[1].type, duration(TimeUnit::MILLI)); + AssertTypeEqual(args[1].type, time32(TimeUnit::MILLI)); args = {duration(TimeUnit::SECOND), date64()}; ty = CommonTemporalResolution(args.data(), args.size()); @@ -233,7 +237,7 @@ TEST(TestDispatchBest, ReplaceTemporalTypes) { ty = CommonTemporalResolution(args.data(), args.size()); ReplaceTemporalTypes(ty, &args); AssertTypeEqual(args[0].type, timestamp(TimeUnit::NANO, tz)); - AssertTypeEqual(args[1].type, duration(TimeUnit::NANO)); + AssertTypeEqual(args[1].type, time64(TimeUnit::NANO)); args = {timestamp(TimeUnit::SECOND, tz), date64()}; ty = CommonTemporalResolution(args.data(), args.size()); From 06a54e79aa74ce7e528d99e112e3f76b9a7d0c69 Mon Sep 17 00:00:00 2001 From: Rok Date: Tue, 1 Feb 2022 14:54:32 +0100 Subject: [PATCH 6/6] Review feedback --- .../compute/kernels/scalar_arithmetic.cc | 17 --- .../compute/kernels/scalar_cast_temporal.cc | 12 +-- .../compute/kernels/scalar_temporal_test.cc | 101 +++++++----------- 3 files changed, 38 insertions(+), 92 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 774efbc30ed..dd0476584fc 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1513,7 +1513,6 @@ ArrayKernelExec ArithmeticExecFromOp(detail::GetTypeId get_id) { case Type::UINT32: return KernelGenerator::Exec; case Type::INT64: - case Type::DURATION: case Type::TIMESTAMP: return KernelGenerator::Exec; case Type::UINT64: @@ -2474,13 +2473,6 @@ 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(Type::DURATION); - DCHECK_OK(subtract->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); - } - // Add subtract(timestamp, duration) -> timestamp for (auto unit : TimeUnit::values()) { InputType in_type(match::TimestampTypeUnit(unit)); @@ -2532,15 +2524,6 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { std::move(exec))); } - // Add subtract_checked(duration, duration) -> duration - for (auto unit : TimeUnit::values()) { - InputType in_type(match::DurationTypeUnit(unit)); - auto exec = - ArithmeticExecFromOp(Type::DURATION); - DCHECK_OK( - subtract_checked->AddKernel({in_type, in_type}, duration(unit), std::move(exec))); - } - // Add subtract_checked(timestamp, duration) -> timestamp for (auto unit : TimeUnit::values()) { InputType in_type(match::TimestampTypeUnit(unit)); diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc b/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc index 5e6983ffd88..74274e963a1 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc @@ -338,12 +338,10 @@ struct CastFunctor { }; // ---------------------------------------------------------------------- -// From one time32 or time64 to another or to duration +// From one time32 or time64 to another template -struct CastFunctor::value && is_time_type::value) || - (is_time_type::value && is_duration_type::value)>> { +struct CastFunctor::value && is_time_type::value>> { using in_t = typename I::c_type; using out_t = typename O::c_type; @@ -521,12 +519,6 @@ std::shared_ptr GetDurationCast() { // Between durations AddCrossUnitCast(func.get()); - // time32/64 -> duration - AddSimpleCast(InputType(Type::TIME64), kOutputTargetType, - func.get()); - AddSimpleCast(InputType(Type::TIME32), kOutputTargetType, - func.get()); - return func; } diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index a2785219a14..7b7a31422bf 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1118,71 +1118,42 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractTimestamp) { } TEST_F(ScalarTemporalTest, TestTemporalSubtractTime) { - std::string op = "subtract"; - auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); - auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); - auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); - auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); - auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); - auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); - auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); - auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); - - CheckScalarBinary(op, arr_s2, arr_s, - ArrayFromJSON(duration(TimeUnit::SECOND), seconds_between_time)); - CheckScalarBinary(op, arr_ms2, arr_ms, - ArrayFromJSON(duration(TimeUnit::MILLI), milliseconds_between_time)); - CheckScalarBinary(op, arr_us2, arr_us, - ArrayFromJSON(duration(TimeUnit::MICRO), microseconds_between_time)); - CheckScalarBinary(op, arr_ns2, arr_ns, - ArrayFromJSON(duration(TimeUnit::NANO), nanoseconds_between_time)); - - CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(duration(TimeUnit::SECOND), zeros)); - CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); - CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); - CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); - - auto seconds_3 = ArrayFromJSON(time32(TimeUnit::SECOND), R"([3, null])"); - auto milliseconds_2k = ArrayFromJSON(time32(TimeUnit::MILLI), R"([1999, null])"); - auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1001, null])"); - auto microseconds_2M = ArrayFromJSON(time64(TimeUnit::MICRO), R"([1999999, null])"); - auto microseconds_1M = ArrayFromJSON(duration(TimeUnit::MICRO), R"([1000001, null])"); - CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); - CheckScalarBinary(op, seconds_3, microseconds_2M, microseconds_1M); -} - -TEST_F(ScalarTemporalTest, TestTemporalSubtractCheckedTime) { - std::string op = "subtract_checked"; - auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); - auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); - auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); - auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); - auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); - auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); - auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); - auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); - - CheckScalarBinary(op, arr_s2, arr_s, - ArrayFromJSON(duration(TimeUnit::SECOND), seconds_between_time)); - CheckScalarBinary(op, arr_ms2, arr_ms, - ArrayFromJSON(duration(TimeUnit::MILLI), milliseconds_between_time)); - CheckScalarBinary(op, arr_us2, arr_us, - ArrayFromJSON(duration(TimeUnit::MICRO), microseconds_between_time)); - CheckScalarBinary(op, arr_ns2, arr_ns, - ArrayFromJSON(duration(TimeUnit::NANO), nanoseconds_between_time)); - - CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(duration(TimeUnit::SECOND), zeros)); - CheckScalarBinary(op, arr_ms, arr_ms, ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); - CheckScalarBinary(op, arr_us, arr_us, ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); - CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); - - auto seconds_3 = ArrayFromJSON(time32(TimeUnit::SECOND), R"([3, null])"); - auto milliseconds_2k = ArrayFromJSON(time32(TimeUnit::MILLI), R"([1999, null])"); - auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1001, null])"); - auto microseconds_2M = ArrayFromJSON(time64(TimeUnit::MICRO), R"([1999999, null])"); - auto microseconds_1M = ArrayFromJSON(duration(TimeUnit::MICRO), R"([1000001, null])"); - CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); - CheckScalarBinary(op, seconds_3, microseconds_2M, microseconds_1M); + for (auto op : {"subtract", "subtract_checked"}) { + auto arr_s = ArrayFromJSON(time32(TimeUnit::SECOND), times_s); + auto arr_s2 = ArrayFromJSON(time32(TimeUnit::SECOND), times_s2); + auto arr_ms = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms); + auto arr_ms2 = ArrayFromJSON(time32(TimeUnit::MILLI), times_ms2); + auto arr_us = ArrayFromJSON(time64(TimeUnit::MICRO), times_us); + auto arr_us2 = ArrayFromJSON(time64(TimeUnit::MICRO), times_us2); + auto arr_ns = ArrayFromJSON(time64(TimeUnit::NANO), times_ns); + auto arr_ns2 = ArrayFromJSON(time64(TimeUnit::NANO), times_ns2); + + CheckScalarBinary(op, arr_s2, arr_s, + ArrayFromJSON(duration(TimeUnit::SECOND), seconds_between_time)); + CheckScalarBinary( + op, arr_ms2, arr_ms, + ArrayFromJSON(duration(TimeUnit::MILLI), milliseconds_between_time)); + CheckScalarBinary( + op, arr_us2, arr_us, + ArrayFromJSON(duration(TimeUnit::MICRO), microseconds_between_time)); + CheckScalarBinary(op, arr_ns2, arr_ns, + ArrayFromJSON(duration(TimeUnit::NANO), nanoseconds_between_time)); + + CheckScalarBinary(op, arr_s, arr_s, ArrayFromJSON(duration(TimeUnit::SECOND), zeros)); + CheckScalarBinary(op, arr_ms, arr_ms, + ArrayFromJSON(duration(TimeUnit::MILLI), zeros)); + CheckScalarBinary(op, arr_us, arr_us, + ArrayFromJSON(duration(TimeUnit::MICRO), zeros)); + CheckScalarBinary(op, arr_ns, arr_ns, ArrayFromJSON(duration(TimeUnit::NANO), zeros)); + + auto seconds_3 = ArrayFromJSON(time32(TimeUnit::SECOND), R"([3, null])"); + auto milliseconds_2k = ArrayFromJSON(time32(TimeUnit::MILLI), R"([1999, null])"); + auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1001, null])"); + auto microseconds_2M = ArrayFromJSON(time64(TimeUnit::MICRO), R"([1999999, null])"); + auto microseconds_1M = ArrayFromJSON(duration(TimeUnit::MICRO), R"([1000001, null])"); + CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); + CheckScalarBinary(op, seconds_3, microseconds_2M, microseconds_1M); + } } TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) {