[Doc] Correct ListArray documentation
#8803
Conversation
I think the diagram is actually correct -- the nullability of the ListArray is defined by its own null mask (not the null mask of the child) |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @liamzwbao -- I took another look and I agree the changes on line 156 are great.
I think the change on line R117 should be reverted
Thanks again for the find and sticking with this PR
arrow-array/src/array/list_array.rs
Outdated
| /// ┌─────────────┐ ┌───────┐ │ ┌───┐ ┌───┐ ┌───┐ ┌───┐ | ||
| /// │ [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 │ | ||
| /// ├─────────────┤ ├───────┤ │ ├───┤ ├───┤ ├───┤ ├───┤ | ||
| /// │ [NULL, F] │ │ (5,7) │ │ 1 │ │ 5 │ │ │ 1 │ │ D │ │ 4 │ | ||
| /// └─────────────┘ └───────┘ │ └───┘ ├───┤ ├───┤ ├───┤ | ||
| /// │ 7 │ │ │ 0 │ │ ? │ │ 5 │ | ||
| /// │ Validity └───┘ ├───┤ ├───┤ | ||
| /// Logical Logical (nulls) Offsets │ │ 1 │ │ F │ │ 6 │ | ||
| /// Values Offsets │ └───┘ └───┘ | ||
| /// │ Values │ │ | ||
| /// (offsets[i], │ ListArray (Array) | ||
| /// offsets[i+1]) └ ─ ─ ─ ─ ─ ─ ┘ │ |
There was a problem hiding this comment.
Hi @alamb, while taking a look at generic_list_builder, I noticed that we shouldn't increment offset for NULL as the top level element. so the logic offsets should be [[0, 3], [3, 3], [3, 3], [3, 4], [4, 6]] and the values offsets should be [0, 3, 3, 3, 4, 6], is that right?
There was a problem hiding this comment.
nd the values offsets should be [0, 3, 3, 3, 4, 6], is that right?
Yes that sounds right
Isn't that what this line says:
&[0, 3, 3, 3, 4, 6]Maybe I don't understand your question
There was a problem hiding this comment.
I mean the example in generic_list_builder is correct but the example here in list_array is wrong. So we need to change the doc here, let me update the PR to make it clear
There was a problem hiding this comment.
I see -- you are saying that we don't have to have an element in the underlying values array for a null list in the parent. I think either is fine, but your change here is probably clearer.
Thank you
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @liamzwbao -- this looks great to me now
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @liamzwbao -- this looks great to me now
2 participants
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Fix the minor errors in the
ListArraydoc.slice(1, 3)should getvalue(3) = Nullandvalue(4) = D, so thevalue(5)is unusedAre these changes tested?
Docs only
Are there any user-facing changes?
No