Skip to content

fix(row): apply array offsets in ArrowArray wrappers - #1039

Merged
wgtmac merged 1 commit into
apache:mainfrom
YinZheng-Sun:fix/arrow-array-wrapper-sliced-offset
Oct 21, 2026
Merged

wgtmac merged 1 commit into
apache:mainfrom
YinZheng-Sun:fix/arrow-array-wrapper-sliced-offset

Conversation

@YinZheng-Sun

Copy link
Copy Markdown
Contributor

ArrowArrayStructLike, ArrowArrayArrayLike and ArrowArrayMapLike ignored the offset of the wrapped ArrowArray, so sliced inputs returned wrong rows:

  • Struct children are not shifted by the parent's offset in the Arrow C data interface, so GetField must add the struct's own offset.
  • ArrowArrayViewListChildOffset and the raw map offsets buffer are indexed without the array's offset, so list/map rows must add it before lookup.
  • Map key/value children are not shifted by the entries struct's offset.

Add tests for sliced struct, list and map arrays.

ArrowArrayStructLike, ArrowArrayArrayLike and ArrowArrayMapLike ignored the
offset of the wrapped ArrowArray, so sliced inputs returned wrong rows:

- Struct children are not shifted by the parent's offset in the Arrow C data
  interface, so GetField must add the struct's own offset.
- ArrowArrayViewListChildOffset and the raw map offsets buffer are indexed
  without the array's offset, so list/map rows must add it before lookup.
- Map key/value children are not shifted by the entries struct's offset.

Add tests for sliced struct, list and map arrays.
@YinZheng-Sun
YinZheng-Sun force-pushed the fix/arrow-array-wrapper-sliced-offset branch from 4a9aab8 to 7c30438 Compare October 19, 2026 03:58
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];

@shangxinli shangxinli Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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);

@shangxinli shangxinli Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@wgtmac wgtmac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@wgtmac
wgtmac merged commit 08d2f86 into apache:main Oct 21, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants