Skip to content

Commit b61301e

Browse files
committed
more tests and comments
Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 2589f6e commit b61301e

2 files changed

Lines changed: 30 additions & 0 deletions

File tree

cpp/src/arrow/util/byte_size.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,12 @@ struct GetByteRangesArray {
201201
RETURN_NOT_OK(range_offsets->Append(sizeof(c_type) * offset));
202202
RETURN_NOT_OK(range_lengths->Append(sizeof(c_type) * length));
203203

204+
// The following calculation is an over/under estimate of the size since views buffer
205+
// might
206+
// 1. Not reference all the values in data buffers (the array was filtered without gc)
207+
// 2. Reference a value multiple times without repeating it in the data buffer
208+
//
209+
// Producing exact byte size would require linear scan of all values in view buffer
204210
for (int i = 2; i < input.buffers.size(); i++) {
205211
const Buffer& buf = *input.buffers[i];
206212
RETURN_NOT_OK(range_starts->Append(reinterpret_cast<uint64_t>(buf.data())));
@@ -255,6 +261,12 @@ struct GetByteRangesArray {
255261
RETURN_NOT_OK(range_offsets->Append(sizeof(offset_type) * offset));
256262
RETURN_NOT_OK(range_lengths->Append(sizeof(offset_type) * length));
257263

264+
// The following calculation is an over/under estimate of the byte size since views
265+
// buffer might
266+
// 1. Not reference all the values in data buffers (the array was filtered without gc)
267+
// 2. Reference a value multiple times without repeating it in the data buffer
268+
//
269+
// Producing exact byte size would require linear scan of all values in view buffer
258270
GetByteRangesArray child{*input.child_data[0],
259271
0,
260272
(*input.child_data[0]).length,

cpp/src/arrow/util/byte_size_test.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,11 @@ TEST(ByteRanges, StringViewType) {
326326
std::shared_ptr<Array> str_view_with_nulls = ArrayFromJSON(
327327
utf8_view(), R"(["short", null, "another long string that requires a buffer"])");
328328
CheckBufferRanges(str_view_with_nulls, {{0, 0, 1}, {1, 0, 48}, {2, 0, 42}});
329+
330+
CheckBufferRanges(str_view_arr->Slice(1, 1), {{0, 16, 16}, {1, 0, 38}});
331+
CheckBufferRanges(str_view_arr->Slice(0, 1), {{0, 0, 16}, {1, 0, 38}});
332+
CheckBufferRanges(str_view_with_nulls->Slice(2, 1),
333+
{{0, 0, 1}, {1, 32, 16}, {2, 0, 42}});
329334
}
330335

331336
TEST(ByteRanges, BinaryViewType) {
@@ -337,6 +342,10 @@ TEST(ByteRanges, BinaryViewType) {
337342
std::shared_ptr<Array> bin_view_with_nulls =
338343
ArrayFromJSON(binary_view(), R"(["AB", null, "CDEFGHIJKLMNOPQRSTUVWXYZ"])");
339344
CheckBufferRanges(bin_view_with_nulls, {{0, 0, 1}, {1, 0, 48}, {2, 0, 24}});
345+
346+
CheckBufferRanges(bin_view_arr->Slice(1, 1), {{0, 16, 16}, {1, 0, 22}});
347+
CheckBufferRanges(bin_view_with_nulls->Slice(2, 1),
348+
{{0, 0, 1}, {1, 32, 16}, {2, 0, 24}});
340349
}
341350

342351
using ListViewArrowTypes = ::testing::Types<ListViewType, LargeListViewType>;
@@ -359,6 +368,15 @@ TYPED_TEST(ByteRangesListView, Basic) {
359368
{1, 0, 3 * sizeof(offset_type)},
360369
{2, 0, 3 * sizeof(offset_type)},
361370
{3, 0, 20}});
371+
CheckBufferRanges(list_view_arr->Slice(2, 1),
372+
{{0, 2 * sizeof(offset_type), sizeof(offset_type)},
373+
{1, 2 * sizeof(offset_type), sizeof(offset_type)},
374+
{2, 0, 16}});
375+
CheckBufferRanges(list_view_with_nulls->Slice(2, 1),
376+
{{0, 0, 1},
377+
{1, 2 * sizeof(offset_type), sizeof(offset_type)},
378+
{2, 2 * sizeof(offset_type), sizeof(offset_type)},
379+
{3, 0, 20}});
362380
}
363381

364382
TYPED_TEST(ByteRangesListView, NestedListView) {

0 commit comments

Comments
 (0)