You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2026/08/25 13:40:34 UTC
[PR] docs(arrow-data): Document how `ArrayData::offset` applies to buffers, child_data and nulls [arrow-rs]
alamb opened a new pull request, #10838:
URL: https://github.com/apache/arrow-rs/pull/10838
# Which issue does this PR close?
- Part of #7595.
# Rationale for this change
The invariants governing `ArrayData::offset` — that it applies to `buffers` and `child_data` (composing cumulatively with any offset a child itself has) but NOT to `nulls` — are currently documented only on the **private fields** of `ArrayData`, so they never appear in rendered rustdoc. This makes it hard to reason about what a correct fix looks like when offset-handling bugs come up (e.g. #7595 / #7750 and the review of #10835, where the correct output representation of `ArrayData::slice` for structs hinges on exactly this contract).
This PR surfaces those invariants on the public accessors so they are visible in the docs and can be cited as the authority in code and reviews.
# What changes are included in this PR?
Documentation only, in `arrow-data/src/data.rs`:
- `ArrayData::offset()`: states where the offset applies (buffers and child data, never nulls) and spells out the struct case: logical element `i` of the struct corresponds to element `offset + i` of each child, composing cumulatively with the child's own offset.
- `ArrayData::child_data()`: notes that for element-aligned children (such as structs) the parent's `offset`/`len` window the children, and points back to `Self::offset`.
# Are these changes tested?
Docs only; covered by CI doc builds.
# Are there any user-facing changes?
Documentation only — no behavior changes.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs(arrow-data): Document how `ArrayData::offset` applies to buffers, child_data and nulls [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10838:
URL: https://github.com/apache/arrow-rs/pull/10838#issuecomment-5431408395
I am happy to change this documentation if we find other issues with it. However, I am merging it in for now to unblock progress on https://github.com/apache/arrow-rs/pull/10838
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs(arrow-data): Document how `ArrayData::offset` applies to buffers, child_data and nulls [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10838:
URL: https://github.com/apache/arrow-rs/pull/10838#discussion_r3857334243
##########
arrow-data/src/data.rs:
##########
@@ -471,7 +476,57 @@ impl ArrayData {
self.len == 0
}
- /// Returns the offset of this [`ArrayData`]
+ /// Returns the offset in elements of this [`ArrayData`]
+ ///
+ /// The offset applies to [`Self::buffers`] and [`Self::child_data`],
+ /// but does NOT apply to [`Self::nulls`], which always represents exactly
+ /// [`Self::len`] elements.
+ ///
+ /// # Offsets for Non-nested types
+ ///
+ /// For non-nested types, the offset skips leading elements in the buffers.
+ /// Logical element `i` is stored at physical position `offset + i`.
+ ///
+ /// For example, with `offset = 2` and `len = 3` the following array
Review Comment:
I spent quite a while trying to make comments and ASCII art that illustrate what is going on
##########
arrow-data/src/data.rs:
##########
@@ -471,7 +476,57 @@ impl ArrayData {
self.len == 0
}
- /// Returns the offset of this [`ArrayData`]
+ /// Returns the offset in elements of this [`ArrayData`]
+ ///
+ /// The offset applies to [`Self::buffers`] and [`Self::child_data`],
+ /// but does NOT apply to [`Self::nulls`], which always represents exactly
+ /// [`Self::len`] elements.
+ ///
+ /// # Offsets for Non-nested types
+ ///
+ /// For non-nested types, the offset skips leading elements in the buffers.
+ /// Logical element `i` is stored at physical position `offset + i`.
+ ///
+ /// For example, with `offset = 2` and `len = 3` the following array
+ /// represents elements `[C, D, E]`:
+ ///
+ /// ```text
+ /// offset: 2 len: 3
+ /// ◀───────────▶◀────────────────▶
+ /// ┌─────┬─────┬─────┬─────┬─────┬─────┐
+ /// values buffer │ A │ B │ C │ D │ E │ F │
+ /// └─────┴─────┴─────┴─────┴─────┴─────┘
+ /// physical index 0 1 2 3 4 5
+ /// logical index 0 1 2
+ /// ```
+ ///
+ /// # Offsets for Struct types
Review Comment:
I think the offset rules are different for other types (like Unions) but this PR is already pretty tricky so we can perhaps add more detailed information as a follow on PR
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs(arrow-data): Document how `ArrayData::offset` applies to buffers, child_data and nulls [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #10838:
URL: https://github.com/apache/arrow-rs/pull/10838
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs(arrow-data): Document how `ArrayData::offset` applies to buffers, child_data and nulls [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10838:
URL: https://github.com/apache/arrow-rs/pull/10838#discussion_r3860365179
##########
arrow-data/src/data.rs:
##########
@@ -471,7 +476,57 @@ impl ArrayData {
self.len == 0
}
- /// Returns the offset of this [`ArrayData`]
+ /// Returns the offset in elements of this [`ArrayData`]
+ ///
+ /// The offset applies to [`Self::buffers`] and [`Self::child_data`],
+ /// but does NOT apply to [`Self::nulls`], which always represents exactly
+ /// [`Self::len`] elements.
+ ///
+ /// # Offsets for Non-nested types
+ ///
+ /// For non-nested types, the offset skips leading elements in the buffers.
+ /// Logical element `i` is stored at physical position `offset + i`.
+ ///
+ /// For example, with `offset = 2` and `len = 3` the following array
Review Comment:
<img width="1200" height="675" alt="Image" src="https://github.com/user-attachments/assets/2244fda3-5b13-4090-b971-eb861b658ef4" />
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org