Improve parquet performance: Skip levels computation for required struct arrays in parquet #1035
Conversation
tustvold
force-pushed
the
struct-reader-levels-computation
branch
from
4533e29 to
944c722
Compare
Codecov Report
@@ Coverage Diff @@
## master #1035 +/- ##
==========================================
- Coverage 82.31% 82.29% -0.03%
==========================================
Files 168 168
Lines 49420 49423 +3
==========================================
- Hits 40681 40673 -8
- Misses 8739 8750 +11
Continue to review full report at Codecov.
|
alamb
changed the title
There was a problem hiding this comment.
I finally had a chance to review this PR, and now that I see what it is doing it looks correct to me. 👍 thank you @tustvold
cc @nevi-me our resident array reader / struct array level expert and @liurenjie1024 as the original author
parquet/src/arrow/array_reader.rs
Outdated
| let mut def_level_data_buffer = MutableBuffer::new(buffer_size); | ||
| def_level_data_buffer.resize(buffer_size, 0); | ||
| // Now we can build array data | ||
| let mut array_data = ArrayDataBuilder::new(self.data_type.clone()) |
There was a problem hiding this comment.
Maybe calling this variable array_data_builder would be clearer as it not ArrayData?
|
@tustvold note there are some conflicts that need to be resolved in this PR |
|
Since no one else has any comments, merging this in |
Which issue does this PR close?
Closes #1034.
Rationale for this change
See ticket
What changes are included in this PR?
Changes StructArrayReader to not compute definition, repetition and validity buffers for required struct arrays.
Are there any user-facing changes?
Technically this alters the precise semantics of
ArrayReaderwhich is currently a public trait. I think this is potentially unintentional (#1032), but even then the documented purpose of these methods is for parentArrayReaderto handle nulls and repeated arrays. However, such a parent cannot exist in the altered edge-case, as otherwise the definition/repetition levels of the struct array would be non-zero.