divjotarora commented on code in PR #3819:
URL: https://github.com/apache/parquet-java/pull/3819#discussion_r4102438334
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -828,35 +828,43 @@ public static Statistics toParquetStatistics(
// rationale is that some engines may use the minimum value in the page as
// the true minimum for aggregations and there is no way to mark that a
// value has been truncated and is a lower bound and not in the page.
- if (!stats.isEmpty() && withinLimit(stats, truncateLength)) {
- formatStats.setNull_count(stats.getNumNulls());
- if (stats.isNanCountSet()) {
- formatStats.setNan_count(stats.getNanCount());
+ if (stats.isEmpty()) {
+ return formatStats;
+ }
+
+ // null_count is independent of the min/max size limit and remains useful
+ // when min/max values are omitted.
+ formatStats.setNull_count(stats.getNumNulls());
+ if (!withinLimit(stats, truncateLength)) {
+ return formatStats;
+ }
+
+ if (stats.isNanCountSet()) {
+ formatStats.setNan_count(stats.getNanCount());
+ }
Review Comment:
Why not move this before the `withinLimit` check as well? Seems the
reasoning can be that fixed-width stats (e.g. counts) can always be published
while variable-length stats (min/max) are published only if `withinLimit`.
--
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]
