From f0761a2bcdcef31d2cc6669df10776a72fb51f0c Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Fri, 22 Aug 2025 19:44:13 -0400 Subject: [PATCH 1/8] Implement GetByteRangesArray for view types --- cpp/src/arrow/util/byte_size.cc | 53 +++++++++++++++++++++++++ cpp/src/arrow/util/byte_size_test.cc | 59 ++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 53382a107759..847edb10cc53 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -192,6 +192,29 @@ struct GetByteRangesArray { Status Visit(const LargeBinaryType& type) const { return VisitBaseBinary(type); } + template + Status VisitBaseViewType(const BaseViewType& type) const { + using c_type = typename BaseViewType::c_type; + RETURN_NOT_OK(VisitBitmap(input.buffers[0])); + const Buffer& views = *input.buffers[1]; + RETURN_NOT_OK(range_starts->Append(reinterpret_cast(views.data()))); + RETURN_NOT_OK(range_offsets->Append(sizeof(c_type) * offset)); + RETURN_NOT_OK(range_lengths->Append(sizeof(c_type) * length)); + + for (std::size_t i = 2; i < input.buffers.size(); i++) { + const Buffer& buf = *input.buffers[i]; + RETURN_NOT_OK(range_starts->Append(reinterpret_cast(buf.data()))); + RETURN_NOT_OK(range_offsets->Append(0)); + RETURN_NOT_OK(range_lengths->Append(static_cast(buf.size()))); + } + + return Status::OK(); + } + + Status Visit(const StringViewType& type) const { return VisitBaseViewType(type); } + + Status Visit(const BinaryViewType& type) const { return VisitBaseViewType(type); } + template Status VisitBaseList(const BaseListType& type) const { using offset_type = typename BaseListType::offset_type; @@ -215,6 +238,36 @@ struct GetByteRangesArray { Status Visit(const LargeListType& type) const { return VisitBaseList(type); } + template + Status VisitBaseListView(const BaseListViewType& type) const { + using offset_type = typename BaseListViewType::offset_type; + RETURN_NOT_OK(VisitBitmap(input.buffers[0])); + + const Buffer& offsets_buffer = *input.buffers[1]; + RETURN_NOT_OK( + range_starts->Append(reinterpret_cast(offsets_buffer.data()))); + RETURN_NOT_OK(range_offsets->Append(sizeof(offset_type) * offset)); + RETURN_NOT_OK(range_lengths->Append(sizeof(offset_type) * length)); + + const Buffer& lengths_buffer = *input.buffers[2]; + RETURN_NOT_OK( + range_starts->Append(reinterpret_cast(lengths_buffer.data()))); + RETURN_NOT_OK(range_offsets->Append(sizeof(offset_type) * offset)); + RETURN_NOT_OK(range_lengths->Append(sizeof(offset_type) * length)); + + GetByteRangesArray child{*input.child_data[0], + 0, + (*input.child_data[0]).length, + range_starts, + range_offsets, + range_lengths}; + return VisitTypeInline(*type.value_type(), &child); + } + + Status Visit(const ListViewType& type) const { return VisitBaseListView(type); } + + Status Visit(const LargeListViewType& type) const { return VisitBaseListView(type); } + Status Visit(const FixedSizeListType& type) const { RETURN_NOT_OK(VisitBitmap(input.buffers[0])); GetByteRangesArray child{*input.child_data[0], diff --git a/cpp/src/arrow/util/byte_size_test.cc b/cpp/src/arrow/util/byte_size_test.cc index 0aaf0a76a2a4..d5c6c37a751b 100644 --- a/cpp/src/arrow/util/byte_size_test.cc +++ b/cpp/src/arrow/util/byte_size_test.cc @@ -316,6 +316,65 @@ TEST(ByteRanges, FixedSizeList) { CheckBufferRanges(list_arr_with_nulls->Slice(4, 1), {{0, 0, 1}, {1, 1, 1}, {2, 8, 2}}); } +TEST(ByteRanges, StringViewType) { + std::shared_ptr str_view_arr = ArrayFromJSON( + utf8_view(), R"(["short", "a longer string that requires a buffer"])"); + // First buffer is validity (null in this case), second is views (16 bytes per view), + // third is the data buffer for long strings + CheckBufferRanges(str_view_arr, {{0, 0, 32}, {1, 0, 38}}); + + std::shared_ptr str_view_with_nulls = ArrayFromJSON( + utf8_view(), R"(["short", null, "another long string that requires a buffer"])"); + CheckBufferRanges(str_view_with_nulls, {{0, 0, 1}, {1, 0, 48}, {2, 0, 42}}); +} + +TEST(ByteRanges, BinaryViewType) { + std::shared_ptr bin_view_arr = + ArrayFromJSON(binary_view(), R"(["ABCD", "EFGHIJKLMNOPQRSTUVWXYZ"])"); + // Similar to string view: views buffer (16 bytes per view), then data buffers + CheckBufferRanges(bin_view_arr, {{0, 0, 32}, {1, 0, 22}}); + + std::shared_ptr bin_view_with_nulls = + ArrayFromJSON(binary_view(), R"(["AB", null, "CDEFGHIJKLMNOPQRSTUVWXYZ"])"); + CheckBufferRanges(bin_view_with_nulls, {{0, 0, 1}, {1, 0, 48}, {2, 0, 24}}); +} + +using ListViewArrowTypes = ::testing::Types; +template +class ByteRangesListView : public ::testing::Test {}; +TYPED_TEST_SUITE(ByteRangesListView, ListViewArrowTypes); + +TYPED_TEST(ByteRangesListView, Basic) { + using offset_type = typename TypeParam::offset_type; + std::shared_ptr type = std::make_shared(int32()); + std::shared_ptr list_view_arr = ArrayFromJSON(type, "[[1, 2], [3], [0]]"); + // Offsets buffer, sizes buffer, then child data + CheckBufferRanges( + list_view_arr, + {{0, 0, 3 * sizeof(offset_type)}, {1, 0, 3 * sizeof(offset_type)}, {2, 0, 16}}); + + std::shared_ptr list_view_with_nulls = + ArrayFromJSON(type, "[[1, 2], null, [3, 4, 5]]"); + CheckBufferRanges(list_view_with_nulls, {{0, 0, 1}, + {1, 0, 3 * sizeof(offset_type)}, + {2, 0, 3 * sizeof(offset_type)}, + {3, 0, 20}}); +} + +TYPED_TEST(ByteRangesListView, NestedListView) { + using offset_type = typename TypeParam::offset_type; + std::shared_ptr type = + std::make_shared(std::make_shared(int32())); + std::shared_ptr list_view_arr = + ArrayFromJSON(type, "[[[1], [2, 3]], [[4, 5, 6]], [[7]]]"); + // Parent offsets, parent sizes, child offsets, child sizes, grandchild data + CheckBufferRanges(list_view_arr, {{0, 0, 3 * sizeof(offset_type)}, + {1, 0, 3 * sizeof(offset_type)}, + {2, 0, 4 * sizeof(offset_type)}, + {3, 0, 4 * sizeof(offset_type)}, + {4, 0, 28}}); +} + TEST(ByteRanges, Map) { std::shared_ptr map_arr = ArrayFromJSON( map(utf8(), uint16()), R"([[["x", 1], ["y", 2]], [["x", 3], ["y", 4]]])"); From 1cab5a0440736c6e1b6eaed3eb2b0614a1a7ec89 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 11:17:13 +0000 Subject: [PATCH 2/8] Update cpp/src/arrow/util/byte_size.cc Co-authored-by: Rok Mihevc --- cpp/src/arrow/util/byte_size.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 847edb10cc53..9bae4cbc6219 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -201,7 +201,7 @@ struct GetByteRangesArray { RETURN_NOT_OK(range_offsets->Append(sizeof(c_type) * offset)); RETURN_NOT_OK(range_lengths->Append(sizeof(c_type) * length)); - for (std::size_t i = 2; i < input.buffers.size(); i++) { + for (int i = 2; i < input.buffers.size(); i++) { const Buffer& buf = *input.buffers[i]; RETURN_NOT_OK(range_starts->Append(reinterpret_cast(buf.data()))); RETURN_NOT_OK(range_offsets->Append(0)); From bfa5c33c3ab7095addaff8acb37292499870229d Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 11:48:56 +0000 Subject: [PATCH 3/8] more tests and comments Signed-off-by: Robert Kruszewski --- cpp/src/arrow/util/byte_size.cc | 12 ++++++++++++ cpp/src/arrow/util/byte_size_test.cc | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 9bae4cbc6219..88d0044f0740 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -201,6 +201,12 @@ struct GetByteRangesArray { RETURN_NOT_OK(range_offsets->Append(sizeof(c_type) * offset)); RETURN_NOT_OK(range_lengths->Append(sizeof(c_type) * length)); + // The following calculation is an over/under estimate of the size since views buffer + // might + // 1. Not reference all the values in data buffers (the array was filtered without gc) + // 2. Reference a value multiple times without repeating it in the data buffer + // + // Producing exact byte size would require linear scan of all values in view buffer for (int i = 2; i < input.buffers.size(); i++) { const Buffer& buf = *input.buffers[i]; RETURN_NOT_OK(range_starts->Append(reinterpret_cast(buf.data()))); @@ -255,6 +261,12 @@ struct GetByteRangesArray { RETURN_NOT_OK(range_offsets->Append(sizeof(offset_type) * offset)); RETURN_NOT_OK(range_lengths->Append(sizeof(offset_type) * length)); + // The following calculation is an over/under estimate of the byte size since views + // buffer might + // 1. Not reference all the values in data buffers (the array was filtered without gc) + // 2. Reference a value multiple times without repeating it in the data buffer + // + // Producing exact byte size would require linear scan of all values in view buffer GetByteRangesArray child{*input.child_data[0], 0, (*input.child_data[0]).length, diff --git a/cpp/src/arrow/util/byte_size_test.cc b/cpp/src/arrow/util/byte_size_test.cc index d5c6c37a751b..e8e9748ecd10 100644 --- a/cpp/src/arrow/util/byte_size_test.cc +++ b/cpp/src/arrow/util/byte_size_test.cc @@ -326,6 +326,11 @@ TEST(ByteRanges, StringViewType) { std::shared_ptr str_view_with_nulls = ArrayFromJSON( utf8_view(), R"(["short", null, "another long string that requires a buffer"])"); CheckBufferRanges(str_view_with_nulls, {{0, 0, 1}, {1, 0, 48}, {2, 0, 42}}); + + CheckBufferRanges(str_view_arr->Slice(1, 1), {{0, 16, 16}, {1, 0, 38}}); + CheckBufferRanges(str_view_arr->Slice(0, 1), {{0, 0, 16}, {1, 0, 38}}); + CheckBufferRanges(str_view_with_nulls->Slice(2, 1), + {{0, 0, 1}, {1, 32, 16}, {2, 0, 42}}); } TEST(ByteRanges, BinaryViewType) { @@ -337,6 +342,10 @@ TEST(ByteRanges, BinaryViewType) { std::shared_ptr bin_view_with_nulls = ArrayFromJSON(binary_view(), R"(["AB", null, "CDEFGHIJKLMNOPQRSTUVWXYZ"])"); CheckBufferRanges(bin_view_with_nulls, {{0, 0, 1}, {1, 0, 48}, {2, 0, 24}}); + + CheckBufferRanges(bin_view_arr->Slice(1, 1), {{0, 16, 16}, {1, 0, 22}}); + CheckBufferRanges(bin_view_with_nulls->Slice(2, 1), + {{0, 0, 1}, {1, 32, 16}, {2, 0, 24}}); } using ListViewArrowTypes = ::testing::Types; @@ -359,6 +368,15 @@ TYPED_TEST(ByteRangesListView, Basic) { {1, 0, 3 * sizeof(offset_type)}, {2, 0, 3 * sizeof(offset_type)}, {3, 0, 20}}); + CheckBufferRanges(list_view_arr->Slice(2, 1), + {{0, 2 * sizeof(offset_type), sizeof(offset_type)}, + {1, 2 * sizeof(offset_type), sizeof(offset_type)}, + {2, 0, 16}}); + CheckBufferRanges(list_view_with_nulls->Slice(2, 1), + {{0, 0, 1}, + {1, 2 * sizeof(offset_type), sizeof(offset_type)}, + {2, 2 * sizeof(offset_type), sizeof(offset_type)}, + {3, 0, 20}}); } TYPED_TEST(ByteRangesListView, NestedListView) { From db8feb5d241c6cd64c0cc09d0bf04795cca5d36b Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 11:54:29 +0000 Subject: [PATCH 4/8] type Signed-off-by: Robert Kruszewski --- cpp/src/arrow/util/byte_size.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 88d0044f0740..51cb64ca23c0 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -207,7 +207,7 @@ struct GetByteRangesArray { // 2. Reference a value multiple times without repeating it in the data buffer // // Producing exact byte size would require linear scan of all values in view buffer - for (int i = 2; i < input.buffers.size(); i++) { + for (size_t i = 2; i < input.buffers.size(); i++) { const Buffer& buf = *input.buffers[i]; RETURN_NOT_OK(range_starts->Append(reinterpret_cast(buf.data()))); RETURN_NOT_OK(range_offsets->Append(0)); From f79a3d837570d6bc3baff6748d391f0442a60b87 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 13:32:46 +0000 Subject: [PATCH 5/8] Update cpp/src/arrow/util/byte_size.cc Co-authored-by: Rok Mihevc --- cpp/src/arrow/util/byte_size.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 51cb64ca23c0..e0d86de362b0 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -269,7 +269,7 @@ struct GetByteRangesArray { // Producing exact byte size would require linear scan of all values in view buffer GetByteRangesArray child{*input.child_data[0], 0, - (*input.child_data[0]).length, + input.child_data[0]->length, range_starts, range_offsets, range_lengths}; From d6ad32c06d483e07d92a45f0183ab3e6de0587cd Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 13:35:05 +0000 Subject: [PATCH 6/8] Update cpp/src/arrow/util/byte_size.cc Co-authored-by: Rok Mihevc --- cpp/src/arrow/util/byte_size.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index e0d86de362b0..809697306b1b 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -201,7 +201,7 @@ struct GetByteRangesArray { RETURN_NOT_OK(range_offsets->Append(sizeof(c_type) * offset)); RETURN_NOT_OK(range_lengths->Append(sizeof(c_type) * length)); - // The following calculation is an over/under estimate of the size since views buffer + // The following calculation is an over estimate of the size since views buffer // might // 1. Not reference all the values in data buffers (the array was filtered without gc) // 2. Reference a value multiple times without repeating it in the data buffer From 70b4f6d389b6deaff346f628bb5d1260fbf96326 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 13:35:13 +0000 Subject: [PATCH 7/8] Update cpp/src/arrow/util/byte_size.cc Co-authored-by: Rok Mihevc --- cpp/src/arrow/util/byte_size.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 809697306b1b..12f947d23b36 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -261,7 +261,7 @@ struct GetByteRangesArray { RETURN_NOT_OK(range_offsets->Append(sizeof(offset_type) * offset)); RETURN_NOT_OK(range_lengths->Append(sizeof(offset_type) * length)); - // The following calculation is an over/under estimate of the byte size since views + // The following calculation is an over estimate of the byte size since views // buffer might // 1. Not reference all the values in data buffers (the array was filtered without gc) // 2. Reference a value multiple times without repeating it in the data buffer From f6e0026ed75ea72e5da78b4c10de4ed78959114c Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 3 Mar 2026 14:26:03 +0000 Subject: [PATCH 8/8] reformat Signed-off-by: Robert Kruszewski --- cpp/src/arrow/util/byte_size.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/util/byte_size.cc b/cpp/src/arrow/util/byte_size.cc index 12f947d23b36..783056f7d9f4 100644 --- a/cpp/src/arrow/util/byte_size.cc +++ b/cpp/src/arrow/util/byte_size.cc @@ -267,12 +267,9 @@ struct GetByteRangesArray { // 2. Reference a value multiple times without repeating it in the data buffer // // Producing exact byte size would require linear scan of all values in view buffer - GetByteRangesArray child{*input.child_data[0], - 0, - input.child_data[0]->length, - range_starts, - range_offsets, - range_lengths}; + GetByteRangesArray child{ + *input.child_data[0], 0, input.child_data[0]->length, range_starts, range_offsets, + range_lengths}; return VisitTypeInline(*type.value_type(), &child); }