Optionally write version 2 page headers in Parquet writer #13751
Conversation
|
Pull requests from external contributors require approval from a |
vuule
added
feature request
|
/ok to test |
|
/ok to test |
| ParquetFieldInt32(3, p->compressed_page_size), | ||
| ParquetFieldStruct(5, p->data_page_header), | ||
| ParquetFieldStruct(7, p->dictionary_page_header)); | ||
| ParquetFieldStruct(7, p->dictionary_page_header), | ||
| ParquetFieldStruct(8, p->data_page_header_v2)); |
There was a problem hiding this comment.
6 would be the un-implemented IndexPageHeader. It's a placeholder type in the Parquet spec.
And before you ask, 4 is an optional CRC that no one adds either. 😉
…o feature/write_v2_header
|
/ok to test |
|
/ok to test |
| @@ -275,6 +276,18 @@ bool CompactProtocolReader::read(DictionaryPageHeader* d) | |||
| return function_builder(this, op); | |||
| } | |||
|
|
|||
| bool CompactProtocolReader::read(DataPageHeaderV2* d) | |||
There was a problem hiding this comment.
How come this wasn't needed in #11778?
There was a problem hiding this comment.
Because the header decoder is device code, so there's a duplicate thrift decoder buried in page_hdr.cu We've got little bits of thrift decoding sprinkled all over the place :(
cpp/src/io/parquet/page_enc.cu
Outdated
| comp_in[blockIdx.x] = {base, actual_data_size}; | ||
| comp_out[blockIdx.x] = {s->page.compressed_data + s->page.max_hdr_size, 0}; // size is unused | ||
| // V2 does not compress rep and def level data | ||
| size_t const offset = s->page.def_lvl_bytes + s->page.rep_lvl_bytes; |
There was a problem hiding this comment.
Took me a bit to connect the dots in this block. Maybe a rename?
| size_t const offset = s->page.def_lvl_bytes + s->page.rep_lvl_bytes; | |
| size_t const skip_comp_size = s->page.def_lvl_bytes + s->page.rep_lvl_bytes; |
There was a problem hiding this comment.
sounds good. Oh, the memcpy just below...I'm thinking that should be parallelized, but keep forgetting. Thoughts? The level data can get pretty large (100s of bytes?).
Or I can just address this in the next PR.
|
/ok to test |
Co-authored-by: Nghia Truong <[email protected]>
|
/ok to test |
|
/merge |
Description
Part of #13501. This adds the ability to write V2 page headers to the Parquet writer.
Checklist