westonpace commented on code in PR #7385:
URL: https://github.com/apache/arrow-rs/pull/7385#discussion_r2027419606
##########
arrow-data/src/data.rs:
##########
@@ -201,26 +201,45 @@ pub(crate) fn new_buffers(data_type: &DataType, capacity:
usize) -> [MutableBuff
#[derive(Debug, Clone)]
pub struct ArrayData {
- /// The data type for this array data
+ /// The data type
data_type: DataType,
- /// The number of elements in this array data
+ /// The number of elements
len: usize,
- /// The offset into this array data, in number of items
+ /// The offset in number of items (not bytes).
+ ///
+ /// The offset applies to [`Self::child_data`] and [`Self::buffers`]. It
+ /// does NOT apply to [`Self::nulls`].
offset: usize,
- /// The buffers for this array data. Note that depending on the array
types, this
- /// could hold different kinds of buffers (e.g., value buffer, value
offset buffer)
- /// at different positions.
+ /// The buffers that store the actual data for this array, as defined
+ /// in the [Arrow Spec].
+ ///
+ /// Depending on the array types, [`Self::buffers`] can hold different
+ /// kinds of buffers (e.g., value buffer, value offset buffer) at different
+ /// positions.
+ ///
+ /// This ArrayData's first logical element begins at `offset`
+ ///
+ /// [Arrow
Spec](https://arrow.apache.org/docs/format/Columnar.html#physical-memory-layout)
buffers: Vec<Buffer>,
- /// The child(ren) of this array. Only non-empty for nested types,
currently
- /// `ListArray` and `StructArray`.
+ /// The child(ren) of this array.
+ ///
+ /// Only non-empty for nested types, such as `ListArray` and
+ /// `StructArray`.
+ ///
+ /// The first logical element in each child element begins at `offset`.
Review Comment:
```suggestion
/// The first logical element in each child element begins at `offset`.
///
/// If the child element also has an offset then these offsets are
/// cumulative.
```
##########
arrow-data/src/data.rs:
##########
@@ -201,26 +201,45 @@ pub(crate) fn new_buffers(data_type: &DataType, capacity:
usize) -> [MutableBuff
#[derive(Debug, Clone)]
pub struct ArrayData {
- /// The data type for this array data
+ /// The data type
data_type: DataType,
- /// The number of elements in this array data
+ /// The number of elements
len: usize,
- /// The offset into this array data, in number of items
+ /// The offset in number of items (not bytes).
+ ///
+ /// The offset applies to [`Self::child_data`] and [`Self::buffers`]. It
+ /// does NOT apply to [`Self::nulls`].
offset: usize,
- /// The buffers for this array data. Note that depending on the array
types, this
- /// could hold different kinds of buffers (e.g., value buffer, value
offset buffer)
- /// at different positions.
+ /// The buffers that store the actual data for this array, as defined
+ /// in the [Arrow Spec].
+ ///
+ /// Depending on the array types, [`Self::buffers`] can hold different
+ /// kinds of buffers (e.g., value buffer, value offset buffer) at different
+ /// positions.
+ ///
+ /// This ArrayData's first logical element begins at `offset`
Review Comment:
The phrasing here, and the fact we are talking about buffers, makes me think
of a byte-offset (even though it's pointed out that it is not a byte offset
above).
```suggestion
/// The buffer may be larger than needed. Some items at the beginning
may be skipped if
/// there is an `offset`. Some items at the end may be skipped if the
buffer is longer than
/// we need to satisfy `len`.
```
--
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]
