Awesome06 opened a new issue, #55256:
URL: https://github.com/apache/arrow/issues/55256

   ### Describe the bug, including details regarding any error messages, 
version, and platform.
   
   The vector replacement compute functions (`replace_with_mask`, 
`fill_null_forward`, and `fill_null_backward`) fail with 
`arrow::Status::NotImplemented` when invoked with:
   - `utf8_view` (`string_view`) or `binary_view`
   - Modern 32-bit and 64-bit decimal types (`decimal32`, `decimal64`)
   
   While classic variable-length binary types (`utf8`, `large_utf8`, `binary`, 
`large_binary`), primitives, and `Decimal128` / `Decimal256` are supported, 
modern view types and small decimals are omitted from the kernel registration 
table.
   
   In 
[`cpp/src/arrow/compute/kernels/vector_replace.cc`](https://github.com/apache/arrow/blob/main/cpp/src/arrow/compute/kernels/vector_replace.cc#L845-L865),
 `RegisterVectorFunction` registers fixed-size binary, legacy decimals, and 
`BaseBinaryTypes()`:
   
   ```cpp
   AddKernel(Type::FIXED_SIZE_BINARY,
             
Functor<FixedSizeBinaryType>::GetSignature(Type::FIXED_SIZE_BINARY),
             Functor<FixedSizeBinaryType>::Exec, 
ChunkedFunctor<FixedSizeBinaryType>::Exec,
             registry, func.get());
   AddKernel(Type::DECIMAL128,
             Functor<FixedSizeBinaryType>::GetSignature(Type::DECIMAL128),
             Functor<FixedSizeBinaryType>::Exec, 
ChunkedFunctor<FixedSizeBinaryType>::Exec,
             registry, func.get());
   AddKernel(Type::DECIMAL256,
             Functor<FixedSizeBinaryType>::GetSignature(Type::DECIMAL256),
             Functor<FixedSizeBinaryType>::Exec, 
ChunkedFunctor<FixedSizeBinaryType>::Exec,
             registry, func.get());
   for (const auto& ty : BaseBinaryTypes()) {
     AddKernel(ty->id(), Functor<FixedSizeBinaryType>::GetSignature(ty->id()),
               GenerateTypeAgnosticVarBinaryBase<Functor>(*ty),
               GenerateTypeAgnosticVarBinaryBase<ChunkedFunctor>(*ty), registry,
               func.get());
   }
   ```
   
   Because `replace_with_mask`, `fill_null_forward`, and `fill_null_backward` 
all instantiate kernels via this template helper, none of them support:
   1. `BinaryViewType` or `StringViewType` (`BaseBinaryTypes()` only covers 
`binary`, `large_binary`, `utf8`, `large_utf8`).
   2. `Decimal32Type` or `Decimal64Type` (only `DECIMAL128` and `DECIMAL256` 
are registered).
   
   #### Reproduction
   
   The issue reproduces in C++ (and PyArrow):
   
   ```cpp
   TEST(TestVectorReplace, StringViewAndSmallDecimals) {
     // 1. replace_with_mask on utf8_view
     auto view_arr = ArrayFromJSON(utf8_view(), "[\"apple\", null, 
\"cherry\"]");
     auto mask = ArrayFromJSON(boolean(), "[false, true, false]");
     auto replacements = ArrayFromJSON(utf8_view(), "[\"banana\"]");
   
     auto res1 = arrow::compute::ReplaceWithMask(view_arr, mask, replacements);
     ASSERT_OK(res1.status());
   
     // 2. fill_null_forward on utf8_view
     auto res2 = arrow::compute::FillNullForward(view_arr);
     ASSERT_OK(res2.status());
   
     // 3. replace_with_mask on decimal32
     auto dec32_type = decimal32(7, 2);
     auto dec_arr = ArrayFromJSON(dec32_type, "[\"12.34\", null, \"56.78\"]");
     auto dec_repl = ArrayFromJSON(dec32_type, "[\"99.99\"]");
   
     auto res3 = arrow::compute::ReplaceWithMask(dec_arr, mask, dec_repl);
     ASSERT_OK(res3.status());
   }
   ```
   
   Or in Python:
   
   ```python
   import pyarrow as pa
   import pyarrow.compute as pc
   
   # 1. string_view replace_with_mask
   views = pa.array(["apple", None, "cherry"], type=pa.string_view())
   mask = pa.array([False, True, False])
   repl = pa.array(["banana"], type=pa.string_view())
   
   pc.replace_with_mask(views, mask, repl)
   
   # 2. string_view fill_null_forward
   pc.fill_null_forward(views)
   
   # 3. decimal32 replace_with_mask
   dec_arr = pa.array([1234, None, 5678], type=pa.decimal32(7, 2))
   dec_repl = pa.array([9999], type=pa.decimal32(7, 2))
   pc.replace_with_mask(dec_arr, mask, dec_repl)
   ```
   
   **Expected:**
   - `replace_with_mask(views, mask, repl)` returns `string_view` array 
`["apple", "banana", "cherry"]`.
   - `fill_null_forward(views)` returns `string_view` array `["apple", "apple", 
"cherry"]`.
   - `replace_with_mask(dec_arr, mask, dec_repl)` returns `decimal32(7, 2)` 
array `["12.34", "99.99", "56.78"]`.
   
   **Actual:**
   ```text
   ArrowNotImplementedError: Function 'replace_with_mask' has no kernel 
matching input types (string_view, boolean, string_view)
   ArrowNotImplementedError: Function 'fill_null_forward' has no kernel 
matching input types (string_view)
   ArrowNotImplementedError: Function 'fill_null_backward' has no kernel 
matching input types (string_view)
   ArrowNotImplementedError: Function 'replace_with_mask' has no kernel 
matching input types (decimal32(7, 2), boolean, decimal32(7, 2))
   ```
   
   Reproduced on current `main` across all platforms. This is an unfiled 
type-matrix parity gap in Arrow C++ compute.
   
   #### Suggested fix
   
   1. **For `decimal32` and `decimal64`:**
      In `RegisterVectorFunction` in 
`cpp/src/arrow/compute/kernels/vector_replace.cc`, register `Type::DECIMAL32` 
and `Type::DECIMAL64` matching the existing `DECIMAL128` fixed-width pattern:
      ```cpp
      AddKernel(Type::DECIMAL32,
                Functor<FixedSizeBinaryType>::GetSignature(Type::DECIMAL32),
                Functor<FixedSizeBinaryType>::Exec, 
ChunkedFunctor<FixedSizeBinaryType>::Exec,
                registry, func.get());
      AddKernel(Type::DECIMAL64,
                Functor<FixedSizeBinaryType>::GetSignature(Type::DECIMAL64),
                Functor<FixedSizeBinaryType>::Exec, 
ChunkedFunctor<FixedSizeBinaryType>::Exec,
                registry, func.get());
      ```
   
   2. **For `string_view` and `binary_view`:**
      Add view-aware execution functors to `vector_replace.cc` and 
`copy_data_internal.h`:
      - Copy the 16-byte view descriptors (`StringViewType::c_type`) during 
element replacement or propagation.
      - For `replace_with_mask`, merge the data buffers of the replacement 
array/scalar into the output array's `data->buffers`.
      - For `fill_null_forward` / `fill_null_backward`, forward the 16-byte 
descriptors directly and retain the input array's data buffers.
   
   3. **Add Unit Tests:**
      Add test coverage in 
`cpp/src/arrow/compute/kernels/vector_replace_test.cc` verifying:
      - Sliced view arrays with non-zero offsets.
      - Runs of multiple consecutive nulls with `fill_null_forward` and 
`fill_null_backward`.
      - Array-array and array-scalar replacement permutations for `utf8_view`, 
`binary_view`, `decimal32`, and `decimal64`.
   
   ### Component(s)
   
   C++


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to