diff --git a/cpp/src/arrow/compute/expression_test.cc b/cpp/src/arrow/compute/expression_test.cc index 10c1b348e62..fd2725d55b4 100644 --- a/cpp/src/arrow/compute/expression_test.cc +++ b/cpp/src/arrow/compute/expression_test.cc @@ -2379,5 +2379,55 @@ TEST(Projection, AugmentWithKnownValues) { })); } +TEST(Expression, TestIssue54668DecimalComparisonDifferentScales) { + auto test_schema = arrow::schema({ + field("a", decimal128(3, 2)), + field("b", decimal128(3, 1)), + }); + + auto batch = RecordBatchFromJSON(test_schema, R"([ + {"a": "1.00", "b": "9.0"}, + {"a": "9.00", "b": "1.0"} + ])"); + + // 1.00 < 9.0 -> true; 9.00 < 1.0 -> false + ASSERT_OK_AND_ASSIGN(auto less_expr, + less(field_ref("a"), field_ref("b")).Bind(*test_schema)); + ASSERT_OK_AND_ASSIGN(auto less_res, + ExecuteScalarExpression(less_expr, *test_schema, batch)); + AssertDatumsEqual(Datum(ArrayFromJSON(boolean(), "[true, false]")), less_res); + + // 1.00 <= 9.0 -> true; 9.00 <= 1.0 -> false + ASSERT_OK_AND_ASSIGN(auto le_expr, + less_equal(field_ref("a"), field_ref("b")).Bind(*test_schema)); + ASSERT_OK_AND_ASSIGN(auto le_res, + ExecuteScalarExpression(le_expr, *test_schema, batch)); + AssertDatumsEqual(Datum(ArrayFromJSON(boolean(), "[true, false]")), le_res); + + // Negative decimals with different scales + auto neg_batch = RecordBatchFromJSON(test_schema, R"([ + {"a": "-1.00", "b": "-9.0"}, + {"a": "-9.00", "b": "-1.0"} + ])"); + // -1.00 < -9.0 -> false; -9.00 < -1.0 -> true + ASSERT_OK_AND_ASSIGN(auto neg_less_res, + ExecuteScalarExpression(less_expr, *test_schema, neg_batch)); + AssertDatumsEqual(Datum(ArrayFromJSON(boolean(), "[false, true]")), neg_less_res); + + // Mixed string and string_view in expressions with literal on LHS (exercising expression flipping) + auto str_schema = arrow::schema({field("view_col", utf8_view())}); + auto str_batch = RecordBatchFromJSON(str_schema, R"([ + {"view_col": "apple"}, + {"view_col": "banana"}, + {"view_col": "cherry"} + ])"); + // "banana" < view_col -> [false, false, true] + ASSERT_OK_AND_ASSIGN(auto lit_less_expr, + less(literal("banana"), field_ref("view_col")).Bind(*str_schema)); + ASSERT_OK_AND_ASSIGN(auto lit_less_res, + ExecuteScalarExpression(lit_less_expr, *str_schema, str_batch)); + AssertDatumsEqual(Datum(ArrayFromJSON(boolean(), "[false, false, true]")), lit_less_res); +} + } // namespace compute } // namespace arrow diff --git a/cpp/src/arrow/compute/kernels/scalar_compare.cc b/cpp/src/arrow/compute/kernels/scalar_compare.cc index 0f2c479b082..e34bdeb5594 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare.cc @@ -352,6 +352,8 @@ struct CompareFunction : ScalarFunction { EnsureDictionaryDecoded(types); ReplaceNullWithOtherType(types); + if (auto kernel = DispatchExactImpl(this, *types)) return kernel; + if (auto type = CommonNumeric(*types)) { ReplaceTypes(type, types); } else if (auto type = CommonTemporal(types->data(), types->size())) { @@ -440,6 +442,25 @@ std::shared_ptr MakeCompareFunction(std::string name, FunctionDo DCHECK_OK(func->AddKernel({ty, ty}, boolean(), std::move(exec))); } + auto exec_base_view = + applicator::ScalarBinary::Exec; + auto exec_view_base = + applicator::ScalarBinary::Exec; + auto exec_large_view = + applicator::ScalarBinary::Exec; + auto exec_view_large = + applicator::ScalarBinary::Exec; + + DCHECK_OK(func->AddKernel({utf8(), utf8_view()}, boolean(), exec_base_view)); + DCHECK_OK(func->AddKernel({utf8_view(), utf8()}, boolean(), exec_view_base)); + DCHECK_OK(func->AddKernel({large_utf8(), utf8_view()}, boolean(), exec_large_view)); + DCHECK_OK(func->AddKernel({utf8_view(), large_utf8()}, boolean(), exec_view_large)); + + DCHECK_OK(func->AddKernel({binary(), binary_view()}, boolean(), exec_base_view)); + DCHECK_OK(func->AddKernel({binary_view(), binary()}, boolean(), exec_view_base)); + DCHECK_OK(func->AddKernel({large_binary(), binary_view()}, boolean(), exec_large_view)); + DCHECK_OK(func->AddKernel({binary_view(), large_binary()}, boolean(), exec_view_large)); + for (const auto id : {Type::DECIMAL128, Type::DECIMAL256}) { auto exec = GenerateDecimal(id); DCHECK_OK(func->AddKernel({InputType(id), InputType(id)}, boolean(), std::move(exec), @@ -478,6 +499,11 @@ std::shared_ptr MakeFlippedCompare(std::string name, std::make_shared(name, Arity::Binary(), std::move(doc)); for (const ScalarKernel* kernel : func.kernels()) { ScalarKernel flipped_kernel = *kernel; + const auto& in_types = kernel->signature->in_types(); + if (in_types[0] != in_types[1]) { + flipped_kernel.signature = KernelSignature::Make( + {in_types[1], in_types[0]}, kernel->signature->out_type()); + } if (kernel->data) { auto compare_data = checked_cast(kernel->data.get()); flipped_kernel.data = diff --git a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc index 2aae5bf2ee9..5a78eabe1f9 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc @@ -1021,6 +1021,16 @@ TEST(TestCompareKernel, DispatchBest) { CheckDispatchBest(name, {binary(), fixed_size_binary(2)}, {binary(), binary()}); CheckDispatchBest(name, {fixed_size_binary(4), fixed_size_binary(2)}, {fixed_size_binary(4), fixed_size_binary(2)}); + CheckDispatchBest(name, {utf8(), utf8_view()}, {utf8(), utf8_view()}); + CheckDispatchBest(name, {utf8_view(), utf8()}, {utf8_view(), utf8()}); + CheckDispatchBest(name, {large_utf8(), utf8_view()}, {large_utf8(), utf8_view()}); + CheckDispatchBest(name, {utf8_view(), large_utf8()}, {utf8_view(), large_utf8()}); + CheckDispatchBest(name, {binary(), binary_view()}, {binary(), binary_view()}); + CheckDispatchBest(name, {binary_view(), binary()}, {binary_view(), binary()}); + CheckDispatchBest(name, {large_binary(), binary_view()}, + {large_binary(), binary_view()}); + CheckDispatchBest(name, {binary_view(), large_binary()}, + {binary_view(), large_binary()}); CheckDispatchBest(name, {decimal128(3, 2), decimal128(6, 3)}, {decimal128(4, 3), decimal128(6, 3)}); @@ -1245,6 +1255,198 @@ TEST(TestBinaryViewCompareKernel, ArrayScalar) { } } +TEST(TestBinaryViewCompareKernel, MixedStringTypes) { + // Array - Array (short inline strings <= 12 bytes) + auto str_arr = ArrayFromJSON(utf8(), R"(["a", "b", null, "d"])"); + auto large_str_arr = ArrayFromJSON(large_utf8(), R"(["a", "b", null, "d"])"); + auto view_arr = ArrayFromJSON(utf8_view(), R"(["a", "x", "c", null])"); + + CheckScalarBinary("equal", str_arr, view_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + CheckScalarBinary("equal", view_arr, str_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + CheckScalarBinary("equal", large_str_arr, view_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + CheckScalarBinary("equal", view_arr, large_str_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + + CheckScalarBinary("not_equal", str_arr, view_arr, + ArrayFromJSON(boolean(), "[false, true, null, null]")); + CheckScalarBinary("less", str_arr, view_arr, + ArrayFromJSON(boolean(), "[false, true, null, null]")); + CheckScalarBinary("greater", str_arr, view_arr, + ArrayFromJSON(boolean(), "[false, false, null, null]")); + + // Array - Array (long strings > 12 bytes testing out-of-line prefix + buffer lookup) + auto long_str_arr = ArrayFromJSON( + utf8(), + R"(["this_is_a_long_string_1", "this_is_a_long_string_2", null, "short"])"); + auto long_view_arr = ArrayFromJSON( + utf8_view(), + R"(["this_is_a_long_string_1", "this_is_a_different_long_string", "c", null])"); + CheckScalarBinary("equal", long_str_arr, long_view_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + CheckScalarBinary("equal", long_view_arr, long_str_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + CheckScalarBinary("not_equal", long_str_arr, long_view_arr, + ArrayFromJSON(boolean(), "[false, true, null, null]")); + + // Empty arrays + auto empty_str = ArrayFromJSON(utf8(), "[]"); + auto empty_view = ArrayFromJSON(utf8_view(), "[]"); + CheckScalarBinary("equal", empty_str, empty_view, ArrayFromJSON(boolean(), "[]")); + CheckScalarBinary("equal", empty_view, empty_str, ArrayFromJSON(boolean(), "[]")); + + // All-null arrays + auto null_str = ArrayFromJSON(utf8(), "[null, null]"); + auto null_view = ArrayFromJSON(utf8_view(), "[null, null]"); + CheckScalarBinary("equal", null_str, null_view, + ArrayFromJSON(boolean(), "[null, null]")); + + // Sliced arrays + auto sliced_str = str_arr->Slice(1, 2); + auto sliced_view = view_arr->Slice(1, 2); + CheckScalarBinary("equal", sliced_str, sliced_view, + ArrayFromJSON(boolean(), "[false, null]")); + CheckScalarBinary("equal", sliced_view, sliced_str, + ArrayFromJSON(boolean(), "[false, null]")); + + // Array - Scalar + auto scalar_str = ScalarFromJSON(utf8(), R"("b")"); + auto scalar_view = ScalarFromJSON(utf8_view(), R"("b")"); + auto scalar_large = ScalarFromJSON(large_utf8(), R"("b")"); + + auto arr_view = ArrayFromJSON(utf8_view(), R"(["a", "b", "c", null])"); + auto arr_str = ArrayFromJSON(utf8(), R"(["a", "b", "c", null])"); + + CheckScalarBinary("equal", arr_view, scalar_str, + ArrayFromJSON(boolean(), "[false, true, false, null]")); + CheckScalarBinary("equal", scalar_str, arr_view, + ArrayFromJSON(boolean(), "[false, true, false, null]")); + CheckScalarBinary("equal", arr_str, scalar_view, + ArrayFromJSON(boolean(), "[false, true, false, null]")); + CheckScalarBinary("equal", scalar_view, arr_str, + ArrayFromJSON(boolean(), "[false, true, false, null]")); + CheckScalarBinary("equal", arr_view, scalar_large, + ArrayFromJSON(boolean(), "[false, true, false, null]")); + + // Long scalar (> 12 bytes) + auto long_scalar_str = ScalarFromJSON(utf8(), R"("this_is_a_long_string_1")"); + auto long_scalar_view = ScalarFromJSON(utf8_view(), R"("this_is_a_long_string_1")"); + CheckScalarBinary("equal", long_view_arr, long_scalar_str, + ArrayFromJSON(boolean(), "[true, false, false, null]")); + CheckScalarBinary("equal", long_scalar_str, long_view_arr, + ArrayFromJSON(boolean(), "[true, false, false, null]")); + CheckScalarBinary("equal", long_str_arr, long_scalar_view, + ArrayFromJSON(boolean(), "[true, false, null, false]")); + + // Asymmetric slice offsets + auto sliced_lhs = str_arr->Slice(1, 2); // ["b", null] + auto sliced_rhs = view_arr->Slice(2, 2); // ["c", null] + CheckScalarBinary("equal", sliced_lhs, sliced_rhs, + ArrayFromJSON(boolean(), "[false, null]")); + + // Null scalar comparisons + auto null_scalar_str = MakeNullScalar(utf8()); + auto null_scalar_view = MakeNullScalar(utf8_view()); + CheckScalarBinary("equal", view_arr, null_scalar_str, + ArrayFromJSON(boolean(), "[null, null, null, null]")); + CheckScalarBinary("equal", str_arr, null_scalar_view, + ArrayFromJSON(boolean(), "[null, null, null, null]")); + + // Embedded null bytes (\0) across inline (<=12B) and out-of-line (>12B) views + auto null_byte_str = ArrayFromJSON( + utf8(), R"(["a\u0000b", "a\u0000x", "long_prefix_\u0000_x"])"); + auto null_byte_view = ArrayFromJSON( + utf8_view(), R"(["a\u0000b", "a\u0000y", "long_prefix_\u0000_y"])"); + CheckScalarBinary("equal", null_byte_str, null_byte_view, + ArrayFromJSON(boolean(), "[true, false, false]")); + CheckScalarBinary("less", null_byte_str, null_byte_view, + ArrayFromJSON(boolean(), "[false, true, true]")); + + // Binary and BinaryView + auto bin_arr = ArrayFromJSON(binary(), R"(["a", "b", null, "d"])"); + auto bin_view_arr = ArrayFromJSON(binary_view(), R"(["a", "x", "c", null])"); + CheckScalarBinary("equal", bin_arr, bin_view_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); + CheckScalarBinary("equal", bin_view_arr, bin_arr, + ArrayFromJSON(boolean(), "[true, false, null, null]")); +} + +TEST(TestCompareKernel, TestIssue54668) { + // Direct mixed comparison between utf8 / large_utf8 and utf8_view without promotion + for (std::string name : + {"equal", "not_equal", "less", "less_equal", "greater", "greater_equal"}) { + CheckDispatchBest(name, {utf8(), utf8_view()}, {utf8(), utf8_view()}); + CheckDispatchBest(name, {utf8_view(), utf8()}, {utf8_view(), utf8()}); + CheckDispatchBest(name, {large_utf8(), utf8_view()}, {large_utf8(), utf8_view()}); + CheckDispatchBest(name, {utf8_view(), large_utf8()}, {utf8_view(), large_utf8()}); + } + + auto str_arr = ArrayFromJSON(utf8(), R"(["apple", "banana", "cherry"])"); + auto view_arr = ArrayFromJSON(utf8_view(), R"(["banana", "banana", "apple"])"); + + // Verify less and less_equal with asymmetric types + CheckScalarBinary("less", str_arr, view_arr, + ArrayFromJSON(boolean(), "[true, false, false]")); + CheckScalarBinary("less", view_arr, str_arr, + ArrayFromJSON(boolean(), "[false, false, true]")); + CheckScalarBinary("greater", str_arr, view_arr, + ArrayFromJSON(boolean(), "[false, false, true]")); + CheckScalarBinary("greater", view_arr, str_arr, + ArrayFromJSON(boolean(), "[true, false, false]")); + + // Verify decimal comparisons with different scales (e.g. 1.00 < 9.0) + auto d2_type = decimal128(3, 2); + auto d1_type = decimal128(3, 1); + auto d2_arr = ArrayFromJSON(d2_type, R"(["1.00", "9.00"])"); + auto d1_arr = ArrayFromJSON(d1_type, R"(["9.0", "1.0"])"); + CheckScalarBinary("less", d2_arr, d1_arr, + ArrayFromJSON(boolean(), "[true, false]")); + CheckScalarBinary("less_equal", d2_arr, d1_arr, + ArrayFromJSON(boolean(), "[true, false]")); + CheckScalarBinary("greater", d2_arr, d1_arr, + ArrayFromJSON(boolean(), "[false, true]")); + CheckScalarBinary("greater_equal", d2_arr, d1_arr, + ArrayFromJSON(boolean(), "[false, true]")); + + // Verify negative decimal comparisons with different scales + // -1.00 > -9.00, so -1.00 < -9.0 is false; unscaled integers (-100 < -90) would be true + auto d2_neg = ArrayFromJSON(d2_type, R"(["-1.00", "-9.00"])"); + auto d1_neg = ArrayFromJSON(d1_type, R"(["-9.0", "-1.0"])"); + CheckScalarBinary("less", d2_neg, d1_neg, + ArrayFromJSON(boolean(), "[false, true]")); + CheckScalarBinary("less_equal", d2_neg, d1_neg, + ArrayFromJSON(boolean(), "[false, true]")); + CheckScalarBinary("greater", d2_neg, d1_neg, + ArrayFromJSON(boolean(), "[true, false]")); + CheckScalarBinary("greater_equal", d2_neg, d1_neg, + ArrayFromJSON(boolean(), "[true, false]")); + + // Verify 12-byte inline vs 13-byte out-of-line StringView boundary & 4-byte prefix match + auto boundary_str = + ArrayFromJSON(utf8(), R"(["123456789012", "1234567890123", "prefix_alpha"])"); + auto boundary_view = + ArrayFromJSON(utf8_view(), R"(["1234567890123", "123456789012", "prefix_omega"])"); + CheckScalarBinary("less", boundary_str, boundary_view, + ArrayFromJSON(boolean(), "[true, false, true]")); + CheckScalarBinary("less", boundary_view, boundary_str, + ArrayFromJSON(boolean(), "[false, true, false]")); + CheckScalarBinary("equal", boundary_str, boundary_view, + ArrayFromJSON(boolean(), "[false, false, false]")); + CheckScalarBinary("not_equal", boundary_str, boundary_view, + ArrayFromJSON(boolean(), "[true, true, true]")); + + // Dictionary-encoded string vs utf8_view (both directions) + auto dict_str = + ArrayFromJSON(dictionary(int32(), utf8()), R"(["apple", "banana", null])"); + auto plain_view = ArrayFromJSON(utf8_view(), R"(["apple", "cherry", "banana"])"); + CheckScalarBinary("equal", dict_str, plain_view, + ArrayFromJSON(boolean(), "[true, false, null]")); + CheckScalarBinary("equal", plain_view, dict_str, + ArrayFromJSON(boolean(), "[true, false, null]")); +} + template class TestVarArgsCompare : public ::testing::Test { protected: diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index d4d44e9347e..12b3cb71cf3 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -1873,6 +1873,63 @@ def con(values): assert result.equals(con([False, True, True, None])) +@pytest.mark.parametrize( + "left_type", [pa.string(), pa.large_string(), pa.string_view()] +) +@pytest.mark.parametrize( + "right_type", [pa.string(), pa.large_string(), pa.string_view()] +) +def test_compare_mixed_string_types(left_type, right_type): + # Short strings (<= 12 bytes) + left = pa.array(["a", "b", None, "d"], type=left_type) + right = pa.array(["a", "x", "c", None], type=right_type) + + result = pc.equal(left, right) + assert result.to_pylist() == [True, False, None, None] + + result = pc.not_equal(left, right) + assert result.to_pylist() == [False, True, None, None] + + result = pc.less(left, right) + assert result.to_pylist() == [False, True, None, None] + + # Sliced arrays + assert pc.equal(left.slice(1, 2), right.slice(1, 2)).to_pylist() == [False, None] + + # Long strings (> 12 bytes testing out-of-line view prefix + data buffer) + long_left = pa.array( + ["this_is_a_long_string_1", "this_is_a_long_string_2", None], type=left_type + ) + long_right = pa.array( + ["this_is_a_long_string_1", "this_is_a_different_string", "c"], type=right_type + ) + assert pc.equal(long_left, long_right).to_pylist() == [True, False, None] + + # Array vs Scalar + scalar = pa.scalar("b", type=right_type) + result = pc.equal(left, scalar) + assert result.to_pylist() == [False, True, None, False] + + +@pytest.mark.parametrize( + "left_type", [pa.binary(), pa.large_binary(), pa.binary_view()] +) +@pytest.mark.parametrize( + "right_type", [pa.binary(), pa.large_binary(), pa.binary_view()] +) +def test_compare_mixed_binary_types(left_type, right_type): + left = pa.array([b"a", b"b", None, b"d"], type=left_type) + right = pa.array([b"a", b"x", b"c", None], type=right_type) + + assert pc.equal(left, right).to_pylist() == [True, False, None, None] + assert pc.not_equal(left, right).to_pylist() == [False, True, None, None] + assert pc.less(left, right).to_pylist() == [False, True, None, None] + + # Array vs Scalar + scalar = pa.scalar(b"b", type=right_type) + assert pc.equal(left, scalar).to_pylist() == [False, True, None, False] + + @pytest.mark.parametrize("typ", ["array", "chunked_array"]) def test_compare_scalar(typ): if typ == "array":