emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622627276
##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
6: required i64 total_uncompressed_size
Review Comment:
> Can we make num_values, total_uncompressed_size, total_compressed_size
i32s?
No, we've had bugs in the past due to i32 overflow in various
implementations and incompatibilities with Arrow's i32 offsets because the data
stored is larger. I don't recall which of these fields had issues exactly but
based on that it would indicate that there are in fact some users that overflow
at least signed representations, so even unsigned int32 seems like a potential
risk.
> In addition num_values can be optional and if left unset it can inherit
RowGroup.num_rows. Most column chunks are dense and we can save repeating the
same value over and over for every column.
I agree this is a reasonable optimizations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
