sunchao / parquet-rs

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

Add page writer #133

Closed sadikovi closed 6 years ago

sadikovi commented 6 years ago

This PR adds:

sadikovi commented 6 years ago

@sunchao I implemented page writer. Let me know if I need to add more tests, I feel like the currently added tests may not cover all of the cases. Thanks!

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 560


Files with Coverage Reduction New Missed Lines %
encodings/encoding.rs 1 94.8%
column/page.rs 4 96.3%
<!-- Total: 5 -->
Totals Coverage Status
Change from base Build 558: 0.05%
Covered Lines: 11348
Relevant Lines: 11890

💛 - Coveralls
sadikovi commented 6 years ago

@sunchao can you have a look again? I addressed your comments and added more docs describing each method.

I was thinking if PageWriter trait needs an improvement; it might be a bit difficult to reason about the API without column writer. Let me know what you think.

Thanks!

sadikovi commented 6 years ago

Yes, you are right, it does. We should be able to refactor both when working on column chunk writer.

sunchao commented 6 years ago

Merged. Thanks @sadikovi !

sadikovi commented 6 years ago

Thanks @sunchao!