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

Prevent parquet from closing zstd streams on finalization #2586

Merged
merged 4 commits into from
Jun 29, 2022

Conversation

niloc132
Copy link
Member

Fixes #2569

@niloc132 niloc132 added parquet Related to the Parquet integration NoDocumentationNeeded labels Jun 27, 2022
@niloc132 niloc132 added this to the Jun 2022 milestone Jun 27, 2022
devinrsmith
devinrsmith previously approved these changes Jun 27, 2022
*/
public class ZstdCodec extends ZstandardCodec {
@Override
public CompressionInputStream createInputStream(InputStream stream) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

I think all of these changes belong in a differently-licensed module. Probably ParquetHadoop.

Copy link
Member

Choose a reason for hiding this comment

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

Or we can make this module apache as well? (If it makes more sense to put into ParquetHadoop b/c they are intimately related, then I agree that might be best.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the code for the moment anyway, but if we end up adding much more in this compression stuff I think it could be nice to open that up, offer as a different option than the upstream CompressionCodecFactory(s).

…c/zstd/ZstdDecompressorStream.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@niloc132 niloc132 merged commit 4b82c7b into deephaven:main Jun 29, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
NoDocumentationNeeded parquet Related to the Parquet integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parquet ZSTD file is not decompressed properly
3 participants