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

feat: parse Parquet schema from ParquetMetadataConverter #6550

Conversation

devinrsmith
Copy link
Member

@devinrsmith devinrsmith commented Jan 10, 2025

This removes the DH's duplication of schema reading logic, and removes the warning

DO NOT RELY ON {@link ParquetMetadataConverter} FOR THIS! USE

There has been no tests to indicate that using ParquetMetadataConverter causes any issues. It may have been true historically, but does not seem to be true anymore.

@devinrsmith
Copy link
Member Author

This is a much smaller piece of #6189.

At least, we need to figure out if there is any merit to

DO NOT RELY ON {@link ParquetMetadataConverter} FOR THIS! USE {@link ParquetFileReader}

@devinrsmith devinrsmith changed the title feat: Parse Parquet schema MessageType from ParquetMetadataConverter feat: parse Parquet schema from ParquetMetadataConverter Jan 13, 2025
@devinrsmith devinrsmith self-assigned this Jan 13, 2025
@devinrsmith devinrsmith added this to the 0.38.0 milestone Jan 13, 2025
@devinrsmith devinrsmith marked this pull request as ready for review January 13, 2025 16:50
Copy link
Contributor

@malhotrashivam malhotrashivam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have verified that all the recent bug fixes and changes we have done in Parquet type deduction logic is included in the ParquetMetadataConverter, so this change looks good to me, other than a small minor comment.

@devinrsmith devinrsmith requested a review from rcaudy January 14, 2025 18:36
@devinrsmith devinrsmith merged commit 50e5408 into deephaven:main Jan 16, 2025
17 checks passed
@devinrsmith devinrsmith deleted the nightly/parse-parquet-schema-from-ParquetMetadataConverter branch January 16, 2025 00:40
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants