shangxinli commented on code in PR #1039:
URL: https://github.com/apache/iceberg-cpp/pull/1039#discussion_r4328472060
##########
src/iceberg/row/arrow_array_wrapper.cc:
##########
@@ -365,12 +367,17 @@ class ArrowArrayMapLike::Impl {
// XXX: ArrowArrayViewListChildOffset does not work for map types.
// We need to directly access the offsets buffer instead.
auto* offsets_buffer = array_view_.buffer_views[1].data.as_int32;
- offset_ = offsets_buffer[row_index_];
- length_ = offsets_buffer[row_index_ + 1] - offset_;
+ const int64_t row = array_view_.offset + row_index_;
+ offset_ = offsets_buffer[row];
Review Comment:
The XXX note above is stale - ArrowArrayViewListChildOffset does handle
NANOARROW_TYPE_MAP, see inline_array.h 1114-1120. The helper would drop exactly
the arithmetic this PR is fixing.
##########
src/iceberg/test/struct_like_test.cc:
##########
@@ -663,4 +666,120 @@ TEST(ArrowArrayStructLike, Accessor) {
}
}
+TEST(ArrowArrayStructLike, SlicedStruct) {
+ auto person_type =
+ ::arrow::struct_({::arrow::field("name", ::arrow::utf8(),
/*nullable=*/false)});
+ auto root_type = ::arrow::struct_(
+ {::arrow::field("id", ::arrow::int64(), /*nullable=*/false),
+ ::arrow::field("person", person_type, /*nullable=*/false),
+ ::arrow::field("tags", ::arrow::list(::arrow::int32()),
/*nullable=*/false)});
+
+ auto arrow_array = ::arrow::json::ArrayFromJSONString(root_type, R"([
+ {"id": 1, "person": {"name": "Alice"}, "tags": [1]},
+ {"id": 2, "person": {"name": "Bob"}, "tags": [2, 3]},
+ {"id": 3, "person": {"name": "Carol"}, "tags": []}])")
+ .ValueOrDie();
+ auto sliced = arrow_array->Slice(1);
+
+ ArrowSchema c_schema;
+ ArrowArray c_array;
+ internal::ArrowSchemaGuard schema_guard(&c_schema);
+ internal::ArrowArrayGuard array_guard(&c_array);
+ ASSERT_TRUE(::arrow::ExportType(*root_type, &c_schema).ok());
+ ASSERT_TRUE(::arrow::ExportArray(*sliced, &c_array).ok());
+ ASSERT_EQ(c_array.offset, 1);
Review Comment:
This slices only the parent, so no child ever carries its own offset. The
map test asserts children[0]->offset is 1 - worth the same here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
