diff --git a/cpp/src/arrow/array/array_nested.cc b/cpp/src/arrow/array/array_nested.cc index 6849aca089b7..5e78023b5ecd 100644 --- a/cpp/src/arrow/array/array_nested.cc +++ b/cpp/src/arrow/array/array_nested.cc @@ -1412,11 +1412,21 @@ Result> SparseUnionArray::Make( return Status::Invalid("type_codes must have the same length as children"); } - BufferVector buffers = {nullptr, checked_cast(type_ids).values()}; + // The union's offset applies to the children as well as to the type ids, but + // the children are required to have the same length as `type_ids`, i.e. they + // are aligned with the logical positions of `type_ids`. Hence, if `type_ids` + // is sliced, slice its buffer (zero-copy) to the logical window and build the + // union with offset 0 (GH-54907). + std::shared_ptr type_ids_buffer = + checked_cast(type_ids).values(); + if (type_ids.offset() != 0) { + type_ids_buffer = + SliceBuffer(std::move(type_ids_buffer), type_ids.offset(), type_ids.length()); + } + BufferVector buffers = {nullptr, std::move(type_ids_buffer)}; auto union_type = sparse_union(children, std::move(field_names), std::move(type_codes)); - auto internal_data = - ArrayData::Make(std::move(union_type), type_ids.length(), std::move(buffers), - /*null_count=*/0, type_ids.offset()); + auto internal_data = ArrayData::Make(std::move(union_type), type_ids.length(), + std::move(buffers), /*null_count=*/0); for (const auto& child : children) { internal_data->child_data.push_back(child->data()); if (child->length() != type_ids.length()) { diff --git a/cpp/src/arrow/array/array_union_test.cc b/cpp/src/arrow/array/array_union_test.cc index ec93329d0839..169205d4d490 100644 --- a/cpp/src/arrow/array/array_union_test.cc +++ b/cpp/src/arrow/array/array_union_test.cc @@ -179,6 +179,34 @@ TEST(TestSparseUnionArray, SliceIsNull) { check_slice(sliced_children_1_4, {false, false, true, false}); } +TEST(TestSparseUnionArray, MakeFromSlicedTypeIds) { + // GH-54907: SparseUnionArray::Make used type_ids.offset() as the offset of the + // union array, which also shifts the positions looked up in the children. + auto type_ids = ArrayFromJSON(int8(), "[1, 0, 1, 0, 0]")->Slice(1, 3); + ASSERT_EQ(type_ids->offset(), 1); + auto ints = ArrayFromJSON(int64(), "[10, 20, 30]"); + auto strs = ArrayFromJSON(utf8(), R"(["a", "b", "c"])"); + + ASSERT_OK_AND_ASSIGN(auto arr, SparseUnionArray::Make(*type_ids, {ints, strs})); + ASSERT_OK(arr->ValidateFull()); + ASSERT_EQ(arr->length(), 3); + auto expected = ArrayFromJSON(arr->type(), R"([[0, 10], [1, "b"], [0, 30]])"); + AssertArraysEqual(*expected, *arr, /*verbose=*/true); + + // Children that are themselves sliced compose with the sliced type ids. + auto sliced_ints = ArrayFromJSON(int64(), "[999, 10, 20, 30]")->Slice(1); + auto sliced_strs = ArrayFromJSON(utf8(), R"(["z", "a", "b", "c"])")->Slice(1); + ASSERT_OK_AND_ASSIGN(auto arr_sliced_children, + SparseUnionArray::Make(*type_ids, {sliced_ints, sliced_strs})); + ASSERT_OK(arr_sliced_children->ValidateFull()); + AssertArraysEqual(*expected, *arr_sliced_children, /*verbose=*/true); + + // Children must still match the length of the (sliced) type ids. + auto long_ints = ArrayFromJSON(int64(), "[10, 20, 30, 40]"); + auto long_strs = ArrayFromJSON(utf8(), R"(["a", "b", "c", "d"])"); + ASSERT_RAISES(Invalid, SparseUnionArray::Make(*type_ids, {long_ints, long_strs})); +} + TEST(TestSparseUnionArray, Validate) { auto a = ArrayFromJSON(int32(), "[4, 5]"); auto type = sparse_union({field("a", int32())});