feat(parquet): Add support for parquet version config #11151
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
svm1
force-pushed
the
parq_2
branch
2 times, most recently
from
40a6c6c to
4982e1f
Compare
|
@yingsu00 @majetideepak Thank you for reviewing - made all necessary changes, please take a look! |
majetideepak
changed the title
parquet_writer_version session property
|
@svm1 Arrow has two versions a user can set ParquetVersion and ParquetDataPageVersion. Based on the defaults (2.6), and the issue here prestodb/presto#22595, I see the Presto ParquetWriterVersion maps to ParquetVersion. https://github.com/apache/arrow/blob/main/cpp/src/parquet/properties.h#L258 @jkhaliqi Can you please evaluate these options with respect to RLE V2? |
| @@ -108,6 +109,7 @@ struct WriterOptions : public dwio::common::WriterOptions { | |||
| /// Timestamp time zone for Parquet write through Arrow bridge. | |||
| std::optional<std::string> parquetWriteTimestampTimeZone; | |||
| bool writeInt96AsTimestamp = false; | |||
| std::optional<arrow::ParquetDataPageVersion> parquetDataPageVersion; | |||
There was a problem hiding this comment.
Velox should have its own enum and not borrow from Arrow.
There was a problem hiding this comment.
I must've misinterpreted one of your previous comments, and also felt that as we are no longer working in the HiveConnector, there was no need for an intermediary enum. I can add it back.
I understand the two different versions - I think we had a discussion about this very debate back when I first began working on this fix, and we determined that the Presto I also investigated the Presto Java source - The Presto docs also seem to point to this mapping:
The format version also appears far more granular than simply "1 vs 2" (https://github.com/apache/parquet-format/blob/master/CHANGES.md). Therefore I believe the mapping established in this PR would be correct - please correct me if I am mistaken. |
|
@svm1 The issue reported here prestodb/presto#22595 hints that it is not the DataPageVersion but ParquetVersion. |
|
Also there are 4 ParquetVersions |
Allow the Presto session property
parquet_writer_version, which is currently ignored by Velox, to toggle the parquet writer datapage version (V1 or V2). The value can be set as a session property or can be provided in the Hive config. Defaults to V1.