This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 2c1293f3c5 GH-47447: [C++] Fix replace_with_mask for null type arrays
(#49950)
2c1293f3c5 is described below
commit 2c1293f3c5438202e133a59d9478cecef33b35ea
Author: AnkitAhlawat <[email protected]>
AuthorDate: Thu May 28 08:10:56 2026 +0530
GH-47447: [C++] Fix replace_with_mask for null type arrays (#49950)
### Rationale for this change
replace_with_mask crashes when called with null type arrays because a code
path incorrectly returns Status::OK() from a function with return type
Result<int64_t>.
This PR fixes the issue by ensuring the function returns a valid int64_t
value instead of Status::OK() for successful execution.
### What changes are included in this PR?
1. Fixed the replace_with_mask kernel implementation for null type arrays.
2. Replaced the invalid Status::OK() return path with a valid int64_t
result.
3. Prevented construction of Result<int64_t> with an OK Status.
### Are these changes tested?
Yes, manually run the test case
### Are there any user-facing changes?
No
**This PR contains a "Critical Fix".**
This change fixes a crash in replace_with_mask when operating on null type
arrays. The crash occurs due to an invalid successful return path internally
constructing Result<T> with Status::OK().
* GitHub Issue: #47447
Authored-by: [email protected] <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/arrow/compute/kernels/vector_replace.cc | 8 ++---
.../arrow/compute/kernels/vector_replace_test.cc | 36 ++++++++++++++++++++++
python/pyarrow/tests/test_compute.py | 28 +++++++++++++++++
3 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/cpp/src/arrow/compute/kernels/vector_replace.cc
b/cpp/src/arrow/compute/kernels/vector_replace.cc
index 1a3e743784..ae39977aae 100644
--- a/cpp/src/arrow/compute/kernels/vector_replace.cc
+++ b/cpp/src/arrow/compute/kernels/vector_replace.cc
@@ -217,15 +217,15 @@ struct ReplaceMaskImpl<Type, enable_if_null<Type>> {
static Result<int64_t> ExecScalarMask(KernelContext* ctx, const ArraySpan&
array,
const BooleanScalar& mask, ExecValue
replacements,
int64_t replacements_offset,
ExecResult* out) {
- out->value = array;
- return Status::OK();
+ out->value = array.ToArrayData();
+ return replacements_offset;
}
static Result<int64_t> ExecArrayMask(KernelContext* ctx, const ArraySpan&
array,
const ArraySpan& mask, int64_t
mask_offset,
ExecValue replacements,
int64_t replacements_offset,
ExecResult* out) {
- out->value = array;
- return Status::OK();
+ out->value = array.ToArrayData();
+ return replacements_offset;
}
};
diff --git a/cpp/src/arrow/compute/kernels/vector_replace_test.cc
b/cpp/src/arrow/compute/kernels/vector_replace_test.cc
index 587b9f2a60..9dc8e70ab6 100644
--- a/cpp/src/arrow/compute/kernels/vector_replace_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_replace_test.cc
@@ -199,6 +199,13 @@ class TestReplaceBoolean : public
TestReplaceKernel<BooleanType> {
}
};
+class TestReplaceNull : public TestReplaceKernel<NullType> {
+ protected:
+ std::shared_ptr<DataType> type() override {
+ return TypeTraits<NullType>::type_singleton();
+ }
+};
+
class TestReplaceFixedSizeBinary : public
TestReplaceKernel<FixedSizeBinaryType> {
protected:
std::shared_ptr<DataType> type() override { return fixed_size_binary(3); }
@@ -538,6 +545,35 @@ TEST_F(TestReplaceBoolean, ReplaceWithMask) {
}
}
+TEST_F(TestReplaceNull, ReplaceWithMask) {
+ std::vector<ReplaceWithMaskCase> cases = {
+ {this->array("[]"), this->mask_scalar(false), this->array("[]"),
this->array("[]")},
+ {this->array("[]"), this->mask_scalar(true), this->array("[]"),
this->array("[]")},
+ {this->array("[]"), this->null_mask_scalar(), this->array("[]"),
this->array("[]")},
+
+ {this->array("[null]"), this->mask_scalar(false), this->array("[]"),
+ this->array("[null]")},
+
+ {this->array("[null]"), this->mask_scalar(true), this->array("[null]"),
+ this->array("[null]")},
+
+ {this->array("[null]"), this->null_mask_scalar(), this->array("[]"),
+ this->array("[null]")},
+
+ {this->array("[null, null]"), this->mask("[false, false]"),
this->array("[]"),
+ this->array("[null, null]")},
+ {this->array("[null, null]"), this->mask("[true, true]"),
+ this->array("[null, null]"), this->array("[null, null]")},
+ {this->array("[null, null]"), this->mask("[null, null]"),
this->array("[]"),
+ this->array("[null, null]")},
+ };
+
+ for (auto test_case : cases) {
+ this->Assert(ReplaceWithMask, test_case.input, test_case.mask,
test_case.replacements,
+ test_case.expected);
+ }
+}
+
TEST_F(TestReplaceBoolean, ReplaceWithMaskErrors) {
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid,
diff --git a/python/pyarrow/tests/test_compute.py
b/python/pyarrow/tests/test_compute.py
index 4e44a912d9..1c82d6c944 100644
--- a/python/pyarrow/tests/test_compute.py
+++ b/python/pyarrow/tests/test_compute.py
@@ -1269,6 +1269,34 @@ def test_extract_regex_span():
assert struct.tolist() == expected
+def test_replace_with_mask_null_type():
+ # GH-47447: replace_with_mask crashed for null type arrays
+ input = pa.array([None], pa.null())
+ replacements = pa.array([None], pa.null())
+
+ result = pc.replace_with_mask(input, True, replacements)
+ assert result.type == pa.null()
+ result.validate(full=True)
+ assert result.to_pylist() == [None]
+
+ result = pc.replace_with_mask(input, False, replacements)
+ assert result.type == pa.null()
+ result.validate(full=True)
+ assert result.to_pylist() == [None]
+
+ mask = pa.array([True])
+ result = pc.replace_with_mask(input, mask, replacements)
+ assert result.type == pa.null()
+ result.validate(full=True)
+ assert result.to_pylist() == [None]
+
+ mask = pa.array([False])
+ result = pc.replace_with_mask(input, mask, replacements)
+ assert result.type == pa.null()
+ result.validate(full=True)
+ assert result.to_pylist() == [None]
+
+
def test_binary_join():
ar_list = pa.array([['foo', 'bar'], None, []])
expected = pa.array(['foo-bar', None, ''])
