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

Add support of parquet_writer_version, 1.0 and 2.0 #20209

Closed

Conversation

wanglinsong
Copy link
Member

@wanglinsong wanglinsong commented Jul 6, 2023

Test plan - Add test cases for versions 1.0 and 2.0.

Enable Hive and Iceberg connectors to use either Parquet writer version 1.0 or 2.0 when writing Parquet data.

== RELEASE NOTES ==

Parquet Changes
* Update Hive and Iceberg connectors to use either Parquet writer version 1.0 or 2.0 when writing Parquet data.

@wanglinsong wanglinsong requested review from shangxinli and a team as code owners July 6, 2023 06:22
@wanglinsong wanglinsong requested a review from presto-oss July 6, 2023 06:22
@wanglinsong wanglinsong marked this pull request as draft July 6, 2023 06:22
@wanglinsong wanglinsong force-pushed the 17240-parquet-writer-version branch 3 times, most recently from 90e0d34 to 84d365f Compare July 6, 2023 09:08
Copy link
Contributor

@yingsu00 yingsu00 left a comment

Choose a reason for hiding this comment

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

Looks good, except the Jenkinsfile change which I don't know what is about.

Jenkinsfile Outdated Show resolved Hide resolved
@yingsu00
Copy link
Contributor

yingsu00 commented Jul 6, 2023

@wanglinsong you also need to update the PR message and add the release note section.

@wanglinsong wanglinsong force-pushed the 17240-parquet-writer-version branch 3 times, most recently from ff9ce25 to b156a36 Compare July 19, 2023 17:00
@yingsu00
Copy link
Contributor

@wanglinsong Any update on this PR?

@wanglinsong wanglinsong force-pushed the 17240-parquet-writer-version branch 2 times, most recently from f0c70a2 to 6b7cef6 Compare August 20, 2023 19:01
@wanglinsong wanglinsong changed the title Add support of parquet_writer_format_version, 1.0 and 2.0 Add support of parquet_writer_version, 1.0 and 2.0 Aug 20, 2023
@wanglinsong wanglinsong force-pushed the 17240-parquet-writer-version branch 2 times, most recently from 7f47c39 to 114a722 Compare August 20, 2023 20:48
@wanglinsong wanglinsong force-pushed the 17240-parquet-writer-version branch from 114a722 to 12ecc43 Compare August 21, 2023 02:46
@wanglinsong
Copy link
Member Author

@wanglinsong Any update on this PR?

Hi @yingsu00
After looking into this issue closely, adding v1 writer is too big for me to manage for now. Here is how trino did it (trinodb/trino#9611). What I can do now is to add v1/v2 session properties, set the default to v2, and add the placeholder for v1 writer (raise an exception when the session prop is set to v1).
Let me know if this works for you.

@wanglinsong wanglinsong force-pushed the 17240-parquet-writer-version branch from d46393e to 12ecc43 Compare September 2, 2023 17:56
@yingsu00
Copy link
Contributor

yingsu00 commented Apr 4, 2024

The work has been finished in another PR. Closing.

@yingsu00 yingsu00 closed this Apr 4, 2024
@wanglinsong wanglinsong deleted the 17240-parquet-writer-version branch April 6, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants