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

Revert making parquet::data_type and parquet::arrow::schema experimental #1244

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

tustvold
Copy link
Contributor

@tustvold tustvold commented Jan 28, 2022

Which issue does this PR close?

Closes #1032

Rationale for this change

#1134 was a tad overzealous and moved some functionality behind the experimental flags that downstream projects depend on.

What changes are included in this PR?

  • arrow schema conversion functions are made public, these are used by IOx to get the arrow schema from parquet metadata cached in its catalog
  • data_type is made public, this is used by sqlite2parquet in order to interface with the ColumnWriter APIs

I debated spending the time to instead mark the various bits of these modules experimental, but since the move to more frequent breaking releases (#1120) there seems to be limited value add from such an undertaking. The concept of experimental made sense when breaking changes were quarterly, now that they're biweekly...

Are there any user-facing changes?

No breaking changes, this can (and perhaps should be) a patch release.

@github-actions github-actions bot added the parquet Changes to the parquet crate label Jan 28, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1244 (6a4469c) into master (02573e9) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1244   +/-   ##
=======================================
  Coverage   82.95%   82.95%           
=======================================
  Files         178      178           
  Lines       51522    51522           
=======================================
+ Hits        42740    42741    +1     
+ Misses       8782     8781    -1     
Impacted Files Coverage Δ
arrow/src/array/transform/mod.rs 84.64% <0.00%> (-0.13%) ⬇️
parquet_derive/src/parquet_field.rs 65.98% <0.00%> (ø)
parquet/src/encodings/encoding.rs 93.71% <0.00%> (+0.19%) ⬆️
arrow/src/datatypes/datatype.rs 66.80% <0.00%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02573e9...6a4469c. Read the comment docs.

@pacman82
Copy link

DataType is used in odbc2parquet, too. I would welcome to have it back in the public API again, or to at least have an alternative.

@alamb
Copy link
Contributor

alamb commented Jan 28, 2022

Thank you for the report @pacman82 👍

@alamb alamb merged commit 4b7afa6 into apache:master Jan 28, 2022
@pacman82
Copy link

You are welcome @alamb !

@pacman82
Copy link

Thanks for merging!

@alamb
Copy link
Contributor

alamb commented Jan 28, 2022

This should be available in arrow 9.0.0 (scheduled for release in ~ Feb 7, 2022) -- I'll make an RC next friday and then it takes 3ish days to get approved

@alamb alamb added the bug label Feb 3, 2022
@alamb alamb changed the title Revert making parquet::data_type and parquet::arrow::schema experimental Revert making parquet::data_type and parquet::arrow::schema experimental Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce Public Parquet API
4 participants