Skip to content

Commit fea7897

Browse files
authored
GH-50113: [C++][Python] Fix count for sliced union arrays (#50114)
### Rationale for this change Sliced union arrays could report incorrect results from `count`. For sliced union arrays, `span.GetValues<int8_t>(1)` and `span.GetValues<int32_t>(2)` already return offset-adjusted pointers. The logical null count path then indexed those pointers with `span.offset + i`, effectively applying the parent offset twice. The sparse union path also checked child nullness with `i` instead of `span.offset + i`. ### What changes are included in this PR? * Fixes logical null counting for sliced sparse and dense union arrays. * Add C++ and Python regression tests. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: #50113 Authored-by: fenfeng9 <fenfeng9@qq.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 90a4497 commit fea7897

3 files changed

Lines changed: 73 additions & 4 deletions

File tree

cpp/src/arrow/compute/kernels/aggregate_test.cc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,36 @@ TEST(TestCountKernel, RunEndEncodedNulls) {
951951
ValidateCount(*array->Slice(3, 6), {3, 3});
952952
}
953953

954+
TEST(TestCountKernel, SparseUnionSlicedNulls) {
955+
// GH-50113: Sliced unions can report incorrect null counts in count.
956+
auto type_ids = ArrayFromJSON(int8(), "[0, 1, 0, 0, 1, 1]");
957+
ArrayVector children = {
958+
ArrayFromJSON(float64(), "[0.5, 99.0, null, 3.0, 88.0, 77.0]"),
959+
ArrayFromJSON(boolean(), "[false, null, true, false, true, false]")};
960+
ASSERT_OK_AND_ASSIGN(auto array,
961+
SparseUnionArray::Make(*type_ids, std::move(children)));
962+
963+
// Logical array: [0.5, null, null, 3.0, true, false].
964+
ValidateCount(*array, {4, 2});
965+
// Logical slice: [null, null, 3.0, true].
966+
ValidateCount(*array->Slice(1, 4), {2, 2});
967+
}
968+
969+
TEST(TestCountKernel, DenseUnionSlicedNulls) {
970+
// GH-50113: Sliced unions can report incorrect null counts in count.
971+
auto type_ids = ArrayFromJSON(int8(), "[0, 1, 0, 0, 1, 1]");
972+
auto value_offsets = ArrayFromJSON(int32(), "[0, 0, 1, 2, 1, 2]");
973+
ArrayVector children = {ArrayFromJSON(float64(), "[0.5, null, 3.0]"),
974+
ArrayFromJSON(boolean(), "[null, true, false]")};
975+
ASSERT_OK_AND_ASSIGN(
976+
auto array, DenseUnionArray::Make(*type_ids, *value_offsets, std::move(children)));
977+
978+
// Logical array: [0.5, null, null, 3.0, true, false].
979+
ValidateCount(*array, {4, 2});
980+
// Logical slice: [null, null, 3.0, true].
981+
ValidateCount(*array->Slice(1, 4), {2, 2});
982+
}
983+
954984
template <typename ArrowType>
955985
class TestRandomNumericCountKernel : public ::testing::Test {};
956986

cpp/src/arrow/util/union_util.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ int64_t LogicalSparseUnionNullCount(const ArraySpan& span) {
3333
const int8_t* types = span.GetValues<int8_t>(1); // NOLINT
3434
int64_t null_count = 0;
3535
for (int64_t i = 0; i < span.length; i++) {
36-
const int8_t child_id = sparse_union_type->child_ids()[types[span.offset + i]];
36+
const int8_t child_id = sparse_union_type->child_ids()[types[i]];
3737

38-
null_count += span.child_data[child_id].IsNull(i);
38+
null_count += span.child_data[child_id].IsNull(span.offset + i);
3939
}
4040
return null_count;
4141
}
@@ -48,8 +48,8 @@ int64_t LogicalDenseUnionNullCount(const ArraySpan& span) {
4848
const int32_t* offsets = span.GetValues<int32_t>(2); // NOLINT
4949
int64_t null_count = 0;
5050
for (int64_t i = 0; i < span.length; i++) {
51-
const int8_t child_id = dense_union_type->child_ids()[types[span.offset + i]];
52-
const int32_t offset = offsets[span.offset + i];
51+
const int8_t child_id = dense_union_type->child_ids()[types[i]];
52+
const int32_t offset = offsets[i];
5353
null_count += span.child_data[child_id].IsNull(offset);
5454
}
5555
return null_count;

python/pyarrow/tests/test_compute.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2889,6 +2889,45 @@ def test_count_run_end_encoded_nulls():
28892889
assert pc.count(arr.slice(3, 6), mode="only_null").as_py() == 3
28902890

28912891

2892+
def test_count_sparse_union_sliced_nulls():
2893+
# GH-50113: Sliced unions can report incorrect null counts in count.
2894+
arr = pa.UnionArray.from_sparse(
2895+
pa.array([0, 1, 0, 0, 1, 1], type=pa.int8()),
2896+
[
2897+
pa.array([0.5, 99.0, None, 3.0, 88.0, 77.0]),
2898+
pa.array([False, None, True, False, True, False]),
2899+
]
2900+
)
2901+
2902+
# Logical array: [0.5, None, None, 3.0, True, False].
2903+
assert pc.count(arr, mode="only_valid").as_py() == 4
2904+
assert pc.count(arr, mode="only_null").as_py() == 2
2905+
assert pc.count(arr, mode="all").as_py() == 6
2906+
# Logical slice: [None, None, 3.0, True].
2907+
assert pc.count(arr.slice(1, 4), mode="only_valid").as_py() == 2
2908+
assert pc.count(arr.slice(1, 4), mode="only_null").as_py() == 2
2909+
2910+
2911+
def test_count_dense_union_sliced_nulls():
2912+
# GH-50113: Sliced unions can report incorrect null counts in count.
2913+
arr = pa.UnionArray.from_dense(
2914+
pa.array([0, 1, 0, 0, 1, 1], type=pa.int8()),
2915+
pa.array([0, 0, 1, 2, 1, 2], type=pa.int32()),
2916+
[
2917+
pa.array([0.5, None, 3.0]),
2918+
pa.array([None, True, False]),
2919+
]
2920+
)
2921+
2922+
# Logical array: [0.5, None, None, 3.0, True, False].
2923+
assert pc.count(arr, mode="only_valid").as_py() == 4
2924+
assert pc.count(arr, mode="only_null").as_py() == 2
2925+
assert pc.count(arr, mode="all").as_py() == 6
2926+
# Logical slice: [None, None, 3.0, True].
2927+
assert pc.count(arr.slice(1, 4), mode="only_valid").as_py() == 2
2928+
assert pc.count(arr.slice(1, 4), mode="only_null").as_py() == 2
2929+
2930+
28922931
def test_index():
28932932
arr = pa.array([0, 1, None, 3, 4], type=pa.int64())
28942933
assert pc.index(arr, pa.scalar(0)).as_py() == 0

0 commit comments

Comments
 (0)