From e8166b90bc5e03d6c85ce2ac06db7bcc1b1f11f5 Mon Sep 17 00:00:00 2001 From: JonSnow1807 Date: Wed, 21 Oct 2026 16:48:39 -0500 Subject: [PATCH] GH-54937: [C++] Fix DenseUnionArray::Make for sliced type_ids and value_offsets arrays DenseUnionArray::Make used type_ids.offset() as the offset of the union array for both the type ids and the value offsets buffers, and never checked the length of value_offsets. With a value_offsets array sliced differently from type_ids the union read value offsets from the wrong positions (which ValidateFull cannot detect), and a value_offsets array shorter than type_ids was accepted. Reject value_offsets arrays shorter than type_ids, slice both buffers to their logical windows (zero-copy) when the inputs are sliced, and build the union with offset 0. Unsliced inputs keep using the original buffers. --- cpp/src/arrow/array/array_nested.cc | 32 +++++++++++++++++++++---- cpp/src/arrow/array/array_union_test.cc | 31 ++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/array/array_nested.cc b/cpp/src/arrow/array/array_nested.cc index 6849aca089b7..8d0bc3f453da 100644 --- a/cpp/src/arrow/array/array_nested.cc +++ b/cpp/src/arrow/array/array_nested.cc @@ -1380,13 +1380,35 @@ Result> DenseUnionArray::Make( return Status::Invalid("type_codes must have the same length as children"); } - BufferVector buffers = {nullptr, checked_cast(type_ids).values(), - checked_cast(value_offsets).values()}; + // Longer value_offsets arrays are tolerated: only the first len(type_ids) + // offsets are used (and existing callers rely on that). + if (value_offsets.length() < type_ids.length()) { + return Status::Invalid( + "Dense UnionArray value_offsets must have at least the same length as type_ids"); + } + + // `type_ids` and `value_offsets` may be sliced differently, so slice their + // buffers (zero-copy) to their logical windows and build the union with + // offset 0. Unsliced inputs keep using the original buffers. + 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()); + } + std::shared_ptr value_offsets_buffer = + checked_cast(value_offsets).values(); + if (value_offsets.offset() != 0) { + value_offsets_buffer = SliceBuffer(std::move(value_offsets_buffer), + value_offsets.offset() * sizeof(int32_t), + value_offsets.length() * sizeof(int32_t)); + } + BufferVector buffers = {nullptr, std::move(type_ids_buffer), + std::move(value_offsets_buffer)}; auto union_type = dense_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()); } diff --git a/cpp/src/arrow/array/array_union_test.cc b/cpp/src/arrow/array/array_union_test.cc index ec93329d0839..c373d210fa9d 100644 --- a/cpp/src/arrow/array/array_union_test.cc +++ b/cpp/src/arrow/array/array_union_test.cc @@ -425,6 +425,37 @@ TEST_F(TestUnionArrayFactories, TestMakeSparse) { ASSERT_RAISES(Invalid, SparseUnionArray::Make(*type_ids_, children)); } +TEST_F(TestUnionArrayFactories, TestMakeDenseSliced) { + // type_ids unsliced, value_offsets sliced + auto type_ids = ArrayFromJSON(int8(), "[0, 0]"); + auto child = ArrayFromJSON(int64(), "[10, 20, 30]"); + auto value_offsets = ArrayFromJSON(int32(), "[0, 1, 2]")->Slice(1, 2); + ASSERT_OK_AND_ASSIGN(auto arr, + DenseUnionArray::Make(*type_ids, *value_offsets, {child})); + ASSERT_OK(arr->ValidateFull()); + auto expected = ArrayFromJSON(arr->type(), R"([[0, 20], [0, 30]])"); + AssertArraysEqual(*expected, *arr, /*verbose=*/true); + + // type_ids sliced, value_offsets unsliced + auto sliced_type_ids = ArrayFromJSON(int8(), "[1, 0, 0]")->Slice(1, 2); + auto plain_value_offsets = ArrayFromJSON(int32(), "[1, 2]"); + ASSERT_OK_AND_ASSIGN( + arr, DenseUnionArray::Make(*sliced_type_ids, *plain_value_offsets, {child})); + ASSERT_OK(arr->ValidateFull()); + AssertArraysEqual(*expected, *arr, /*verbose=*/true); + + // both inputs sliced + auto sliced_value_offsets = ArrayFromJSON(int32(), "[9, 1, 2]")->Slice(1, 2); + ASSERT_OK_AND_ASSIGN( + arr, DenseUnionArray::Make(*sliced_type_ids, *sliced_value_offsets, {child})); + ASSERT_OK(arr->ValidateFull()); + AssertArraysEqual(*expected, *arr, /*verbose=*/true); + + // value_offsets shorter than type_ids is rejected + auto short_offsets = ArrayFromJSON(int32(), "[0, 1]")->Slice(0, 1); + ASSERT_RAISES(Invalid, DenseUnionArray::Make(*type_ids, *short_offsets, {child})); +} + template class UnionBuilderTest : public ::testing::Test { public: