Skip to content

Commit

Permalink
Simplifications
Browse files Browse the repository at this point in the history
  • Loading branch information
WillAyd committed Feb 12, 2025
1 parent 75ddddd commit 63bccb8
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 61 deletions.
32 changes: 10 additions & 22 deletions src/nanoarrow/common/array_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1581,10 +1581,6 @@ TEST(ArrayTest, ArrayTestAppendToListViewArray) {

EXPECT_EQ(ArrowArrayAppendEmpty(&array, 1), NANOARROW_OK);

ASSERT_EQ(ArrowArrayAppendInt(array.children[0], 42), NANOARROW_OK);
ASSERT_EQ(ArrowArrayAppendInt(array.children[0], 555), NANOARROW_OK);
EXPECT_EQ(ArrowArrayFinishElement(&array), NANOARROW_OK);

// Make sure number of children is checked at finish
array.n_children = 0;
EXPECT_EQ(ArrowArrayFinishBuildingDefault(&array, &error), EINVAL);
Expand All @@ -1597,7 +1593,7 @@ TEST(ArrayTest, ArrayTestAppendToListViewArray) {
EXPECT_EQ(ArrowArrayFinishBuilding(&array, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
EXPECT_STREQ(ArrowErrorMessage(&error),
"Offset: 3 + size: 2 at index: 4 exceeds length of child view: 4");
"Offset: 1 + size: 2 at index: 2 exceeds length of child view: 2");
array.children[0]->length = array.children[0]->length + 1;

EXPECT_EQ(ArrowArrayFinishBuildingDefault(&array, &error), NANOARROW_OK);
Expand All @@ -1606,10 +1602,10 @@ TEST(ArrayTest, ArrayTestAppendToListViewArray) {
auto arrow_array = ImportArray(&array, &schema);
ARROW_EXPECT_OK(arrow_array);

constexpr size_t nelems = 5;
const std::array<int32_t, nelems> offsets = {0, 1, 1, 3, 3};
const std::array<int32_t, nelems> sizes = {1, 0, 2, 0, 2};
const std::array<uint8_t, nelems> valid_bytes = {1, 0, 1, 1, 1};
constexpr size_t nelems = 4;
const std::array<int32_t, nelems> offsets = {0, 1, 1, 3};
const std::array<int32_t, nelems> sizes = {1, 0, 2, 0};
const std::array<uint8_t, nelems> valid_bytes = {1, 0, 1, 1};

auto child_builder = std::make_shared<Int64Builder>();
auto builder =
Expand All @@ -1619,8 +1615,6 @@ TEST(ArrayTest, ArrayTestAppendToListViewArray) {
ARROW_EXPECT_OK(child_builder->Append(123));
ARROW_EXPECT_OK(child_builder->Append(456));
ARROW_EXPECT_OK(child_builder->Append(789));
ARROW_EXPECT_OK(child_builder->Append(42));
ARROW_EXPECT_OK(child_builder->Append(555));
auto expected_array = builder.Finish();
ARROW_EXPECT_OK(expected_array);

Expand Down Expand Up @@ -1658,10 +1652,6 @@ TEST(ArrayTest, ArrayTestAppendToLargeListViewArray) {

EXPECT_EQ(ArrowArrayAppendEmpty(&array, 1), NANOARROW_OK);

ASSERT_EQ(ArrowArrayAppendInt(array.children[0], 42), NANOARROW_OK);
ASSERT_EQ(ArrowArrayAppendInt(array.children[0], 555), NANOARROW_OK);
EXPECT_EQ(ArrowArrayFinishElement(&array), NANOARROW_OK);

// Make sure number of children is checked at finish
array.n_children = 0;
EXPECT_EQ(ArrowArrayFinishBuildingDefault(&array, &error), EINVAL);
Expand All @@ -1674,7 +1664,7 @@ TEST(ArrayTest, ArrayTestAppendToLargeListViewArray) {
EXPECT_EQ(ArrowArrayFinishBuilding(&array, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
EXPECT_STREQ(ArrowErrorMessage(&error),
"Offset: 3 + size: 2 at index: 4 exceeds length of child view: 4");
"Offset: 1 + size: 2 at index: 2 exceeds length of child view: 2");
array.children[0]->length = array.children[0]->length + 1;

EXPECT_EQ(ArrowArrayFinishBuildingDefault(&array, &error), NANOARROW_OK);
Expand All @@ -1683,10 +1673,10 @@ TEST(ArrayTest, ArrayTestAppendToLargeListViewArray) {
auto arrow_array = ImportArray(&array, &schema);
ARROW_EXPECT_OK(arrow_array);

constexpr size_t nelems = 5;
const std::array<int64_t, nelems> offsets = {0, 1, 1, 3, 3};
const std::array<int64_t, nelems> sizes = {1, 0, 2, 0, 2};
const std::array<uint8_t, nelems> valid_bytes = {1, 0, 1, 1, 1};
constexpr size_t nelems = 4;
const std::array<int64_t, nelems> offsets = {0, 1, 1, 3};
const std::array<int64_t, nelems> sizes = {1, 0, 2, 0};
const std::array<uint8_t, nelems> valid_bytes = {1, 0, 1, 1};

auto child_builder = std::make_shared<Int64Builder>();
auto builder =
Expand All @@ -1696,8 +1686,6 @@ TEST(ArrayTest, ArrayTestAppendToLargeListViewArray) {
ARROW_EXPECT_OK(child_builder->Append(123));
ARROW_EXPECT_OK(child_builder->Append(456));
ARROW_EXPECT_OK(child_builder->Append(789));
ARROW_EXPECT_OK(child_builder->Append(42));
ARROW_EXPECT_OK(child_builder->Append(555));
auto expected_array = builder.Finish();
ARROW_EXPECT_OK(expected_array);

Expand Down
50 changes: 11 additions & 39 deletions src/nanoarrow/common/inline_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -801,50 +801,22 @@ static inline ArrowErrorCode ArrowArrayFinishElement(struct ArrowArray* array) {
return EOVERFLOW;
}

const int32_t last_valid_pos = (int32_t)private_data->list_view_offset;
int32_t offset;
if (array->length == 0) {
offset = 0;
} else {
const struct ArrowBuffer *offsets_buf, *sizes_buf;
offsets_buf = ArrowArrayBuffer(array, 1);
sizes_buf = ArrowArrayBuffer(array, 2);
NANOARROW_DCHECK(offsets_buf->size_bytes == sizes_buf->size_bytes);
struct ArrowBufferView offsets, sizes;
offsets.data.data = offsets_buf->data;
sizes.data.data = sizes_buf->data;
const int32_t prev_offset = offsets.data.as_int32[last_valid_pos];
const int32_t prev_size = sizes.data.as_int32[last_valid_pos];
offset = prev_offset + prev_size;
}
NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt32(ArrowArrayBuffer(array, 1), offset));
NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt32(ArrowArrayBuffer(array, 2),
(int32_t)child_length - offset));
private_data->list_view_offset = (int32_t)array->length;
const int32_t last_valid_offset = (int32_t)private_data->list_view_offset;
NANOARROW_RETURN_NOT_OK(
ArrowBufferAppendInt32(ArrowArrayBuffer(array, 1), last_valid_offset));
NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt32(
ArrowArrayBuffer(array, 2), (int32_t)child_length - last_valid_offset));
private_data->list_view_offset = (int32_t)child_length;
break;
}
case NANOARROW_TYPE_LARGE_LIST_VIEW: {
child_length = array->children[0]->length;
const int32_t last_valid_pos = (int32_t)private_data->list_view_offset;
int64_t offset;
if (array->length == 0) {
offset = 0;
} else {
const struct ArrowBuffer *offsets_buf, *sizes_buf;
offsets_buf = ArrowArrayBuffer(array, 1);
sizes_buf = ArrowArrayBuffer(array, 2);
NANOARROW_DCHECK(offsets_buf->size_bytes == sizes_buf->size_bytes);
struct ArrowBufferView offsets, sizes;
offsets.data.data = offsets_buf->data;
sizes.data.data = sizes_buf->data;
const int64_t prev_offset = offsets.data.as_int64[last_valid_pos];
const int64_t prev_size = sizes.data.as_int64[last_valid_pos];
offset = prev_offset + prev_size;
}
NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt64(ArrowArrayBuffer(array, 1), offset));
const int64_t last_valid_offset = private_data->list_view_offset;
NANOARROW_RETURN_NOT_OK(
ArrowBufferAppendInt64(ArrowArrayBuffer(array, 2), child_length - offset));
private_data->list_view_offset = array->length;
ArrowBufferAppendInt64(ArrowArrayBuffer(array, 1), last_valid_offset));
NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt64(ArrowArrayBuffer(array, 2),
child_length - last_valid_offset));
private_data->list_view_offset = child_length;
break;
}

Expand Down

0 comments on commit 63bccb8

Please sign in to comment.