sunchao / parquet-rs

Apache Parquet implementation in Rust
Apache License 2.0
149 stars 20 forks source link

Fix max definition and repetition levels in schema descriptor #24

Closed sadikovi closed 6 years ago

sadikovi commented 6 years ago

This PR fixes issue when max definition and repetition levels are not resolved correctly. This is observed well on optional struct type fields:

OPTIONAL group b {
  OPTIONAL INT32 _1;
  OPTIONAL INT32 _2;
}

as well as repeated fields, however, because both repetition and definition levels are incremented, issue might have been masked due to switch def -> rep and rep -> def in the code.

It should be correct now, I tested on several files locally and added unit-test (passes with fix, fails otherwise).

sadikovi commented 6 years ago

@sunchao Could you review this PR, please? Thanks!

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.03%) to 88.633% when pulling 04a12391163a22412696a30892a0ad10e2dc2143 on sadikovi:fix-column-descr into 403e6f7ecd6b20e8026893b37f0171d99c7f2479 on sunchao:master.

sunchao commented 6 years ago

Merged!

sadikovi commented 6 years ago

Thanks for merging! More than happy to help and fix them!