JFinis commented on code in PR #449:
URL: https://github.com/apache/parquet-format/pull/449#discussion_r1730336950
##########
src/main/thrift/parquet.thrift:
##########
@@ -257,7 +257,14 @@ struct Statistics {
*/
1: optional binary max;
2: optional binary min;
- /** count of null value in the column */
+ /**
+ * Count of null values in the column.
+ *
+ * Writers SHOULD always write this field even if it is zero (i.e. no null
value)
+ * or the column is not nullable.
Review Comment:
Why should writers write this field if the column isn't nullable? That feels
strictly redundant and therefore a waste of memory. If a reader assumes that
their could be nulls even though the type is required in the schema, then this
reader has a bug. There wouldn't even be a way to write nulls into a required
column, as they cannot be represented in this case.
(Also, if you disregard this comment, at least change the wording from `not
nullable` to `not required`, as that's the correct term w.rt. to the schema
(there is no `nullable` field in the schema, just a repetition type).)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
