From f53968e2cd51d7676eb713ca6c84ace0dc556024 Mon Sep 17 00:00:00 2001 From: Mrigank Bhatnagar Date: Sat, 24 Oct 2026 14:07:13 +0530 Subject: [PATCH 1/5] GH-54668: [C++][Compute] Implement equality comparison between Utf8View and other string types ### Rationale for this change Currently, Arrow compute supports comparing `Utf8` with `LargeUtf8` and comparing `Utf8View` with `Utf8View`, but attempting to compare a `Utf8View` array/scalar with either `Utf8` or `LargeUtf8` fails with: ```text ArrowNotImplementedError: Function 'equal' has no kernel matching input types (string, string_view) ``` In `scalar_compare.cc`, comparison operators (`equal`, `not_equal`, `less`, `greater`, etc.) enforce homogeneous symmetric kernel signatures (`{ty, ty}`) and rely on `DispatchBest` to resolve heterogeneous types via `CommonBinary()`. However, `CommonBinary()` did not previously recognize `Type::STRING_VIEW` or `Type::BINARY_VIEW`, returning `nullptr` and causing `DispatchBest` to fail. When mixing string/binary view types with offset-based string/binary types, promoting to the view type (`utf8_view` or `binary_view`) is optimal: 1. `ZeroCopyCastExec` in `scalar_cast_string.cc` reuses the existing character data buffer directly as variadic buffer 0, only allocating 16-byte descriptors without copying string payload data. (Conversely, promoting to `LargeUtf8` would require a deep copy of all string data into contiguous buffers). 2. For scalar-array comparisons (`string_view_array == "literal"`), only the scalar literal is converted ($O(1)$ zero-allocation), leaving array memory completely untouched. ### What changes are included in this PR? 1. `cpp/src/arrow/compute/kernels/codegen_internal.cc`: - Extended `CommonBinary()` to handle `Type::STRING_VIEW` and `Type::BINARY_VIEW`. - Promotes mixed string types to `utf8_view()`, and mixed binary/fixed-size types to `binary_view()`. 2. `cpp/src/arrow/compute/kernels/scalar_compare_test.cc`: - Added `CheckDispatchBest` assertions for view <-> non-view binary and string combinations (`utf8`, `large_utf8`, `utf8_view`, `binary`, `large_binary`, `binary_view`, `fixed_size_binary`). - Added `TEST(TestBinaryViewCompareKernel, MixedStringTypes)` validating: - Short strings (<= 12 bytes, testing inline descriptors) - Long strings (> 12 bytes, testing out-of-line prefix + buffer lookup) - Sliced arrays (`array.Slice(1, 2)`) - Empty arrays (`[]`) and all-null arrays (`[null, null]`) - Array-scalar and scalar-array operations across `equal`, `not_equal`, `less`, and `greater`. 3. `python/pyarrow/tests/test_compute.py`: - Added `test_compare_mixed_string_types` parameterized across all 9 permutations of `string`, `large_string`, and `string_view` (including sliced and >12-byte strings). - Added `test_compare_mixed_binary_types` parameterized across all 9 permutations of `binary`, `large_binary`, and `binary_view`. ### Are these changes tested? Yes: - C++ unit tests in `arrow-compute-scalar-math-test` (including `TestCompareKernel.DispatchBest` and `TestBinaryViewCompareKernel.MixedStringTypes`). All 594 tests in the test suite pass with 0 failures. - PyArrow unit tests added to `python/pyarrow/tests/test_compute.py`. ### Are there any user-facing changes? Yes. Users can now directly compare `Utf8View` / `BinaryView` columns against `Utf8`, `LargeUtf8`, `Binary`, and `LargeBinary` columns or scalars in expressions and compute kernels without explicit casting or `ArrowNotImplementedError`. ### Was AI used for this PR? AI was used to diagnose the type promotion dispatch path, draft implementation code, and construct test cases under developer direction. The changes and tests were reviewed, compiled, and verified locally. **PR code and description written by:** - [X] Human - [X] AI **Reviewed before submission by:** - [X] Human - [X] AI - [ ] Not reviewed * GitHub Issue: #54668 --- .../arrow/compute/kernels/codegen_internal.cc | 16 ++- .../compute/kernels/scalar_compare_test.cc | 110 ++++++++++++++++++ python/pyarrow/tests/test_compute.py | 57 +++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.cc b/cpp/src/arrow/compute/kernels/codegen_internal.cc index 99a9173041fc..4b3ba05f7ffe 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.cc +++ b/cpp/src/arrow/compute/kernels/codegen_internal.cc @@ -348,7 +348,7 @@ TypeHolder CommonTemporal(const TypeHolder* begin, size_t count) { } TypeHolder CommonBinary(const TypeHolder* begin, size_t count) { - bool all_utf8 = true, all_offset32 = true, all_fixed_width = true; + bool all_utf8 = true, all_offset32 = true, all_fixed_width = true, has_view = false; const TypeHolder* end = begin + count; for (auto it = begin; it != end; ++it) { @@ -374,6 +374,15 @@ TypeHolder CommonBinary(const TypeHolder* begin, size_t count) { all_fixed_width = false; all_utf8 = false; continue; + case Type::STRING_VIEW: + all_fixed_width = false; + has_view = true; + continue; + case Type::BINARY_VIEW: + all_fixed_width = false; + all_utf8 = false; + has_view = true; + continue; default: return TypeHolder(nullptr); } @@ -384,6 +393,11 @@ TypeHolder CommonBinary(const TypeHolder* begin, size_t count) { return TypeHolder(nullptr); } + if (has_view) { + if (all_utf8) return utf8_view(); + return binary_view(); + } + if (all_utf8) { if (all_offset32) return utf8(); return large_utf8(); diff --git a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc index 2aae5bf2ee90..37df0ba12c89 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc @@ -1021,6 +1021,22 @@ 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_view(), utf8_view()}); + CheckDispatchBest(name, {utf8_view(), utf8()}, {utf8_view(), utf8_view()}); + CheckDispatchBest(name, {large_utf8(), utf8_view()}, {utf8_view(), utf8_view()}); + CheckDispatchBest(name, {utf8_view(), large_utf8()}, {utf8_view(), utf8_view()}); + CheckDispatchBest(name, {binary(), binary_view()}, {binary_view(), binary_view()}); + CheckDispatchBest(name, {binary_view(), binary()}, {binary_view(), binary_view()}); + CheckDispatchBest(name, {large_binary(), binary_view()}, + {binary_view(), binary_view()}); + CheckDispatchBest(name, {binary_view(), large_binary()}, + {binary_view(), binary_view()}); + CheckDispatchBest(name, {utf8_view(), binary_view()}, + {binary_view(), binary_view()}); + CheckDispatchBest(name, {utf8_view(), binary()}, {binary_view(), binary_view()}); + CheckDispatchBest(name, {utf8(), binary_view()}, {binary_view(), binary_view()}); + CheckDispatchBest(name, {fixed_size_binary(2), utf8_view()}, + {binary_view(), binary_view()}); CheckDispatchBest(name, {decimal128(3, 2), decimal128(6, 3)}, {decimal128(4, 3), decimal128(6, 3)}); @@ -1245,6 +1261,100 @@ 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]")); + + // 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]")); +} + template class TestVarArgsCompare : public ::testing::Test { protected: diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index d4d44e9347e0..12b3cb71cf32 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": From 22685ea03c4fce7eec16b02b7abe0eff6d5f303e Mon Sep 17 00:00:00 2001 From: Mrigank Bhatnagar Date: Sun, 25 Oct 2026 14:51:01 +0530 Subject: [PATCH 2/5] GH-54668: [C++][Compute] Support direct mixed comparisons between string and string_view Avoid promoting string scalars to string_view in CommonBinary during scalar comparisons. Instead, register direct bidirectional kernels for utf8/binary and their view variants using ScalarBinary to compare borrowed string views zero-copy. Also reverse the input signature in MakeFlippedCompare so asymmetric arguments dispatch correctly to flipped comparison kernels. * Closes: #55171 --- .../arrow/compute/kernels/codegen_internal.cc | 16 +------ .../arrow/compute/kernels/scalar_compare.cc | 25 ++++++++++ .../compute/kernels/scalar_compare_test.cc | 46 +++++++++++++------ 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.cc b/cpp/src/arrow/compute/kernels/codegen_internal.cc index 4b3ba05f7ffe..99a9173041fc 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.cc +++ b/cpp/src/arrow/compute/kernels/codegen_internal.cc @@ -348,7 +348,7 @@ TypeHolder CommonTemporal(const TypeHolder* begin, size_t count) { } TypeHolder CommonBinary(const TypeHolder* begin, size_t count) { - bool all_utf8 = true, all_offset32 = true, all_fixed_width = true, has_view = false; + bool all_utf8 = true, all_offset32 = true, all_fixed_width = true; const TypeHolder* end = begin + count; for (auto it = begin; it != end; ++it) { @@ -374,15 +374,6 @@ TypeHolder CommonBinary(const TypeHolder* begin, size_t count) { all_fixed_width = false; all_utf8 = false; continue; - case Type::STRING_VIEW: - all_fixed_width = false; - has_view = true; - continue; - case Type::BINARY_VIEW: - all_fixed_width = false; - all_utf8 = false; - has_view = true; - continue; default: return TypeHolder(nullptr); } @@ -393,11 +384,6 @@ TypeHolder CommonBinary(const TypeHolder* begin, size_t count) { return TypeHolder(nullptr); } - if (has_view) { - if (all_utf8) return utf8_view(); - return binary_view(); - } - if (all_utf8) { if (all_offset32) return utf8(); return large_utf8(); diff --git a/cpp/src/arrow/compute/kernels/scalar_compare.cc b/cpp/src/arrow/compute/kernels/scalar_compare.cc index 0f2c479b0828..90a63ec83b58 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,10 @@ 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; + auto in_types = kernel->signature->in_types(); + std::reverse(in_types.begin(), in_types.end()); + flipped_kernel.signature = + KernelSignature::Make(std::move(in_types), 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 37df0ba12c89..79e5dc705dc7 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc @@ -1021,22 +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_view(), utf8_view()}); - CheckDispatchBest(name, {utf8_view(), utf8()}, {utf8_view(), utf8_view()}); - CheckDispatchBest(name, {large_utf8(), utf8_view()}, {utf8_view(), utf8_view()}); - CheckDispatchBest(name, {utf8_view(), large_utf8()}, {utf8_view(), utf8_view()}); - CheckDispatchBest(name, {binary(), binary_view()}, {binary_view(), binary_view()}); - CheckDispatchBest(name, {binary_view(), binary()}, {binary_view(), binary_view()}); + 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()}, - {binary_view(), binary_view()}); + {large_binary(), binary_view()}); CheckDispatchBest(name, {binary_view(), large_binary()}, - {binary_view(), binary_view()}); - CheckDispatchBest(name, {utf8_view(), binary_view()}, - {binary_view(), binary_view()}); - CheckDispatchBest(name, {utf8_view(), binary()}, {binary_view(), binary_view()}); - CheckDispatchBest(name, {utf8(), binary_view()}, {binary_view(), binary_view()}); - CheckDispatchBest(name, {fixed_size_binary(2), utf8_view()}, - {binary_view(), binary_view()}); + {binary_view(), large_binary()}); CheckDispatchBest(name, {decimal128(3, 2), decimal128(6, 3)}, {decimal128(4, 3), decimal128(6, 3)}); @@ -1355,6 +1349,30 @@ TEST(TestBinaryViewCompareKernel, MixedStringTypes) { 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]")); +} + template class TestVarArgsCompare : public ::testing::Test { protected: From d47ffb164a18f3d6918251f1e17f780cadc851c6 Mon Sep 17 00:00:00 2001 From: Mrigank Bhatnagar Date: Sun, 25 Oct 2026 15:45:39 +0530 Subject: [PATCH 3/5] GH-54668: [C++][Compute] Preserve decimal match constraint on flipped comparisons In MakeFlippedCompare, only construct a reversed KernelSignature when input types differ (in_types[0] != in_types[1]). This ensures symmetric kernels retain their original signature and attached MatchConstraints (such as DecimalsHaveSameScale), preventing raw unscaled integer comparisons on decimals with differing scales. Also add regression tests for bound expressions and scalar comparisons with different decimal scales. --- cpp/src/arrow/compute/expression_test.cc | 26 +++++++++++++++++++ .../arrow/compute/kernels/scalar_compare.cc | 9 ++++--- .../compute/kernels/scalar_compare_test.cc | 14 ++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/compute/expression_test.cc b/cpp/src/arrow/compute/expression_test.cc index 10c1b348e62d..55b67f76e663 100644 --- a/cpp/src/arrow/compute/expression_test.cc +++ b/cpp/src/arrow/compute/expression_test.cc @@ -2379,5 +2379,31 @@ 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); +} + } // 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 90a63ec83b58..e34bdeb5594d 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare.cc @@ -499,10 +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; - auto in_types = kernel->signature->in_types(); - std::reverse(in_types.begin(), in_types.end()); - flipped_kernel.signature = - KernelSignature::Make(std::move(in_types), kernel->signature->out_type()); + 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 79e5dc705dc7..5b9547df9657 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc @@ -1371,6 +1371,20 @@ TEST(TestCompareKernel, TestIssue54668) { 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]")); } template From b42096cde7fc0b1fe356a77599dd6d50b819bb1c Mon Sep 17 00:00:00 2001 From: Mrigank Bhatnagar Date: Sun, 25 Oct 2026 15:57:30 +0530 Subject: [PATCH 4/5] GH-54668: [C++][Compute] Add edge-case tests for negative decimals and string view boundary Extend TestIssue54668 to cover: 1) Negative decimal comparisons with differing scales, ensuring signed two's complement unscaled integers (-100 < -90) do not falsely return true for -1.00 < -9.0. 2) StringView 12-byte inline vs 13-byte out-of-line payload boundary transitions. 3) Shared 4-byte prefix matching with differing out-of-line payloads (prefix collision). 4) Bound expressions with literals on the LHS ('banana' < view_col) testing expression simplifier flipping. --- cpp/src/arrow/compute/expression_test.cc | 24 +++++++++++++++++ .../compute/kernels/scalar_compare_test.cc | 27 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/cpp/src/arrow/compute/expression_test.cc b/cpp/src/arrow/compute/expression_test.cc index 55b67f76e663..fd2725d55b4f 100644 --- a/cpp/src/arrow/compute/expression_test.cc +++ b/cpp/src/arrow/compute/expression_test.cc @@ -2403,6 +2403,30 @@ TEST(Expression, TestIssue54668DecimalComparisonDifferentScales) { 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 diff --git a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc index 5b9547df9657..07cd73f1eb55 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc @@ -1385,6 +1385,33 @@ TEST(TestCompareKernel, TestIssue54668) { 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]")); } template From 268f038f66abe0b1a1f99a0aacb8f6623aaeee63 Mon Sep 17 00:00:00 2001 From: Mrigank Bhatnagar Date: Sun, 25 Oct 2026 20:56:53 +0530 Subject: [PATCH 5/5] GH-54668: [C++][Compute] Add tests for asymmetric slices, null scalars, embedded nulls, and dictionary views Expand TestBinaryViewCompareKernel and TestIssue54668 with additional edge-case test coverage for cross-type comparisons: - Asymmetric slice start offsets between string and string_view arrays - Explicit null scalar instances against string_view arrays - Embedded null bytes (\0) in out-of-line (>12B) variadic buffer views - Symmetric dictionary-encoded string vs. utf8_view dispatch --- .../compute/kernels/scalar_compare_test.cc | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc index 07cd73f1eb55..5a78eabe1f9a 100644 --- a/cpp/src/arrow/compute/kernels/scalar_compare_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_compare_test.cc @@ -1340,6 +1340,30 @@ TEST(TestBinaryViewCompareKernel, MixedStringTypes) { 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])"); @@ -1412,6 +1436,15 @@ TEST(TestCompareKernel, TestIssue54668) { 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