iemejia commented on code in PR #574: URL: https://github.com/apache/parquet-format/pull/574#discussion_r3359147718
########## VariantEncoding.md: ########## @@ -87,6 +87,7 @@ The `version` is a 4-bit value that must always contain the value `1`. `sorted_strings` is a 1-bit value indicating whether dictionary strings are sorted and unique. `offset_size_minus_one` is a 2-bit value providing the number of bytes per dictionary size and offset field. The actual number of bytes, `offset_size`, is `offset_size_minus_one + 1`. +Bit 5 (marked `R`) is reserved; it must be set to 0 by writers and ignored by readers. Review Comment: You are right that this was not previously specified. However, I think mandating "must be set to 0 by writers" is the standard practice for reserved bits in binary specs — it ensures forward compatibility so that a future version can assign meaning to these bits without having to worry about random garbage from older writers. Without this constraint, a future revision would need a version bump even for a trivially additive change. That said, if the consensus is to keep the spec minimal and only add reader behavior, I'm fine with your suggestion. I could also go with a middle ground: "should be set to 0 by writers; must be ignored by readers." What do you prefer? ########## VariantEncoding.md: ########## @@ -206,14 +207,15 @@ value_header | | | | | The actual number of bytes is computed as `field_offset_size_minus_one + 1` and `field_id_size_minus_one + 1`. `is_large` is a 1-bit value that indicates how many bytes are used to encode the number of elements. If `is_large` is `0`, 1 byte is used, and if `is_large` is `1`, 4 bytes are used. +Bit 5 (marked `R`) is reserved; it must be set to 0 by writers and ignored by readers. Review Comment: Same reasoning as above — will align with whatever we decide for comment 1. ########## VariantEncoding.md: ########## @@ -223,6 +225,7 @@ value_header | | | | The actual number of bytes is computed as `field_offset_size_minus_one + 1`. `is_large` is a 1-bit value that indicates how many bytes are used to encode the number of elements. If `is_large` is `0`, 1 byte is used, and if `is_large` is `1`, 4 bytes are used. +Bits 5-3 (marked `RRR`) are reserved; they must be set to 0 by writers and ignored by readers. Review Comment: Agreed, will follow the same resolution. -- 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]
