Fix statistics isEmpty, handle more edge cases in statistics filter

Author: isnotinvain

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

@@ -28,11 +28,11 @@
  */
 public abstract class Statistics<T extends Comparable<T>> {
 
-  private boolean firstValueAccountedFor;
+  private boolean hasNonNullValue;
   private long num_nulls;
 
   public Statistics() {
-    firstValueAccountedFor = false;
+    hasNonNullValue = false;
     num_nulls = 0;
   }
 
@@ -142,7 +142,10 @@ public void mergeStatistics(Statistics stats) {
 
     if (this.getClass() == stats.getClass()) {
       incrementNumNulls(stats.getNumNulls());
-      mergeStatisticsMinMax(stats);
+      if (stats.hasNonNullValue()) {
+        mergeStatisticsMinMax(stats);
+        markAsNotEmpty();
+      }
     } else {
       throw new StatisticsClassException(this.getClass().toString(), stats.getClass().toString());
     }
@@ -220,11 +223,18 @@ public void setNumNulls(long nulls) {
    * @return true if object is empty, false otherwise
    */
   public boolean isEmpty() {
-    return !firstValueAccountedFor;
+    return !hasNonNullValue && num_nulls == 0;
   }
 
+  /**
+   * Returns whether there have been non-null values added to this statistics
+   */
+  public boolean hasNonNullValue() {
+    return hasNonNullValue;
+  }
+  
   protected void markAsNotEmpty() {
-    firstValueAccountedFor = true;
+    hasNonNullValue = true;
   }
 }
 

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

@@ -67,11 +67,13 @@ private ColumnChunkMetaData getColumnChunk(ColumnPath columnPath) {
   }
 
   // is this column chunk composed entirely of nulls?
+  // assumes the column chunk's statistics is not empty
   private boolean isAllNulls(ColumnChunkMetaData column) {
     return column.getStatistics().getNumNulls() == column.getValueCount();
   }
 
   // are there any nulls in this column chunk?
+  // assumes the column chunk's statistics is not empty
   private boolean hasNulls(ColumnChunkMetaData column) {
     return column.getStatistics().getNumNulls() > 0;
   }
@@ -84,11 +86,7 @@ public <T extends Comparable<T>> Boolean visit(Eq<T> eq) {
     Statistics<T> stats = columnChunk.getStatistics();
 
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
       return false;
     }
 
@@ -116,11 +114,7 @@ public <T extends Comparable<T>> Boolean visit(NotEq<T> notEq) {
     Statistics<T> stats = columnChunk.getStatistics();
 
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
       return false;
     }
 
@@ -136,6 +130,13 @@ public <T extends Comparable<T>> Boolean visit(NotEq<T> notEq) {
       return false;
     }
 
+    if (isAllNulls(columnChunk)) {
+      // there is no min max, there is nothing
+      // else we can say about this chunk, we
+      // cannot drop it.
+      return false;
+    }
+
     // drop if this is a column where min = max = value
     return value.compareTo(stats.genericGetMin()) == 0 && value.compareTo(stats.genericGetMax()) == 0;
   }
@@ -148,11 +149,7 @@ public <T extends Comparable<T>> Boolean visit(Lt<T> lt) {
     Statistics<T> stats = columnChunk.getStatistics();
 
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
       return false;
     }
 
@@ -174,11 +171,7 @@ public <T extends Comparable<T>> Boolean visit(LtEq<T> ltEq) {
     Statistics<T> stats = columnChunk.getStatistics();
 
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
       return false;
     }
 
@@ -200,11 +193,7 @@ public <T extends Comparable<T>> Boolean visit(Gt<T> gt) {
     Statistics<T> stats = columnChunk.getStatistics();
 
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
       return false;
     }
 
@@ -226,11 +215,7 @@ public <T extends Comparable<T>> Boolean visit(GtEq<T> gtEq) {
     Statistics<T> stats = columnChunk.getStatistics();
 
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
       return false;
     }
 
@@ -269,12 +254,16 @@ private <T extends Comparable<T>, U extends UserDefinedPredicate<T>> Boolean vis
     ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath());
     U udp = ud.getUserDefinedPredicate();
     Statistics<T> stats = columnChunk.getStatistics();
+
     if (stats.isEmpty()) {
-     // Do not filter in case of empty statistics
-     // pre-statistics files will have no such info
-     // best not to filter these.
-     // Also, in case of all nulls in a column, empty statistics object 
-     // is returned. This fixes the read path because of that bug
+      // we have no statistics available, we cannot drop any chunks
+      return false;
+    }
+
+    if (isAllNulls(columnChunk)) {
+      // there is no min max, there is nothing
+      // else we can say about this chunk, we
+      // cannot drop it.
       return false;
     }