tustvold commented on code in PR #3529:
URL: https://github.com/apache/arrow-rs/pull/3529#discussion_r1070527371
##########
arrow-data/src/data.rs:
##########
@@ -245,18 +245,58 @@ pub(crate) fn into_buffers(
/// An generic representation of Arrow array data which encapsulates common
attributes and
/// operations for Arrow array. Specific operations for different arrays types
(e.g.,
/// primitive, list, struct) are implemented in `Array`.
+///
+/// # Memory Layout
+///
+/// `ArrayData` has references to one or more underlying data buffers
+/// and optional child ArrayDatas, depending on type as illustrated
+/// below. Bitmaps are not shown for similicity but they are stored
Review Comment:
```suggestion
/// below. Bitmaps are not shown for simplicity but they are stored
```
##########
arrow-data/src/data.rs:
##########
@@ -448,7 +489,17 @@ impl ArrayData {
self.null_count
}
- /// Returns the total number of bytes of memory occupied by the buffers
owned by this [ArrayData].
+ /// Returns the total number of bytes of memory occupied by the
+ /// buffers owned by this [ArrayData] and all of its
+ /// children. (See also diagram on [ArrayData]).
+ ///
+ /// Note that this [`ArrayData`] may only refer to a subset of the
+ /// data in the underlying [`Buffer`]s (due to `offset` and
+ /// `length`), but the size returned includes the entire size of
+ /// the buffer.
Review Comment:
```suggestion
/// the buffers.
```
##########
arrow-data/src/data.rs:
##########
@@ -245,18 +245,58 @@ pub(crate) fn into_buffers(
/// An generic representation of Arrow array data which encapsulates common
attributes and
/// operations for Arrow array. Specific operations for different arrays types
(e.g.,
/// primitive, list, struct) are implemented in `Array`.
+///
+/// # Memory Layout
+///
+/// `ArrayData` has references to one or more underlying data buffers
+/// and optional child ArrayDatas, depending on type as illustrated
+/// below. Bitmaps are not shown for similicity but they are stored
+/// similiarly to the buffers.
+///
+/// ```text
+/// offset
+/// points to
+/// ┌───────────────────┐ start of ┌───────┐ Different
+/// │ │ data │ │ ArrayData may
+/// │ArrayData { │ │.... │ also refer to
+/// │ data_type: ... │ ─ ─ ─ ─▶│1234 │ ┌ ─ the same
+/// │ offset: ... ─ ─ ─│─ ┘ │4372 │ underlying
+/// │ len: ... ─ ─ ─│─ ┐ │4888 │ │ buffer
+/// │ buffers: [ │ │5882 │◀─
+/// │ ... │ │ │4323 │
+/// │ ] │ ─ ─ ─ ─▶│4859 │
+/// │ child_data: [ │ │.... │
+/// │ ... │ │ │
+/// │ ] │ └───────┘
+/// │} │
+/// │ │ Shared Buffer uses
+/// │ │ │ bytes::Bytes to hold
+/// └───────────────────┘ actual data values
+/// ┌ ─ ─ ┘
+///
+/// ▼
+/// ┌───────────────────┐
+/// │ArrayData { │
+/// │ ... │
+/// │} │
+/// │ │
+/// └───────────────────┘
+///
+/// Child ArrayData may also have its own buffers and children
+/// ```
+
#[derive(Debug, Clone)]
pub struct ArrayData {
/// The data type for this array data
data_type: DataType,
- /// The number of elements in this array data
+ /// The number of elements in this array data.
Review Comment:
This is now inconsistent with the other field FWIW
##########
arrow-data/src/data.rs:
##########
@@ -541,8 +610,9 @@ impl ArrayData {
size
}
- /// Creates a zero-copy slice of itself. This creates a new [ArrayData]
- /// with a different offset, len and a shifted null bitmap.
+ /// Creates a zero-copy slice of itself. This creates a new
+ /// [ArrayData] pointing at the same underlying [`Buffer`]s with a
+ /// different offset, len and a shifted null bitmap.
Review Comment:
```suggestion
/// different offset and len
```
We don't treat the null bitmap any differently, this seems to imply we bit
slice it
--
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]
