Fix StructArrayReader handling nested lists (#1651) #1700
Fix StructArrayReader handling nested lists (#1651) #1700tustvold merged 2 commits intoapache:masterfrom
Conversation
There was a problem hiding this comment.
I am a big fan of less unsafe 👍
tustvold
force-pushed
the
struct-array-nested-lists
branch
2 times, most recently
from
c5a8d4b to
230eb38
Compare
tustvold
force-pushed
the
struct-array-nested-lists
branch
from
230eb38 to
f679322
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @tustvold
I found this easier to review by ignoring whitespace https://github.com/apache/arrow-rs/pull/1700/files?w=1
As I am not a super expert in this code, I can't say I fully grok it but if it fixes the file from @kesavkolla sounds good to me.
I had one question on the tests, but I suspect it is my own mis-understanding
| def_level_data_buffer.resize(buffer_size, 0); | ||
|
|
||
| // Safety: the buffer is always treated as `u16` in the code below | ||
| let def_level_data = unsafe { def_level_data_buffer.typed_data_mut() }; |
parquet/src/arrow/array_reader.rs
Outdated
| None, | ||
| ])); | ||
|
|
||
| let nulls = Buffer::from([0b00000111]); |
There was a problem hiding this comment.
doesn't this set the first three elements in the struct array to NULL?
That doesn't seem consistent with the structure created in the comments above
There was a problem hiding this comment.
Will rename to validity, it is an arrow null mask... I agree the naming is perpetually confusing
There was a problem hiding this comment.
I am a big fan of less unsafe 👍
2 participants
Which issue does this PR close?
Closes #1651.
Rationale for this change
See ticket
What changes are included in this PR?
Fixes handling of nested lists within StructArrayReader
Are there any user-facing changes?
Files that used to fail to parse, now parse correctly