emkornfield commented on code in PR #197:
URL: https://github.com/apache/parquet-format/pull/197#discussion_r1313882045


##########
src/main/thrift/parquet.thrift:
##########
@@ -191,6 +191,74 @@ enum FieldRepetitionType {
   REPEATED = 2;
 }
 
+/**
+  * A histogram of repetition and definition levels for either a page or column
+  * chunk.
+  *
+  * This is useful for:
+  *   1. Estimating the size of the data when materialized in
+  *   memory
+  *
+  *   2. For filter push-down on nulls at various levels of nested
+  *   structures and list lengths.
+  */
+struct RepetitionDefinitionLevelHistogram {
+   /**
+    * When present, there is expected to be one element corresponding to each
+    * repetition (i.e. size=max repetition_level+1) where each element
+    * represents the number of times the repetition level was observed in the
+    * data.
+    *
+    * This value should not be written if max_repetition_level is 0.
+    **/
+   1: optional list<i64> repetition_level_histogram;
+   /**
+    * Same as repetition_level_histogram except for definition levels.
+    *
+    * This value should not be written if max_definition_level is 0 or 1.

Review Comment:
   @pitrou thinking about this some more since these are new fields anyways, I 
think it might address complexity concerns if we change "should not" with "may 
not".  This makes writers that don't want to deal with conditionals here free 
to add them.  Readers need to be robust anyways to not having these fields and 
in worst case it means some optimizations might not be applied if they don't 
want to reconstruct state from other locations?



-- 
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: dev-unsubscr...@parquet.apache.org

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

Reply via email to