Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added serde support for RowGroupMetaData. #202

Merged
merged 2 commits into from
Nov 28, 2022

Conversation

youngsofun
Copy link
Contributor

@youngsofun youngsofun commented Nov 18, 2022

use thrift for types in parquet_format_safe.

#200

it turns out that there are as many as 16 types that need to impl De/Serialize, so it is very troublesome to work it around outside.

@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2022

Codecov Report

Base: 85.69% // Head: 85.69% // No change to project coverage 👍

Coverage data is based on head (b1678c9) compared to base (21a7f98).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #202   +/-   ##
=======================================
  Coverage   85.69%   85.69%           
=======================================
  Files          84       84           
  Lines        8254     8254           
=======================================
  Hits         7073     7073           
  Misses       1181     1181           
Impacted Files Coverage Δ
src/metadata/column_chunk_metadata.rs 69.51% <ø> (ø)
src/metadata/column_descriptor.rs 100.00% <ø> (ø)
src/metadata/column_order.rs 0.00% <ø> (ø)
src/metadata/row_metadata.rs 40.00% <ø> (ø)
src/metadata/schema_descriptor.rs 91.39% <ø> (ø)
src/metadata/sort.rs 54.54% <ø> (ø)
src/parquet_bridge.rs 78.90% <ø> (ø)
src/schema/types/basic_type.rs 100.00% <ø> (ø)
src/schema/types/converted_type.rs 93.87% <ø> (ø)
src/schema/types/parquet_type.rs 58.73% <ø> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jorgecarleitao
Copy link
Owner

This would add a significant dependency to this crate. Would it make sense to make this an optional feature under serde. Something like this: https://github.com/jorgecarleitao/arrow2/blob/main/src/datatypes/field.rs#L3

@youngsofun
Copy link
Contributor Author

@jorgecarleitao refactor done, it is a feature now.

use thrift for types in parquet_format_safe.
@youngsofun
Copy link
Contributor Author

@jorgecarleitao Is there anything else needs to improve?

@jorgecarleitao jorgecarleitao added the feature A new feature label Nov 28, 2022
@jorgecarleitao jorgecarleitao changed the title feat: impl De/Serialize for RowGroupMetaData. Added serde support for RowGroupMetaData. Nov 28, 2022
@jorgecarleitao jorgecarleitao merged commit 0cfedf0 into jorgecarleitao:main Nov 28, 2022
@jorgecarleitao
Copy link
Owner

Awesome, thanks a lot!

@youngsofun youngsofun deleted the seder branch November 30, 2022 02:18
@youngsofun
Copy link
Contributor Author

Is it ready to bump to 0.16.4? @jorgecarleitao

@jorgecarleitao
Copy link
Owner

We need to bump to 0.17 - there are breaking changes in main. Released :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants