PARQUET-1217: fix handling of unset nullCount

Author: Gabor Szadovszky

parquet-cli/src/main/java/org/apache/parquet/cli/commands/ParquetMetadataCommand.java

@@ -168,12 +168,12 @@ private void printColumnChunk(Logger console, int width, ColumnChunkMetaData col
     if (typeName == PrimitiveType.PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY) {
       console.info(String.format("%-" + width + "s  FIXED[%d] %s %-7s %-9d %-8s %-7s %s",
           name, type.getTypeLength(), shortCodec(codec), encodingSummary, count,
-          humanReadable(perValue), stats == null ? "" : String.valueOf(stats.getNumNulls()),
+          humanReadable(perValue), stats == null || !stats.isNumNullsSet() ? "" : String.valueOf(stats.getNumNulls()),
           minMaxAsString(stats, type.getOriginalType())));
     } else {
       console.info(String.format("%-" + width + "s  %-9s %s %-7s %-9d %-10s %-7s %s",
           name, typeName, shortCodec(codec), encodingSummary, count, humanReadable(perValue),
-          stats == null ? "" : String.valueOf(stats.getNumNulls()),
+          stats == null || !stats.isNumNullsSet() ? "" : String.valueOf(stats.getNumNulls()),
           minMaxAsString(stats, type.getOriginalType())));
     }
   }

parquet-cli/src/main/java/org/apache/parquet/cli/commands/ShowPagesCommand.java

@@ -191,7 +191,7 @@ public String visit(DataPageV1 page) {
       String enc = encodingAsString(page.getValueEncoding(), false);
       long totalSize = page.getCompressedSize();
       int count = page.getValueCount();
-      long numNulls = page.getStatistics().getNumNulls();
+      String numNulls = page.getStatistics().isNumNullsSet() ? Long.toString(page.getStatistics().getNumNulls()) : "";
       float perValue = ((float) totalSize) / count;
       String minMax = minMaxAsString(page.getStatistics(), type.getOriginalType());
       return String.format("%3d-%-3d  %-5s %s %-2s %-7d %-10s %-10s %-8s %-7s %s",

parquet-column/src/main/java/org/apache/parquet/column/statistics/Statistics.java

@@ -35,6 +35,44 @@
  */
 public abstract class Statistics<T extends Comparable<T>> {
 
+  /**
+   * Builder class to build Statistics objects. Used to read the statistics from the Parquet file.
+   */
+  public static class StatisticsBuilder {
+    private final PrimitiveType type;
+    private byte[] min;
+    private byte[] max;
+    private long numNulls = -1;
+
+    private StatisticsBuilder(PrimitiveType type) {
+      this.type = type;
+    }
+
+    public StatisticsBuilder withMin(byte[] min) {
+      this.min = min;
+      return this;
+    }
+
+    public StatisticsBuilder withMax(byte[] max) {
+      this.max = max;
+      return this;
+    }
+
+    public StatisticsBuilder withNumNulls(long numNulls) {
+      this.numNulls = numNulls;
+      return this;
+    }
+
+    public Statistics<?> build() {
+      Statistics<?> stats = createStats(type);
+      if (min != null && max != null) {
+        stats.setMinMaxFromBytes(min, max);
+      }
+      stats.num_nulls = this.numNulls;
+      return stats;
+    }
+  }
+
   private final PrimitiveType type;
   private final PrimitiveComparator<T> comparator;
   private boolean hasNonNullValue;
@@ -109,6 +147,17 @@ public static Statistics<?> createStats(Type type) {
     }
   }
 
+  /**
+   * Returns a builder to create new statistics object. Used to read the statistics form the parquet file.
+   *
+   * @param type
+   *          type of the column
+   * @return builder to create new statistics object
+   */
+  public static StatisticsBuilder getBuilder(PrimitiveType type) {
+    return new StatisticsBuilder(type);
+  }
+
   /**
    * updates statistics min and max using the passed value
    * @param value value to use to update min and max
@@ -217,7 +266,9 @@ public void mergeStatistics(Statistics stats) {
    * Abstract method to set min and max values from byte arrays.
    * @param minBytes byte array to set the min value to
    * @param maxBytes byte array to set the max value to
+   * @deprecated will be removed in 2.0.0. Use {@link #getBuilder(PrimitiveType)} instead.
    */
+  @Deprecated
   abstract public void setMinMaxFromBytes(byte[] minBytes, byte[] maxBytes);
 
   /**
@@ -310,9 +361,13 @@ public String maxAsString() {
 
   @Override
   public String toString() {
-    if (this.hasNonNullValue())
-      return String.format("min: %s, max: %s, num_nulls: %d", minAsString(), maxAsString(), this.getNumNulls());
-    else if (!this.isEmpty())
+    if (this.hasNonNullValue()) {
+      if (isNumNullsSet()) {
+        return String.format("min: %s, max: %s, num_nulls: %d", minAsString(), maxAsString(), this.getNumNulls());
+      } else {
+        return String.format("min: %s, max: %s, num_nulls not defined", minAsString(), maxAsString());
+      }
+    } else if (!this.isEmpty())
       return String.format("num_nulls: %d, min/max not defined", this.getNumNulls());
     else
       return "no stats for this column";
@@ -335,16 +390,20 @@ public void incrementNumNulls(long increment) {
 
   /**
    * Returns the null count
-   * @return null count
+   * @return null count or {@code -1} if the null count is not set
    */
   public long getNumNulls() {
     return num_nulls;
   }
 
   /**
    * Sets the number of nulls to the parameter value
-   * @param nulls null count to set the count to
+   *
+   * @param nulls
+   *          null count to set the count to
+   * @deprecated will be removed in 2.0.0. Use {@link #getBuilder(PrimitiveType)} instead.
    */
+  @Deprecated
   public void setNumNulls(long nulls) {
     num_nulls = nulls;
   }
@@ -355,7 +414,7 @@ public void setNumNulls(long nulls) {
    * @return true if object is empty, false otherwise
    */
   public boolean isEmpty() {
-    return !hasNonNullValue && num_nulls == 0;
+    return !hasNonNullValue && !isNumNullsSet();
   }
 
   /**
@@ -365,6 +424,13 @@ public boolean hasNonNullValue() {
     return hasNonNullValue;
   }
 
+  /**
+   * @return whether numNulls is set and can be used
+   */
+  public boolean isNumNullsSet() {
+    return num_nulls >= 0;
+  }
+
   /**
    * Sets the page/column as having a valid non-null value
    * kind of misnomer here

parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java

@@ -42,6 +42,7 @@ public class TestStatistics {
   @Test
   public void testNumNulls() {
     IntStatistics stats = new IntStatistics();
+    assertTrue(stats.isNumNullsSet());
     assertEquals(stats.getNumNulls(), 0);
 
     stats.incrementNumNulls();

parquet-hadoop/src/main/java/org/apache/parquet/filter2/statisticslevel/StatisticsFilter.java

@@ -121,6 +121,10 @@ public <T extends Comparable<T>> Boolean visit(Eq<T> eq) {
     }
 
     if (value == null) {
+      // We don't know anything about the nulls in this chunk
+      if (!stats.isNumNullsSet()) {
+        return BLOCK_MIGHT_MATCH;
+      }
       // we are looking for records where v eq(null)
       // so drop if there are no nulls in this chunk
       return !hasNulls(meta);
@@ -170,7 +174,7 @@ public <T extends Comparable<T>> Boolean visit(NotEq<T> notEq) {
       return isAllNulls(meta);
     }
 
-    if (hasNulls(meta)) {
+    if (stats.isNumNullsSet() && hasNulls(meta)) {
       // we are looking for records where v notEq(someNonNull)
       // but this chunk contains nulls, we cannot drop it
       return BLOCK_MIGHT_MATCH;

parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java

@@ -64,6 +64,7 @@
 import org.apache.parquet.hadoop.metadata.ColumnChunkMetaData;
 import org.apache.parquet.hadoop.metadata.CompressionCodecName;
 import org.apache.parquet.column.EncodingStats;
+import org.apache.parquet.column.statistics.Statistics.StatisticsBuilder;
 import org.apache.parquet.hadoop.metadata.ParquetMetadata;
 import org.apache.parquet.io.ParquetDecodingException;
 import org.apache.parquet.schema.ColumnOrder.ColumnOrderName;
@@ -401,17 +402,20 @@ public static org.apache.parquet.column.statistics.Statistics fromParquetStatist
   static org.apache.parquet.column.statistics.Statistics fromParquetStatisticsInternal
       (String createdBy, Statistics formatStats, PrimitiveType type, SortOrder typeSortOrder) {
     // create stats object based on the column type
-    org.apache.parquet.column.statistics.Statistics stats = org.apache.parquet.column.statistics.Statistics.createStats(type);
+    StatisticsBuilder builder = org.apache.parquet.column.statistics.Statistics.getBuilder(type);
 
     if (formatStats != null) {
       // Use the new V2 min-max statistics over the former one if it is filled
       if (formatStats.isSetMin_value() && formatStats.isSetMax_value()) {
         byte[] min = formatStats.min_value.array();
         byte[] max = formatStats.max_value.array();
         if (isMinMaxStatsSupported(type) || Arrays.equals(min, max)) {
-          stats.setMinMaxFromBytes(min, max);
+          builder.withMin(min);
+          builder.withMax(max);
+        }
+        if (formatStats.isSetNull_count()) {
+          builder.withNumNulls(formatStats.null_count);
         }
-        stats.setNumNulls(formatStats.null_count);
       } else {
         boolean isSet = formatStats.isSetMax() && formatStats.isSetMin();
         boolean maxEqualsMin = isSet ? Arrays.equals(formatStats.getMin(), formatStats.getMax()) : false;
@@ -424,13 +428,16 @@ public static org.apache.parquet.column.statistics.Statistics fromParquetStatist
         if (!CorruptStatistics.shouldIgnoreStatistics(createdBy, type.getPrimitiveTypeName()) &&
             (sortOrdersMatch || maxEqualsMin)) {
           if (isSet) {
-            stats.setMinMaxFromBytes(formatStats.min.array(), formatStats.max.array());
+            builder.withMin(formatStats.min.array());
+            builder.withMax(formatStats.max.array());
+          }
+          if (formatStats.isSetNull_count()) {
+            builder.withNumNulls(formatStats.null_count);
           }
-          stats.setNumNulls(formatStats.null_count);
         }
       }
     }
-    return stats;
+    return builder.build();
   }
 
   public org.apache.parquet.column.statistics.Statistics fromParquetStatistics(

parquet-hadoop/src/test/java/org/apache/parquet/filter2/statisticslevel/TestStatisticsFilter.java

@@ -38,6 +38,7 @@
 import org.apache.parquet.hadoop.metadata.ColumnChunkMetaData;
 import org.apache.parquet.hadoop.metadata.CompressionCodecName;
 import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
+import org.apache.parquet.schema.Types;
 
 import static org.apache.parquet.filter2.predicate.FilterApi.binaryColumn;
 import static org.apache.parquet.io.api.Binary.fromString;
@@ -61,7 +62,8 @@
 
 public class TestStatisticsFilter {
 
-  private static ColumnChunkMetaData getIntColumnMeta(IntStatistics stats, long valueCount) {
+  private static ColumnChunkMetaData getIntColumnMeta(org.apache.parquet.column.statistics.Statistics<?> stats,
+      long valueCount) {
     return ColumnChunkMetaData.get(ColumnPath.get("int", "column"),
         PrimitiveTypeName.INT32,
         CompressionCodecName.GZIP,
@@ -70,7 +72,8 @@ private static ColumnChunkMetaData getIntColumnMeta(IntStatistics stats, long va
         0L, 0L, valueCount, 0L, 0L);
   }
 
-  private static ColumnChunkMetaData getDoubleColumnMeta(DoubleStatistics stats, long valueCount) {
+  private static ColumnChunkMetaData getDoubleColumnMeta(org.apache.parquet.column.statistics.Statistics<?> stats,
+      long valueCount) {
     return ColumnChunkMetaData.get(ColumnPath.get("double", "column"),
         PrimitiveTypeName.DOUBLE,
         CompressionCodecName.GZIP,
@@ -86,18 +89,17 @@ private static ColumnChunkMetaData getDoubleColumnMeta(DoubleStatistics stats, l
 
   private static final IntStatistics intStats = new IntStatistics();
   private static final IntStatistics nullIntStats = new IntStatistics();
-  private static final IntStatistics missingMinMaxIntStats = new IntStatistics();
+  private static final org.apache.parquet.column.statistics.Statistics<?> missingMinMaxIntStats = org.apache.parquet.column.statistics.Statistics
+      .getBuilder(Types.required(PrimitiveTypeName.INT32).named("test_int32")).build();
   private static final DoubleStatistics doubleStats = new DoubleStatistics();
-  private static final DoubleStatistics missingMinMaxDoubleStats = new DoubleStatistics();
+  private static final org.apache.parquet.column.statistics.Statistics<?> missingMinMaxDoubleStats = org.apache.parquet.column.statistics.Statistics
+      .getBuilder(Types.required(PrimitiveTypeName.DOUBLE).named("test_double")).withNumNulls(100).build();
 
   static {
     intStats.setMinMax(10, 100);
     doubleStats.setMinMax(10, 100);
 
-    nullIntStats.setMinMax(0, 0);
     nullIntStats.setNumNulls(177);
-
-    missingMinMaxDoubleStats.setNumNulls(100);
   }
 
   private static final List<ColumnChunkMetaData> columnMetas = Arrays.asList(

parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java

@@ -658,6 +658,7 @@ private void testUseStatsWithSignedSortOrder(StatsHelper helper) {
         binaryType);
 
     Assert.assertFalse("Stats should not be empty", convertedStats.isEmpty());
+    Assert.assertTrue(convertedStats.isNumNullsSet());
     Assert.assertEquals("Should have 3 nulls", 3, convertedStats.getNumNulls());
     if (helper == StatsHelper.V1) {
       assertFalse("Min-max should be null for V1 stats", convertedStats.hasNonNullValue());
@@ -669,6 +670,38 @@ private void testUseStatsWithSignedSortOrder(StatsHelper helper) {
     }
   }
 
+  @Test
+  public void testMissingValuesFromStats() {
+    ParquetMetadataConverter converter = new ParquetMetadataConverter();
+    PrimitiveType type = Types.required(PrimitiveTypeName.INT32).named("test_int32");
+
+    org.apache.parquet.format.Statistics formatStats = new org.apache.parquet.format.Statistics();
+    Statistics<?> stats = converter.fromParquetStatistics(Version.FULL_VERSION, formatStats, type);
+    assertFalse(stats.isNumNullsSet());
+    assertFalse(stats.hasNonNullValue());
+    assertTrue(stats.isEmpty());
+    assertEquals(-1, stats.getNumNulls());
+
+    formatStats.clear();
+    formatStats.setMin(BytesUtils.intToBytes(-100));
+    formatStats.setMax(BytesUtils.intToBytes(100));
+    stats = converter.fromParquetStatistics(Version.FULL_VERSION, formatStats, type);
+    assertFalse(stats.isNumNullsSet());
+    assertTrue(stats.hasNonNullValue());
+    assertFalse(stats.isEmpty());
+    assertEquals(-1, stats.getNumNulls());
+    assertEquals(-100, stats.genericGetMin());
+    assertEquals(100, stats.genericGetMax());
+
+    formatStats.clear();
+    formatStats.setNull_count(2000);
+    stats = converter.fromParquetStatistics(Version.FULL_VERSION, formatStats, type);
+    assertTrue(stats.isNumNullsSet());
+    assertFalse(stats.hasNonNullValue());
+    assertFalse(stats.isEmpty());
+    assertEquals(2000, stats.getNumNulls());
+  }
+
   @Test
   public void testSkippedV2Stats() {
     testSkippedV2Stats(

parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestColumnChunkPageWriteStore.java

@@ -60,6 +60,7 @@
 import org.apache.parquet.hadoop.metadata.ParquetMetadata;
 import org.apache.parquet.schema.MessageType;
 import org.apache.parquet.schema.MessageTypeParser;
+import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
 import org.apache.parquet.schema.Types;
 import org.apache.parquet.bytes.HeapByteBufferAllocator;
 
@@ -92,7 +93,8 @@ public void test() throws Exception {
     int v = 3;
     BytesInput definitionLevels = BytesInput.fromInt(d);
     BytesInput repetitionLevels = BytesInput.fromInt(r);
-    Statistics<?> statistics = new BinaryStatistics();
+    Statistics<?> statistics = Statistics.getBuilder(Types.required(PrimitiveTypeName.BINARY).named("test_binary"))
+        .build();
     BytesInput data = BytesInput.fromInt(v);
     int rowCount = 5;
     int nullCount = 1;