alamb commented on code in PR #8803:
URL: https://github.com/apache/arrow-rs/pull/8803#discussion_r2514963527
##########
arrow-array/src/array/list_array.rs:
##########
@@ -110,11 +110,11 @@ impl OffsetSizeTrait for i64 {
/// ┌─────────────┐ ┌───────┐ │ ┌───┐ ┌───┐ ┌───┐
┌───┐
/// │ [A,B,C] │ │ (0,3) │ │ 1 │ │ 0 │ │ │ 1 │ │
A │ │ 0 │
/// ├─────────────┤ ├───────┤ │ ├───┤ ├───┤ ├───┤
├───┤
-/// │ [] │ │ (3,3) │ │ 1 │ │ 3 │ │ │ 1 │ │
B │ │ 1 │
+/// │ [] (empty) │ │ (3,3) │ │ 1 │ │ 3 │ │ │ 1 │ │
B │ │ 1 │
/// ├─────────────┤ ├───────┤ │ ├───┤ ├───┤ ├───┤
├───┤
/// │ NULL │ │ (3,4) │ │ 0 │ │ 3 │ │ │ 1 │ │
C │ │ 2 │
/// ├─────────────┤ ├───────┤ │ ├───┤ ├───┤ ├───┤
├───┤
-/// │ [D] │ │ (4,5) │ │ 1 │ │ 4 │ │ │ ? │ │
? │ │ 3 │
+/// │ [D] │ │ (4,5) │ │ 1 │ │ 4 │ │ │ 0 │ │
? │ │ 3 │
Review Comment:
Sorry -- I was confused about this change at first and got mixed up with the
whole PR.
I actually think it would be better to not change this line and actually
change line 152 also have two `?`,) to emphasize the point that the backing
validity and values for a NULL element in a ListArray are not used / ignored
The fact that there are two validity masks (the ListArray and the child) I
think makes this topic quite trick
##########
arrow-array/src/array/list_array.rs:
##########
@@ -152,8 +152,8 @@ impl OffsetSizeTrait for i64 {
/// │ [D] │ │ (4,5) │ │ 1 │ │ 4 │ │ │ 0 │ │ ? │ │ 3
│
/// └─────────────┘ └───────┘ │ └───┘ ├───┤ ├───┤ ├───┤
/// │ 5 │ │ │ 1 │ │ D │ │ 4
│
-/// │ └───┘ ├───┤ ├───┤
-/// │ │ 0 │ │ ? │ │ 5
│
+/// │ └───┘ ╠═══╣ ╠═══╣
+/// │ ║ 0 ║ ║ ? ║ │ 5
│
Review Comment:
This change makes sense 👍 thank you
--
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]
