You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "emkornfield (via GitHub)" <gi...@apache.org> on 2024/05/27 20:42:51 UTC

[PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

emkornfield opened a new pull request, #250:
URL: https://github.com/apache/parquet-format/pull/250

   As a point of discussion a slightly different version of showing how column metadata could be decoupled from FileMetadata then https://github.com/apache/parquet-format/pull/242
   
   In particular this takes a slightly different approach:
   1.  It introduces a new random access encoding for Parquet to store serialized data instead of relying a one-off index scheme based in the thrift structure.  By taking this approach is allows flexibility for implemenations to further balance size vs compute trade-offs and can potentially make use of any further encoding improvements in the future.  Two downside of this approach is it requires a little bit more work up-front and has slightly more overhead then directly doing this as thrift structures.
   2. It places the serialized data page completely outside of thrift metadata and instead provides an offset within the footer. This is mostly a micro-optimization (likely not critical) to allow parquet implementors to avoid unnecessary copies of string data if the Thrift library supporting it does not allow it.  There is no reason that the pages could not be inlined as a "binary" field in FileMetadata as is done in https://github.com/apache/parquet-format/pull/242
   3. Moves a few other fields out of FileMetadata into a metadata page and raises discussion points on others.
   4. Re-uses existing Thrift objects in attempt to make the transition easier for implementors.
   
   
   Things it does not do:
   1.  Enumerate all fields that should be deprecated https://github.com/apache/parquet-format/pull/242 is a good start and can consolidated on a final list once a general approach is taken.
   2. Incorporate changes in https://github.com/apache/parquet-format/pull/248 these also likely make sense but can be incorporated into any final proposal.
   3. Micro-optimizations to separate scan use cases from filter evaluation use-cases (ColumnChunk structure could potentially be further split apart to give finer grained access to elements that are only needed in once case or another).  
   
   


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620453723


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   I am talking about an enhanced version of my original proposal in #242.
   
   https://github.com/apache/parquet-format/pull/254/files?short_path=b335630#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5
   
   I feel this is very powerful. The only complexity is only on writers to incorporate the short snippet of code that appends binary to an existing thrift struct. Readers can look at the tail of any thrift serialized struct and find the extension they expect without parsing thrift.
   
   This means that we can use the approach as a generalized extension mechanism for *any* thrift struct in `parquet.thrift`. Once a vendor has a solid extension (or part of the extension) they want to make official it can enter the official struct proper and be removed from the extension.
   
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2142613709

   > @emkornfield I have a prototype with flatbuffers that generates footers of similar size to the current thrift protocol. I left some notes in this PR with some of the optimizations I did to achieve this.
   
   I tried to respond to the all the comments.
   
   > That said I feel the end state with flatbuffers is much better. The parse time (with verification) is already at 2.5ms/0.8ms for footer1/footer2 respectively, without substantially deviating from the current thrift object model. There is a lot of room for improvements if we deviate.
   
   I tend to agree, I think we should pose this question on the ML thread to see what peoples thoughts are on  going directly flatbuffers vs something more iterative on thrift.  Deviation might be fine, but I think in terms of development effort we should be aiming for something that we can go back and forth without loss of data with the original footer.  I think this would likely make it much easier for implementations to adopt flatbuffers and simply translate them back to Thrift structures as a bare minimum of support.
   
   > Should we join forces and iterate to make flatbuffers even better, while we collect footers from the fleet to compile a benchmark database to further validate the results? I feel there is a lot of room for shrinking Statistics further at which point we will have both smaller and >10x faster to parse footers.
   
   I'm happy to collaborate on the flatbuffers approach.  Do you want to open up a draft PR with your current state (maybe update to include the responses here).  I think the last major sticking point might be EncodingStatistics and I wonder how much that effects flatbuffers, or if we can use the trick here to embed those within there own string either as flatbuffers or keep them as thrift).  
   
   
   


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620082482


##########
src/main/thrift/parquet.thrift:
##########
@@ -537,6 +537,39 @@ enum Encoding {
       Support for INT32, INT64 and FIXED_LEN_BYTE_ARRAY added in 2.11.
    */
   BYTE_STREAM_SPLIT = 9;
+
+  /** Encoding for variable length binary data that allows random access of values.

Review Comment:
   I removed this and posted the description on encodings thread.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "ggershinsky (via GitHub)" <gi...@apache.org>.
ggershinsky commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620094012


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   sure, will do after the refactoring.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2135858634

   > I would prefer absolute file location rather than an offset to the start of the metadata; but it's probably harder to calculate, correct?
   
   Probably just as easy.  However, it means the footer is not self-contained for systems that might want to cache it (they would need to store an additional size value for the file).


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620073832


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   Removed most of this phrasing in favor of a layout.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620875846


##########
src/main/thrift/parquet.thrift:
##########
@@ -1127,18 +1229,48 @@ struct FileMetaData {
    * are flattened to a list by doing a depth-first traversal.
    * The column metadata contains the path in the schema for that column which can be
    * used to map columns to nodes in the schema.
-   * The first element is the root **/
-  2: required list<SchemaElement> schema;
+   * The first element is the root
+   *
+   * PAR1: Required
+   * PAR3: Use schema_page
+   **/
+  2: optional list<SchemaElement> schema;
+
+  /** Page has BYTE_ARRAY data where each element is REQUIRED. 
+    *
+    * Each element is a serialized SchemaElement.  The order and content should
+    * have a one to one correspondence with schema.
+    */
+  10: optional MetadataPage schema_page;

Review Comment:
   Why isn't this `list<binary>` where each binary is a serialized `SchemaElement`?



##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression

Review Comment:
   My feel is that most of the pages encoded in this form are going to be smallish, less than 1kb. For such small sizes, none of the general purpose compressors will do a good job at compressing.
   
   Are there any benchmarks where we can see the effectiveness compressing the above pages?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621000028


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order

Review Comment:
   I'm afraid this might complicate page index because we have to check consistency of ColumnOrder across row groups. At the moment we have only one ColumnOrder, which is not a big issue. It may have problem if we introduce more orders in the future. This order is important to guide us on how to interpret serialized min_value/max_values in the statistics. Perhaps we can put this in the `SchemaElement` instead?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620997989


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   But I just got this implemented 😭 😉. Joking aside, is there any other use for this field beyond optimizing dictionary based queries?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617749807


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Sorry I think I might have misunderstood your question.  I thought you were asking if 0x00 can appear elsewhere in serialized data, and the answer is yes, but it doesn't introduce ambiguity. But I don't think this is stated explicitly.
   
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618851463


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   @steveloughran That was my suggestion as well :-)



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "ggershinsky (via GitHub)" <gi...@apache.org>.
ggershinsky commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620152849


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   probably the comment change is not updated?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "ggershinsky (via GitHub)" <gi...@apache.org>.
ggershinsky commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620145697


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   I couldn't find a  `column_metadata_page` field in the updated FileMetaData struct.
   Also, in the ColumnChunk struct, we still keep unencrypted ColumnMetaData (field 3:). Why would we move its encrypted version to a different struct?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621075200


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   So for context I want from 8 bits to 64 bits and I think 32 is a reasonable default.  The intent of this particular bitmap is to indicate to consumers that there is backwards incompatible change.  I think if we had this originally we wouldn't of used a different magic number.  Forward looking I imagine things that might go here if we chose to pursue them:
   1.  Allowing non-continguous pages (i.e. making offset index mandatory).
   2.  Compressing the footer as a whole block (might be beneficial if we get to flatbuffers).
   3. Changing to flatbuffers in the future.
   
   I would hope these would be relatively rare, and the last flag of this bitmap can always be reserved to indicate yet another bitmap.
   
   @wgtmac what use-cases where you thinking of?
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620184774


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   should be fixed now (removed PAR3 comment).



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622614076


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   @wgtmac yeah I was imagining this for much fewer use-cases and I think for features that readers can detect as they read that they don't understand I think it is fine for it to happen lazily.
   
   > Put it differently the only feature we cannot encode inside the footer itself is if the footer is encrypted. For this it seems we can keep using a secondary magic number forever?
   
   @alkis at the very least compression.  If we switch to flatbuffers I believe they compress quite well (a lot of extra padding in integers)?  Would we then have a few more magic footers for the cross product of compression and and encryption?
   
   Again, I think there are only even a handful of imagined use-cases that this can be used which is originally why I had it as a single byte originally, IMO it is a small cost to pay for some potential flexibility. and is useful at least for encryption.
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621089084


##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression

Review Comment:
   I think column data is at least 20 bytes per column? this means we get to ~1kb at 50 column and 4kb at at ~200 which I don't think are unreasonable.  What the relative compressibility is metadata is an open question.  I will try to measure this for wider schemas.  For  smaller structures that use this I think the default would be not to compress.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621094106


##########
src/main/thrift/parquet.thrift:
##########
@@ -1127,18 +1229,48 @@ struct FileMetaData {
    * are flattened to a list by doing a depth-first traversal.
    * The column metadata contains the path in the schema for that column which can be
    * used to map columns to nodes in the schema.
-   * The first element is the root **/
-  2: required list<SchemaElement> schema;
+   * The first element is the root
+   *
+   * PAR1: Required
+   * PAR3: Use schema_page
+   **/
+  2: optional list<SchemaElement> schema;
+
+  /** Page has BYTE_ARRAY data where each element is REQUIRED. 
+    *
+    * Each element is a serialized SchemaElement.  The order and content should
+    * have a one to one correspondence with schema.
+    */
+  10: optional MetadataPage schema_page;

Review Comment:
   My intent was to introduce an encoding that allows zero-copy random access which I think would be better then list<binary> which I would guess might be slightly better.  Plain encoding is effectively equivelant to list<binary> on the wire I believe, and this way we avoid the up front cost of decoding the list.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "JFinis (via GitHub)" <gi...@apache.org>.
JFinis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1626057631


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order
+  /** Set to true if all pages in the column chunk are dictionary 
+    * encoded 
+    */
+  11: optional bool all_pages_dictionary_encoded
+ /** 
+   * The index to the SchemaElement in FileMetadata for this 
+   * column.
+   */
+  12: optional i32 schema_index 

Review Comment:
   As discussed on the mailing list, how about this alternative design:
   
   How about just turning things around: Instead of having a schema_index in the ColumnMetadata, we could have a column_metadata_index in the schema. If that index is missing/-1, then this signifies that the column is empty, so no metadata will be present for it. With this, we would get the best of both worlds: We would always have O(1) random I/O even in case of such empty columns (as we would use the column_metadata_index for the lookup) and we would not need to store any ColumnMetadata for empty columns.
   
   After given this a second thought, this also makes more sense in general. As the navigation direction is usually always from schema to metadata (not vice versa!), the schema should point us to the correct metadata instead of the metadata pointing us to the correct schema entry.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "ggershinsky (via GitHub)" <gi...@apache.org>.
ggershinsky commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620218607


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   Thanks, lgtm.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621141875


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order

Review Comment:
   I think maybe just keeping as is make sense actually, so I'll remove this field and we can keep the list or move that to a DATA_PAGE



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618947985


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   @alkis if this is the unknown thrift field approach, I originally started drafting it up but figured I'd hold off due to @pitrou strong feelings 



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621069838


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.

Review Comment:
   I tend to agree, other had concerns that it was gratuitous.  I can update to make it required.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617739189


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Hmm, this doesn't answer my question, does it?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617718549


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.

Review Comment:
   I don't have strong feelings on size.  I was imagining extensibility could be done by saving the last bit to indicate more bits to read.  Maybe 16 or 32 bits?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617731016


##########
src/main/thrift/parquet.thrift:
##########
@@ -537,6 +537,39 @@ enum Encoding {
       Support for INT32, INT64 and FIXED_LEN_BYTE_ARRAY added in 2.11.
    */
   BYTE_STREAM_SPLIT = 9;
+
+  /** Encoding for variable length binary data that allows random access of values.

Review Comment:
   Off topic, but I really like this idea as an alternative to PLAIN encoding for BYTE_ARRAY data. It has the advantages of DELTA_LENGTH_BYTE_ARRAY without having to do delta encoding for the offsets. Random access (in the absence of nulls) is just an added bonus.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617284195


##########
src/main/thrift/parquet.thrift:
##########
@@ -537,6 +537,39 @@ enum Encoding {
       Support for INT32, INT64 and FIXED_LEN_BYTE_ARRAY added in 2.11.
    */
   BYTE_STREAM_SPLIT = 9;
+
+  /** Encoding for variable length binary data that allows random access of values.

Review Comment:
   Can we decouple this from the V3 footer proposal?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2465600637

   Closing for now, as I think there seems to be more support for flatbuffers.


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1635107904


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order
+  /** Set to true if all pages in the column chunk are dictionary 
+    * encoded 
+    */
+  11: optional bool all_pages_dictionary_encoded
+ /** 
+   * The index to the SchemaElement in FileMetadata for this 
+   * column.
+   */
+  12: optional i32 schema_index 

Review Comment:
   @JFinis this would only be useful for columns which are empty throughout all row groups, right? As opposed to skipping _some_ column chunks but not all.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617289140


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.

Review Comment:
   Can we please make this 8 bytes? We don't want to run out of bits for no good reason.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617716430


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Othere fields a length delimited.  Docs: https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md#struct-encoding



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620175482


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   Yes, missed this will update the comment.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618810904


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   There is a way to avoid this. I am preparing a proposal for it.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620960147


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   I'm in favor of the feature flags and it in turn provides unified `PAR3` magic. However, I'm a little bit concerned with the 4 byte limit. Or maybe we need to clarify  what kind of features are qualified to be added to it.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618919895


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Ok, I'll rephrase



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620103875


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   OK, done for now. 



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617749807


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Sorry I think I might have misunderstood your question.  I thought you were asking if 0x00 can appear elsewhere in serialized data, and the answer is yes, but it doesn't introduce ambiguity.
   
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617839545


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Sorry I tried to clean up some grammar/typos and this might have gone out of date.  
   
   I'm not sure what reference you are referring to (I'll reread and try to do another pass).  I think the reasons for mentioning it are either:
   1.  A prior iteration of this which described a way to gaurantee a thrift-parser would ignore the PAR3 footer by simulating an unknown field ID.
   2. As an explanation to build up-to why we believe the current proposal is safe.  In short: FileMetadata is a struct.  When serialized it will always end with 0x00.  As long as implementations do not do any padding with PAR3 after serializaing FileMetadata today, "PAR3" after the offset the offset is an unambiguous signal that a PAR3 footer is present.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617288677


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.

Review Comment:
   Just my 2 cents, but I'd rather see a sketch of the actual layout, rather than a description of a strategy :-)
   (especially as the strategy is best left to each implementation, depending on their specific constraints and tradeoffs)



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621091946


##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression
+   // Number of elements stored.  This is duplicated here to help in 
+   // use-cases where knowing the total number of elements up front for
+   // computation would be useful.
+   3: num_values

Review Comment:
   For fixed width types PLAIN encoding gives us this data.
   
   For variable width types, I wanted to solve this in a more general way with the RANDOM_ACCESS_BYTE_ARRAY so that users could choose this to allow for random access.  In future iterations when we tackle improving for "point lookup" use cases I imagine that would also help or be another option.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621083889


##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.

Review Comment:
   This is also a reasonable approach to define a new page type.  I was leaving this open in case in the future we want nulls.  Whether nulls are allowed and exact structure is dictated by its use on the field and also to minimize spec changes in this draft.  The nice thing about this approach is it can work transparently if/when a new page type is added.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622708989


##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size

Review Comment:
   I would think `uncompressed_size` at a minimum would have to remain i64. I've seen single row groups with string columns that exceed 2GB in size. I'd argue the same for `total_compressed_size` on uncompressed data. 



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621075786


##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.

Review Comment:
   yes, will fix.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620177505


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +926,47 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata

Review Comment:
   I removed the global struct.  There is now a new field on RowGroup that stores the ColumnChunks differently.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621567333


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   I was thinking to leverage the features flag to continue the effort in https://github.com/apache/parquet-format/pull/164 which defines core features that the parquet file implements.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621045335


##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.

Review Comment:
   So the page will have a PageHeader/DataPageHeader at the top? Will nulls or repetition be allowed, i.e. do we need definition and repetition level data? If not, then should we define a new page type instead so we don't have to encode unused level encoding types? Then we could also drop the language below about not writing statistics.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622614076


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   @wgtmac yeah I was imagining this for much fewer use-cases and I think for features that readers can detect as they read that they don't understand I think it is fine for it to happen lazily.
   
   > Put it differently the only feature we cannot encode inside the footer itself is if the footer is encrypted. For this it seems we can keep using a secondary magic number forever?
   
   @alkis at the very least compression.  If we switch to flatbuffers I believe they compress quite well?  Would we then have a few more magic footers for the cross product of compression and and encryption?
   
   Again, I think there are only even a handful of imagined use-cases that this can be used which is originally why I had it as a single byte originally, IMO it is a small cost to pay for some potential flexibility. and is useful at least for encryption.
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622586010


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.

Review Comment:
   there's generally an expectation that filesystems do validation, but CRCs are relatively low cost, and help find problems in networking and NIC



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "JFinis (via GitHub)" <gi...@apache.org>.
JFinis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1624058195


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order
+  /** Set to true if all pages in the column chunk are dictionary 
+    * encoded 
+    */
+  11: optional bool all_pages_dictionary_encoded
+ /** 
+   * The index to the SchemaElement in FileMetadata for this 
+   * column.
+   */
+  12: optional i32 schema_index 

Review Comment:
   Also note that without the order being the same as the schema, the whole "random access" idea of this PR goes out of the window.
   
   It doesn't help me to have a MetadataPage with random access, if I don't know which column I need to access in the first place.
   
   Instead, what I want is that if I want to access the third column in the schema, then I need to access the third column chunk.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622616405


##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression

Review Comment:
   @alkis I'm OK either way since compression is already a supported concept I thought this was a low cost effort that might have value in some use cases.
   
   As a litmus test, I'm curious what happens if you run LZ4 on the footers you have been experimenting with?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "mhaseeb123 (via GitHub)" <gi...@apache.org>.
mhaseeb123 commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1637433667


##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.
+    **/
   8: optional list<KeyValue> key_value_metadata
 
+  /** See description on FileMetata.key_value_metadata **/
+  19: optional MetadataPage key_value_metadata_page
+
   /** Byte offset from beginning of file to first data page **/
   9: required i64 data_page_offset

Review Comment:
   > This is almost always the same as the start of the columnchunk. We should make this optional and imply it is the same as `ColumnChunk.file_offset` if missing.
   
   Makes sense
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617739978


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.

Review Comment:
   Why limit ourselves while it would cost basically nothing to go for 64 bits from the start?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617721216


##########
src/main/thrift/parquet.thrift:
##########
@@ -779,8 +812,12 @@ struct ColumnMetaData {
    * whether we can decode those pages. **/
   2: required list<Encoding> encodings
 
-  /** Path in schema **/
-  3: required list<string> path_in_schema
+  /** Path in schema 
+    *  Example of deprecated a field for PAR3

Review Comment:
   I think there are trade-offs I replied on the mailing list with more details (hopefully we can centralize discussion there).



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2137564885

   > Finally, is there anything needed to support an extended par3 trailing footer in future?
   
   Sorry I missed this.  I think the bitmap gives flexibility for extending par3.  Or was there something else you were concerned with?


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2139554509

   > By splitting metadata we are fixing (1) but we are requiring a secondary fetch from the object store that _depends_ on the first fetch and we make (2) twice as long. At this point we are back to square 1...
   
   @alkis I think I might have misphrased something here or misdesigned something here. Where do you see the require second fetch as a requirement.  I imagine performant readers will still read a significant portion of the tail of the file to avoid the two fetches.


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620989728


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   Sorry I just saw line 943. I meant field 17 is not required any more.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621081568


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order

Review Comment:
   Schema element sounds OK to me we can also leave as is, I think I copied this from #242 



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621094453


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order

Review Comment:
   I'll move.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "JFinis (via GitHub)" <gi...@apache.org>.
JFinis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1624048091


##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.
+    **/
   8: optional list<KeyValue> key_value_metadata
 
+  /** See description on FileMetata.key_value_metadata **/
+  19: optional MetadataPage key_value_metadata_page
+
   /** Byte offset from beginning of file to first data page **/
   9: required i64 data_page_offset

Review Comment:
   If we indeed put the ColumnMetaData in front of the chunk, then this would no longer be true.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617719522


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.

Review Comment:
   I can add this if we decide to move forward with this PR (as compared to the your proposal).



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622701397


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order

Review Comment:
   here is #221 (follow on to #196)



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617292511


##########
src/main/thrift/parquet.thrift:
##########
@@ -779,8 +812,12 @@ struct ColumnMetaData {
    * whether we can decode those pages. **/
   2: required list<Encoding> encodings
 
-  /** Path in schema **/
-  3: required list<string> path_in_schema
+  /** Path in schema 
+    *  Example of deprecated a field for PAR3

Review Comment:
   Obviously I'm a bit biased, but I find the approach of defining separate structs (`FileMetadataV3`, etc.) much cleaner than tediously documenting what is required in V1 and forbidden in V3, and vice-versa.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620073240


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   Left a TODO in the readme to figure out desired approach.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "ggershinsky (via GitHub)" <gi...@apache.org>.
ggershinsky commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620156814


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +926,47 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata

Review Comment:
   this field should be moved (or copied for backward compatibility) to a per-column struct that applies to all row groups / chunks. If we have such a struct.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "ggershinsky (via GitHub)" <gi...@apache.org>.
ggershinsky commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620050809


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   We can move (copy) the field 8 (`optional ColumnCryptoMetaData crypto_metadata`) to a V3 per column metadata structure that applies to all row groups / chunks. The field 9 (`optional binary encrypted_column_metadata`) is simply the encrypted version of field 3 (`optional ColumnMetaData meta_data`) - to hide the stats - so should be set per chunk (row group).
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620997989


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   But I just got this implemented 😭 😉. Joking aside, is there any other use for this field beyond optimizing dictionary based queries?
   
   Edit: looking above, encodings is now deprecated, so this field replaces that and serves the purpose of @alkis's boolean field. So if we adopt the latter, encodings will need to remain required I think.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621186564


##########
src/main/thrift/parquet.thrift:
##########
@@ -1127,18 +1229,48 @@ struct FileMetaData {
    * are flattened to a list by doing a depth-first traversal.
    * The column metadata contains the path in the schema for that column which can be
    * used to map columns to nodes in the schema.
-   * The first element is the root **/
-  2: required list<SchemaElement> schema;
+   * The first element is the root
+   *
+   * PAR1: Required
+   * PAR3: Use schema_page
+   **/
+  2: optional list<SchemaElement> schema;
+
+  /** Page has BYTE_ARRAY data where each element is REQUIRED. 
+    *
+    * Each element is a serialized SchemaElement.  The order and content should
+    * have a one to one correspondence with schema.
+    */
+  10: optional MetadataPage schema_page;

Review Comment:
   A little bit I had a few ideas that I haven't had to write down.  Starting from the easiest:
   1. For formats like Iceberg that use field_id three columns (exact physical layout TBD): <sorted field IDs><schema index><leaf column index>
   2. For name based indexes, I was thinking of flattened list of schema elements, that is flattened in bread-first order.  At each level we sort the column names (I think there are likely two colations that are might be useful, normal lexicographic, and case-insensitive (all column names are normalized to lower case).  The column names are associated with an offset and width to there children.  There are potentially more complex data structures that could make this more efficient but it seems like a reasonable start.
   
   In both cases I think we need to potentially make a decision if this is 1 to 1 or 1 to many (e.g. I've seen parquet files with the duplicate column names that differ only by whether they are upper case or lower case.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2140396764

   > (e.g. if they happen to be exposing thrift structures as a public API)
   
   what do we know here about this? which apps do/don't?


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield closed pull request #250: DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet 
URL: https://github.com/apache/parquet-format/pull/250


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1619192894


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   I'll revise this and move the design considerations elsewhere.
   
   @alkis if this was the approached you sketched on https://github.com/apache/parquet-format/pull/242/files#r1607838732 since I'm revising this anyways, i can incorporate it into the PR so we can consolidate discussion.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622718278


##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.
+    **/
   8: optional list<KeyValue> key_value_metadata
 
+  /** See description on FileMetata.key_value_metadata **/
+  19: optional MetadataPage key_value_metadata_page

Review Comment:
   Maybe in the weeds, but how would one indicate a null value for a key and distinguish it from an empty string? (value is optional in `KeyValue`).



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2141083352

   > > (e.g. if they happen to be exposing thrift structures as a public API)
   > 
   > what do we know here about this? which apps do/don't?
   
   @steveloughran There is a preliminary investigation: https://docs.google.com/document/d/1PQpY418LkIDHMFYCY8ne_G-CFpThK15LLpzWYbc7rFU/edit#heading=h.wkbvu2wh1ahj


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621070736


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.

Review Comment:
   I'm OK adding this.  As a counter pointer 100s of MB would ideally be rejected by reasonable memory limitations on the footer.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622427576


##########
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?
   
   This doesn't matter much for Thrift, but if we are happy with such a change, it makes a difference for other encodings like flatbuffers.
   
   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.



##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.
+    **/
   8: optional list<KeyValue> key_value_metadata
 
+  /** See description on FileMetata.key_value_metadata **/
+  19: optional MetadataPage key_value_metadata_page

Review Comment:
   This is unlikely to make a huge difference. In case it does, should we consider this also:
   
   ```thrift
   struct KeyVals {
     1: optional list<string> keys;
     2: optional list<binary> vals;
   }
   ```
   



##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression

Review Comment:
   I suggest not doing this at all unless benchmarks show it is a large win. From experience with general purpose compressors (lempel ziv style) they fail to compress small entities like ints or ulebs. Since that's the majority of stuff we will put here, I do not expect compression to be profitable.



##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   What are the benefits of feature flags in this place in the file?
   
   We have `FileMetaData.version` which can serve the purpose of feature flags for everything, except encrypted footer (since we can't otherwise read them). Put it differently the only feature we cannot encode inside the footer itself is if the footer is encrypted. For this it seems we can keep using a secondary magic number forever?



##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order

Review Comment:
   Do we have examples of other orders that might be useful?
   Can drop this concept alltogether and make only one ordering available?



##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.
+    **/
   8: optional list<KeyValue> key_value_metadata
 
+  /** See description on FileMetata.key_value_metadata **/
+  19: optional MetadataPage key_value_metadata_page
+
   /** Byte offset from beginning of file to first data page **/
   9: required i64 data_page_offset

Review Comment:
   This is almost always the same as the start of the columnchunk. We should make this optional and imply it is the same as `ColumnChunk.file_offset` if missing.



##########
src/main/thrift/parquet.thrift:
##########
@@ -1127,18 +1229,48 @@ struct FileMetaData {
    * are flattened to a list by doing a depth-first traversal.
    * The column metadata contains the path in the schema for that column which can be
    * used to map columns to nodes in the schema.
-   * The first element is the root **/
-  2: required list<SchemaElement> schema;
+   * The first element is the root
+   *
+   * PAR1: Required
+   * PAR3: Use schema_page
+   **/
+  2: optional list<SchemaElement> schema;
+
+  /** Page has BYTE_ARRAY data where each element is REQUIRED. 
+    *
+    * Each element is a serialized SchemaElement.  The order and content should
+    * have a one to one correspondence with schema.
+    */
+  10: optional MetadataPage schema_page;

Review Comment:
   > My intent was to introduce an encoding that allows zero-copy random access which I think would be better then list which I would guess might be slightly better.
   
   Benchmarks can guide this. From my experiments decoding `list<binary>` is so much faster than `list<MyStruct>` that it doesn't show in profiles anymore. Plus this field is O(columns). The ones we really have to optimize are the column chunks that are O(columns * row groups).



##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   Can we drop `encodings`? The purpose is to detect if a reader can read a column or not. What's going to happen if we don't have that information? Reader will try to read and maybe will fail the query mid way. Isn't the latter an acceptable behavior as well?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618947985


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   @alkis if this is the unknown thrift field approach, I originally started drafting it up but figured I'd hold off due to @pitrou strong feelings but I think it is likely worth seeing it on paper which might help weight the pros/cons.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621080671


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   Yes I think it serves three purposes:
   1.  consolidate encodings.
   2. Determine optimizations available in the engine (e.g. fully dictionary encoded, fully RLE, etc)
   3. Instrumentation for engines to understand what the distribution of data they see to prioritize optimizations for specific encodings.
   
   I'm OK removing completely but my concern around #2 is spec changes take a while to propagate, having the information for engines to determine what optimizations they want to do without requiring a new flag I think is useful.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2139855539

   > @alkis I think I might have misphrased something here or misdesigned something here. Where do you see the require second fetch as a requirement. I imagine performant readers will still read a significant portion of the tail of the file to avoid the two fetches.
   
   Apologies that was my misunderstanding. I conflated #242 which has offsets for metadata elsewhere.
   
   Now that I understand this better it seems that the main trick we are employing is converting `list<ExpensiveToDecodePerColumnStruct>` to something like `list<binary>` (with optional compression) to make deserialization lazy.
   1. How is this going to affect workloads that read all the columns (and thus require all the metadata). Are they getting any benefit?
   2. The effort to fix readers is not going to be trivial. Perhaps it will be similar if we revamp the schema substantially. Why should we stick to thrift if we are going through the effort to refactor readers?


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2140331049

   > Now that I understand this better it seems that the main trick we are employing is converting `list<ExpensiveToDecodePerColumnStruct>` to something like `list<binary>` (with optional compression) to make deserialization lazy.
   > 
   > 1. How is this going to affect workloads that read all the columns (and thus require all the metadata). Are they getting any benefit?
   
   No it does not.  How much regression is present is something we need to benchmark.
   
   > 2. The effort to fix readers is not going to be trivial. Perhaps it will be similar if we revamp the schema substantially. Why should we stick to thrift if we are going through the effort to refactor readers?
   
   I think we might have different priors on effort involved and scope of changes for readers.  I was hoping with these changes existing readers could update to use it without the benefit of laziness in O(hours) - O(days) (e.g. if they happen to be exposing thrift structures as a public API in there system like DuckDB).  If systems already have abstractions in place for metadata making use of the laziness should also be pretty quick.  I see changing to a different encoding format as more efforts and when multiplied across the number of custom parquet readers makes it less likely for ecosystem penetration or at least take longer.  I don't want to rule out moving to flatbuffers in the future (this is yet another reason for the feature bitmap).
   
   CC @JFinis I think your perspective here might be valuable here.
   


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621150969


##########
src/main/thrift/parquet.thrift:
##########
@@ -1127,18 +1229,48 @@ struct FileMetaData {
    * are flattened to a list by doing a depth-first traversal.
    * The column metadata contains the path in the schema for that column which can be
    * used to map columns to nodes in the schema.
-   * The first element is the root **/
-  2: required list<SchemaElement> schema;
+   * The first element is the root
+   *
+   * PAR1: Required
+   * PAR3: Use schema_page
+   **/
+  2: optional list<SchemaElement> schema;
+
+  /** Page has BYTE_ARRAY data where each element is REQUIRED. 
+    *
+    * Each element is a serialized SchemaElement.  The order and content should
+    * have a one to one correspondence with schema.
+    */
+  10: optional MetadataPage schema_page;

Review Comment:
   @emkornfield have you given more thought to a schema index? One use for random access is column projection, but how does one figure out column indexes for the projection without fully decoding the schema?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "jhorstmann (via GitHub)" <gi...@apache.org>.
jhorstmann commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622608438


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   If `ColumnMetaData::encodings` could distinguish between PLAIN encoded dictionary and PLAIN data encodings, the detection of full dictionary encoding could be based on that without page specific statistics. If `Encoding::PLAIN_DICTIONARY` could be un-deprecated and indicate a PLAIN encoded dictionary page then `encoding_stats` might not be needed for most usecases. A list of `[PLAIN_DICTIONARY, RLE_DICTIONARY]` could indicate full dictionary encoding for the column.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2143369480

   @alkis posted a question to ML to get more thoughts on thrift vs flat buffers.  I think before we commit fully to flat buffers I might try to prototype a more shredded version of this that effectively uses mini parquet files for each of the core structs to see what that might look like


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620467287


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.

Review Comment:
   I recommend a crc32 of the length itself. This is to detect early that a footer is corrupt and avoid reading some epic amount of garbage from the end of the file. For example think of a bit flip in one of the top bits of the length, it will cause a reader to read 100s of MBs of the end of the file only to check that the crc doesn't match.



##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression
+   // Number of elements stored.  This is duplicated here to help in 
+   // use-cases where knowing the total number of elements up front for
+   // computation would be useful.
+   3: num_values

Review Comment:
   missing type?



##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   I don't find this flag particularly elegant. There is already a magic number. We can employ the same mechanism as PAR1/PARE had to distinguish an encrypted footer.



##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.

Review Comment:
   This should not be optional.
   
   All bytes are not equal in a file. In particular footer bytes are very important because if those are corrupt - we can't read any bytes of the file. If anything Footers not having a required checksum for their content is a design flaw of the original parquet specification.



##########
src/main/thrift/parquet.thrift:
##########
@@ -792,12 +806,23 @@ struct ColumnMetaData {
   6: required i64 total_uncompressed_size
 
   /** total byte size of all compressed, and potentially encrypted, pages 
-   *  in this column chunk (including the headers) **/
+   *  in this column chunk (including the headers) 
+   * 
+   *  Fetching the range of min(dictionary_page_offset, data_page_offset) 
+   *  + total_compressed_size should fetch all data in the the given column 
+   * chunk.
+   */
   7: required i64 total_compressed_size
 
-  /** Optional key/value metadata **/
+  /** Optional key/value metadata 
+    * PAR1: Optional. 
+    * PAR3: Don't write use key_value_metadata instead.

Review Comment:
   typo?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617030946


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   there's going be a need for some regression tests here, maybe module to validate output from parquet-java against parquet readers from: older parquet-java release(s), parquet-cpp, impala parser, parquet dotnet. Because these are the older/frozen releases, a docker image with everything installed should work.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617787677


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   But then, what is the point of mentioning a trailing byte above?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618850716


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   I know this is a draft PR, but I would find this easier to read if the README concentrated on describing the format at a high level view, rather than explaining specifics of Thrift encoding. If need be, those can be detailed in a separate design document (perhaps a RFC-like document? see also the typical structure of [Python PEPs](https://peps.python.org/pep-0000/)).



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621008508


##########
src/main/thrift/parquet.thrift:
##########
@@ -1115,6 +1189,34 @@ union EncryptionAlgorithm {
   2: AesGcmCtrV1 AES_GCM_CTR_V1
 }
 
+/**
+ * Embedded metadata page.
+ * 
+ * A metadata page is a data page used to store metadata about
+ * the data stored in the file. This is a key feature of PAR3
+ * footers which allow for deferred decoding of metadata.
+ *
+ * For common use cases the current recommendation is to use a 
+ * an encoding that supported random access but implementations may choose
+ * other configuration parameters if necessary. Implementations
+ * SHOULD consider allowing configurability per page to allow for end-users
+ * to optimize size vs compute trade-offs that make sense for their use-case.
+ *
+ * Statistics for Metadata pages SHOULD NOT be written.
+ *
+ * Structs of this type should never be written in PAR1.
+ */
+struct MetadataPage {
+   // A serialized page including metadata thrift header and data.
+   1: required binary page
+   // Optional compression applied to the page.
+   2: optional CompressionCodec compression
+   // Number of elements stored.  This is duplicated here to help in 
+   // use-cases where knowing the total number of elements up front for
+   // computation would be useful.
+   3: num_values

Review Comment:
   Is it possible to add a `list<i32>` with `num_values + 1` size to indicate the offsets to each element before compression to support random access? Or is it supposed to use fixed-length element at all times?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620984136


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   I think it makes sense to adopt @alkis's idea of using a boolean field here to indicate whether dictionary encoding is applied to all data pages.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620076649


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   simplified to just the layout with a TODO to decide on final layout based on other proposal.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620084172


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.

Review Comment:
   Change this to just describe the layout.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617285553


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   I think it would be sufficient to generate dummy PAR1 file with extraneous padding at the end of the FileMetadata serialization, and see if all implementations can still read it.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617282754


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  

Review Comment:
   Does it mean a 0 bytes cannot appear anywhere else in a Thrift serialization? Is this mentioned somewhere in the Thrift specs?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622617792


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   yes, I thought I had marked encodings as dropped (as the information here duplicates it at the very least), if not I'll update it.  I'm also OK with readers discovering unreadable pages lazily.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1620063065


##########
src/main/thrift/parquet.thrift:
##########
@@ -881,15 +922,21 @@ struct ColumnChunk {
   /** Crypto metadata of encrypted columns **/
   8: optional ColumnCryptoMetaData crypto_metadata
 
-  /** Encrypted column metadata for this chunk **/
+  /** Encrypted column metadata for this chunk 
+    *  
+    * PAR3: Not set see column_metadata_page on FileMetadata struct

Review Comment:
   I'm actually moving this back here (doing a pretty large refactor, would you mind seeing if this comment still applies?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1621075200


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 4-byte little-endian flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.

Review Comment:
   So for context I want from 8 bits to 64 bits and I think 32 is a reasonable default.  The intent of this particular bitmap is to indicate to consumers that there is backwards incompatible change.  I think if we had this originally we wouldn't of used a different magic number.  Forward looking I imagine things that might go here if we chose to pursue them:
   1.  Allowing non-continguous pages (i.e. making offset index mandatory).
   2.  Compressing the footer as a whole block (might be beneficial if we get to flatbuffers).
   
   I would hope these would be relatively rare, and the last flag of this bitmap can always be reserved to indicate yet another bitmap.
   
   @wgtmac what use-cases where you thinking of?
   



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1622638874


##########
src/main/thrift/parquet.thrift:
##########
@@ -812,8 +837,20 @@ struct ColumnMetaData {
 
   /** Set of all encodings used for pages in this column chunk.
    * This information can be used to determine if all data pages are
-   * dictionary encoded for example **/
+   * dictionary encoded for example  
+   *
+   *  PAR1: Optional. May be deprecated in a future release in favor
+   *        serialized_encoding_stats.
+   *  PAR3: Don't populate.  Write serialized_page_encoding_stats.
+   **/
   13: optional list<PageEncodingStats> encoding_stats;
+  /** 
+    * Serialized page encoding stats.
+    *
+    * PAR1: Start populating after encoding_stats is deprecated.
+    * PAR3: Populate instead of encoding_stats.
+    */
+  17: optional binary serialized_encoding_stats

Review Comment:
   @jhorstmann this seems plausible however, I still think there is use in this data from a debuggability standpoint to elaborate on #3 I shouldn't have just limited it to engines.  Being able to dump encodings and how many there are quickly allows users to understand a little bit better how there data is being compressed and let them potentially fine tune these aspects.  It isn't a frequent occurrence but there is certainly cases when having a little bit more data helps with debugability.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1617741697


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.

Review Comment:
   I'll update to 64 bits.



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1618835790


##########
README.md:
##########
@@ -118,6 +118,65 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control
+ over the various footer size vs compute trade-offs.  Its format is as follows:
+   - Data pages containing serialized Thrift metadata objects that were modeled as lists
+     in PAR1.These are stored contiguously with offsets stored in the FileMetadata. See 
+     parquet.thrift for more details on each.
+   - Serialized Thrift FileMetadata Structure
+   - (Optional) 4 byte CRC32 of the serialized Thrift FileMetadata.
+   - 4-byte length in bytes (little endian) of the serialized FileMetadata structure.
+   - 4-byte length in bytes (little endian) of all preceding elements in the footer.
+   - 1 byte flag field to indicate features that require special parsing of the footer.
+     Readers MUST raise an error if there is an unrecognized flag.  Current flags:
+
+     * 0x01 - Footer encryption enabled (when set the encryption information is written before 
+        FileMeta structure as in the PAR1 footer).
+     * 0x02 - CRC32 of FileMetadata Footer.
+
+   - 4-byte magic number "PAR3"
+
+  When parsing the footer implementations SHOULD read at least the last 10 bytes of the footer. Then
+  read in the entirety of the footer based on the length of all preceding elements. This prevents further
+  I/O cost for accessing metadata stored in the data pages. PAR3 footers can fully replace PAR1 footers.
+  If a file is written with only PAR3 footer, implementation MUT write PAR3 as the first four bytes in 
+  they file. PAR3 footers can also be written in a backwards compatible way after PAR1 Metadata 
+  (see next section for details).
+
+  #### Dual Mode PAR1 and PAR3 footers
+
+  There is a desire to gradually rollout PAR3 footers to allow newer readers to take advantage of them, while
+  older readers can still properly parse the file. This section outlines a strategy to do this.
+
+  As backgroud, Thrift structs are always serialized with a 0 trailing byte do delimit there ending.  
+  Therefore for PAR1 written before PAR3 was introduced are always expect the files to have the following 
+  trailing 9 bytes [0x00, x, x, x, x, P, A, R, 1] (where x can be any value).  We also expect all compliant 
+  Thrift parsers to only parse the first available FileMetadata message and stop consuming the stream once read. 
+  Today, we don't believe that any Parquet readers validate that the entire "length in bytes of file metadata" 

Review Comment:
   I was thinking of doing this just by modifying parquet-java to add some trailing bytes and see what broke in the rest of the code, that is: make no changes to the reader



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #250:
URL: https://github.com/apache/parquet-format/pull/250#issuecomment-2145560756

   > @alkis posted a question to ML to get more thoughts on thrift vs flat buffers. I think before we commit fully to flat buffers I might try to prototype a more shredded version of this that effectively uses mini parquet files for each of the core structs to see what that might look like
   
   It seems like we might be headed towards flatbuffers as a consenus, so I will put this on the back-burner.


-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "JFinis (via GitHub)" <gi...@apache.org>.
JFinis commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1624053258


##########
src/main/thrift/parquet.thrift:
##########
@@ -883,13 +928,42 @@ struct ColumnChunk {
 
   /** Encrypted column metadata for this chunk **/
   9: optional binary encrypted_column_metadata
+  /** 
+    * The column order for this chunk.
+    * 
+    * If not set readers should check FileMetadata.column_orders
+    * instead.
+    *
+    * Populated in both PAR1 and PAR3
+    */
+  10: optional ColumnOrder column_order
+  /** Set to true if all pages in the column chunk are dictionary 
+    * encoded 
+    */
+  11: optional bool all_pages_dictionary_encoded
+ /** 
+   * The index to the SchemaElement in FileMetadata for this 
+   * column.
+   */
+  12: optional i32 schema_index 

Review Comment:
   what use case is there for having column chunks be out of order? Wouldn't it be easier to just specify that the column chunks have to be in the same order as the schema?



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org


Re: [PR] DRAFT: Alternative Strawman proposal for a new V3 footer format in Parquet [parquet-format]

Posted by "mhaseeb123 (via GitHub)" <gi...@apache.org>.
mhaseeb123 commented on code in PR #250:
URL: https://github.com/apache/parquet-format/pull/250#discussion_r1637412759


##########
README.md:
##########
@@ -118,6 +118,51 @@ chunks they are interested in.  The columns chunks should then be read sequentia
 
  ![File Layout](https://raw.github.com/apache/parquet-format/master/doc/images/FileLayout.gif)
 
+ ### PAR3 File Footers
+
+ PAR3 file footer footer format designed to better support wider-schemas and more control

Review Comment:
   Minor:
   ```suggestion
    PAR3 file footer footer format designed to better support wider-schemas and more control
   ```
   ```suggestion
    PAR3 file footer format designed to better support wider-schemas and more control
   ```



-- 
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: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org