From f38de691551993b981822eb666f063997d64bf53 Mon Sep 17 00:00:00 2001 From: Rok Date: Thu, 13 Jan 2022 14:29:54 +0100 Subject: [PATCH 1/3] Subtract duration --- .../arrow/compute/kernels/scalar_arithmetic.cc | 6 ++++++ .../compute/kernels/scalar_temporal_test.cc | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 249c5ad92f4..49c873af97f 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -2658,6 +2658,12 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto exec = ScalarBinary::Exec; DCHECK_OK(subtract->AddKernel({in_type, duration(unit)}, OutputType(FirstType), 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(time32, time32) -> duration diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 7efb9ce88bb..7cef37672c8 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1429,6 +1429,23 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractTimeAndDuration) { } } +TEST_F(ScalarTemporalTest, TestTemporalSubtractDuration) { + std::string op = "subtract"; + + for (auto u : TimeUnit::values()) { + auto unit = duration(u); + CheckScalarBinary(op, + ArrayFromJSON(unit, times_s2), + ArrayFromJSON(unit, times_s), + ArrayFromJSON(unit, seconds_between_time)); + } + + EXPECT_RAISES_WITH_MESSAGE_THAT( + NotImplemented, testing::HasSubstr("no kernel matching input types"), + Subtract(ArrayFromJSON(duration(TimeUnit::SECOND), times_s), + ArrayFromJSON(duration(TimeUnit::MILLI), times_s))); +} + 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 eaa5c630ed394055547e11eed48be4ea8fe1a346 Mon Sep 17 00:00:00 2001 From: Rok Date: Wed, 19 Jan 2022 12:35:32 +0100 Subject: [PATCH 2/3] Add ReplaceTemporalTypes --- .../compute/kernels/scalar_arithmetic.cc | 10 +++ .../compute/kernels/scalar_temporal_test.cc | 22 +++---- docs/source/cpp/compute.rst | 66 +++++++++---------- 3 files changed, 53 insertions(+), 45 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 49c873af97f..29133a576d0 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -2658,6 +2658,7 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto exec = ScalarBinary::Exec; DCHECK_OK(subtract->AddKernel({in_type, duration(unit)}, OutputType(FirstType), std::move(exec))); + } // Add subtract(duration, duration) -> duration for (auto unit : TimeUnit::values()) { @@ -2720,6 +2721,15 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { OutputType(FirstType), 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(date32, date32) -> duration(TimeUnit::SECOND) auto exec_date_32_checked = ScalarBinaryEqualTypes::Exec; diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 7cef37672c8..ce557b1a29c 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1430,20 +1430,18 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractTimeAndDuration) { } TEST_F(ScalarTemporalTest, TestTemporalSubtractDuration) { - std::string op = "subtract"; + for (auto op : {"subtract", "subtract_checked"}) { + for (auto u : TimeUnit::values()) { + auto unit = duration(u); + CheckScalarBinary(op, ArrayFromJSON(unit, times_s2), ArrayFromJSON(unit, times_s), + ArrayFromJSON(unit, seconds_between_time)); + } - for (auto u : TimeUnit::values()) { - auto unit = duration(u); - CheckScalarBinary(op, - ArrayFromJSON(unit, times_s2), - ArrayFromJSON(unit, times_s), - ArrayFromJSON(unit, seconds_between_time)); + auto seconds_3 = ArrayFromJSON(duration(TimeUnit::SECOND), R"([3, null])"); + auto milliseconds_2k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([2000, null])"); + auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1000, null])"); + CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); } - - EXPECT_RAISES_WITH_MESSAGE_THAT( - NotImplemented, testing::HasSubstr("no kernel matching input types"), - Subtract(ArrayFromJSON(duration(TimeUnit::SECOND), times_s), - ArrayFromJSON(duration(TimeUnit::MILLI), times_s))); } TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) { diff --git a/docs/source/cpp/compute.rst b/docs/source/cpp/compute.rst index 72b19bb4979..97f5f8c12e4 100644 --- a/docs/source/cpp/compute.rst +++ b/docs/source/cpp/compute.rst @@ -423,39 +423,39 @@ floating-point arguments will cast all arguments to floating-point, while mixed decimal and integer arguments will cast all arguments to decimals. Mixed time resolution temporal inputs will be cast to finest input resolution. -+------------------+--------+----------------------------+----------------------------+-------+ -| Function name | Arity | Input types | Output type | Notes | -+==================+========+============================+============================+=======+ -| abs | Unary | Numeric | Numeric | | -+------------------+--------+----------------------------+----------------------------+-------+ -| abs_checked | Unary | Numeric | Numeric | | -+------------------+--------+----------------------------+----------------------------+-------+ -| add | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| add_checked | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| divide | Binary | Numeric | Numeric | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| divide_checked | Binary | Numeric | Numeric | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| multiply | Binary | Numeric | Numeric | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| multiply_checked | Binary | Numeric | Numeric | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| negate | Unary | Numeric | Numeric | | -+------------------+--------+----------------------------+----------------------------+-------+ -| negate_checked | Unary | Signed Numeric | Signed Numeric | | -+------------------+--------+----------------------------+----------------------------+-------+ -| power | Binary | Numeric | Numeric | | -+------------------+--------+----------------------------+----------------------------+-------+ -| power_checked | Binary | Numeric | Numeric | | -+------------------+--------+----------------------------+----------------------------+-------+ -| sign | Unary | Numeric | Int8/Float32/Float64 | \(2) | -+------------------+--------+----------------------------+----------------------------+-------+ -| subtract | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ -| subtract_checked | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | -+------------------+--------+----------------------------+----------------------------+-------+ ++------------------+--------+------------------+----------------------+-------+ +| Function name | Arity | Input types | Output type | Notes | ++==================+========+==================+======================+=======+ +| abs | Unary | Numeric | Numeric | | ++------------------+--------+------------------+----------------------+-------+ +| abs_checked | Unary | Numeric | Numeric | | ++------------------+--------+------------------+----------------------+-------+ +| add | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| add_checked | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| divide | Binary | Numeric | Numeric | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| divide_checked | Binary | Numeric | Numeric | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| multiply | Binary | Numeric | Numeric | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| multiply_checked | Binary | Numeric | Numeric | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| negate | Unary | Numeric | Numeric | | ++------------------+--------+------------------+----------------------+-------+ +| negate_checked | Unary | Signed Numeric | Signed Numeric | | ++------------------+--------+------------------+----------------------+-------+ +| power | Binary | Numeric | Numeric | | ++------------------+--------+------------------+----------------------+-------+ +| power_checked | Binary | Numeric | Numeric | | ++------------------+--------+------------------+----------------------+-------+ +| sign | Unary | Numeric | Int8/Float32/Float64 | \(2) | ++------------------+--------+------------------+----------------------+-------+ +| subtract | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | ++------------------+--------+------------------+----------------------+-------+ +| subtract_checked | Binary | Numeric/Temporal | Numeric/Temporal | \(1) | ++------------------+--------+------------------+----------------------+-------+ * \(1) Precision and scale of computed DECIMAL results From 4f6675dac1e43032d1eec415ac4e2ed366760c72 Mon Sep 17 00:00:00 2001 From: Rok Date: Wed, 16 Feb 2022 00:53:21 +0100 Subject: [PATCH 3/3] Review feedback --- cpp/src/arrow/compute/kernels/scalar_temporal_test.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index ce557b1a29c..560699c09ed 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -1442,6 +1442,17 @@ TEST_F(ScalarTemporalTest, TestTemporalSubtractDuration) { auto milliseconds_1k = ArrayFromJSON(duration(TimeUnit::MILLI), R"([1000, null])"); CheckScalarBinary(op, seconds_3, milliseconds_2k, milliseconds_1k); } + + for (auto unit : TimeUnit::values()) { + auto duration_ty = duration(unit); + auto arr1 = ArrayFromJSON(duration_ty, R"([-9223372036854775808, null])"); + auto arr2 = ArrayFromJSON(duration_ty, R"([1, null])"); + auto arr3 = ArrayFromJSON(duration_ty, R"([9223372036854775807, null])"); + + CheckScalarBinary("subtract", arr1, arr2, arr3); + EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, ::testing::HasSubstr("overflow"), + CallFunction("subtract_checked", {arr1, arr2})); + } } TEST_F(ScalarTemporalTest, TestTemporalDifferenceWeeks) {