PARQUET-136: NPE thrown in StatisticsFilter when all values in a string/binary column trunk are null

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

@@ -24,7 +24,7 @@ public class BinaryStatistics extends Statistics<Binary> {
 
   @Override
   public void updateStats(Binary value) {
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(value, value);
     } else {
       updateStats(value, value);
@@ -34,7 +34,7 @@ public void updateStats(Binary value) {
   @Override
   public void mergeStatisticsMinMax(Statistics stats) {
     BinaryStatistics binaryStats = (BinaryStatistics)stats;
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(binaryStats.getMin(), binaryStats.getMax());
     } else {
       updateStats(binaryStats.getMin(), binaryStats.getMax());
@@ -60,9 +60,11 @@ public byte[] getMinBytes() {
 
   @Override
   public String toString() {
-    if(!this.isEmpty())
+    if (this.hasNonNullValue())
       return String.format("min: %s, max: %s, num_nulls: %d", min.toStringUsingUTF8(), max.toStringUsingUTF8(), this.getNumNulls());
-    else
+   else if (!this.isEmpty())
+      return String.format("num_nulls: %d, min/max not defined", this.getNumNulls());
+   else
       return "no stats for this column";
   }
 
@@ -100,4 +102,4 @@ public void setMinMax(Binary min, Binary max) {
     this.min = min;
     this.markAsNotEmpty();
   }
-}
+}

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

@@ -24,7 +24,7 @@ public class BooleanStatistics extends Statistics<Boolean> {
 
   @Override
   public void updateStats(boolean value) {
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(value, value);
     } else {
       updateStats(value, value);
@@ -34,7 +34,7 @@ public void updateStats(boolean value) {
   @Override
   public void mergeStatisticsMinMax(Statistics stats) {
     BooleanStatistics boolStats = (BooleanStatistics)stats;
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(boolStats.getMin(), boolStats.getMax());
     } else {
       updateStats(boolStats.getMin(), boolStats.getMax());
@@ -60,9 +60,11 @@ public byte[] getMinBytes() {
 
   @Override
   public String toString() {
-    if(!this.isEmpty())
+    if (this.hasNonNullValue())
       return String.format("min: %b, max: %b, num_nulls: %d", min, max, this.getNumNulls());
-    else
+    else if(!this.isEmpty())
+      return String.format("num_nulls: %d, min/max not defined", this.getNumNulls());
+    else  
       return "no stats for this column";
   }
 

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

@@ -24,7 +24,7 @@ public class DoubleStatistics extends Statistics<Double> {
 
   @Override
   public void updateStats(double value) {
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(value, value);
     } else {
       updateStats(value, value);
@@ -34,7 +34,7 @@ public void updateStats(double value) {
   @Override
   public void mergeStatisticsMinMax(Statistics stats) {
     DoubleStatistics doubleStats = (DoubleStatistics)stats;
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(doubleStats.getMin(), doubleStats.getMax());
     } else {
       updateStats(doubleStats.getMin(), doubleStats.getMax());
@@ -60,8 +60,10 @@ public byte[] getMinBytes() {
 
   @Override
   public String toString() {
-    if(!this.isEmpty())
+    if(this.hasNonNullValue())
       return String.format("min: %.5f, max: %.5f, num_nulls: %d", min, max, this.getNumNulls());
+    else if (!this.isEmpty())
+      return String.format("num_nulls: %d, min/max not defined", this.getNumNulls());
     else
       return "no stats for this column";
   }
@@ -100,4 +102,4 @@ public void setMinMax(double min, double max) {
     this.min = min;
     this.markAsNotEmpty();
   }
-}
+}

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

@@ -24,7 +24,7 @@ public class FloatStatistics extends Statistics<Float> {
 
   @Override
   public void updateStats(float value) {
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(value, value);
     } else {
       updateStats(value, value);
@@ -34,7 +34,7 @@ public void updateStats(float value) {
   @Override
   public void mergeStatisticsMinMax(Statistics stats) {
     FloatStatistics floatStats = (FloatStatistics)stats;
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(floatStats.getMin(), floatStats.getMax());
     } else {
       updateStats(floatStats.getMin(), floatStats.getMax());
@@ -60,8 +60,10 @@ public byte[] getMinBytes() {
 
   @Override
   public String toString() {
-    if(!this.isEmpty())
+    if (this.hasNonNullValue())
       return String.format("min: %.5f, max: %.5f, num_nulls: %d", min, max, this.getNumNulls());
+    else if (!this.isEmpty())
+      return String.format("num_nulls: %d, min/max not defined", this.getNumNulls());
     else
       return "no stats for this column";
   }

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

@@ -24,7 +24,7 @@ public class IntStatistics extends Statistics<Integer> {
 
   @Override
   public void updateStats(int value) {
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(value, value);
     } else {
       updateStats(value, value);
@@ -34,7 +34,7 @@ public void updateStats(int value) {
   @Override
   public void mergeStatisticsMinMax(Statistics stats) {
     IntStatistics intStats = (IntStatistics)stats;
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(intStats.getMin(), intStats.getMax());
     } else {
       updateStats(intStats.getMin(), intStats.getMax());
@@ -60,8 +60,10 @@ public byte[] getMinBytes() {
 
   @Override
   public String toString() {
-    if(!this.isEmpty())
+    if (this.hasNonNullValue())
       return String.format("min: %d, max: %d, num_nulls: %d", min, max, this.getNumNulls());
+    else if (!this.isEmpty())
+      return String.format("num_nulls: %d, min/max is not defined", this.getNumNulls());
     else
       return "no stats for this column";
   }

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

@@ -24,7 +24,7 @@ public class LongStatistics extends Statistics<Long> {
 
   @Override
   public void updateStats(long value) {
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(value, value);
     } else {
       updateStats(value, value);
@@ -34,7 +34,7 @@ public void updateStats(long value) {
   @Override
   public void mergeStatisticsMinMax(Statistics stats) {
     LongStatistics longStats = (LongStatistics)stats;
-    if (this.isEmpty()) {
+    if (!this.hasNonNullValue()) {
       initializeStats(longStats.getMin(), longStats.getMax());
     } else {
       updateStats(longStats.getMin(), longStats.getMax());
@@ -60,8 +60,10 @@ public byte[] getMinBytes() {
 
   @Override
   public String toString() {
-    if(!this.isEmpty())
+    if (this.hasNonNullValue())
       return String.format("min: %d, max: %d, num_nulls: %d", min, max, this.getNumNulls());
+    else if (!this.isEmpty())
+      return String.format("num_nulls: %d, min/max not defined", this.getNumNulls());
     else
       return "no stats for this column";
   }
@@ -100,4 +102,4 @@ public void setMinMax(long min, long max) {
     this.min = min;
     this.markAsNotEmpty();
   }
-}
+}

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,22 @@ 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;
+  }
+
+  /**
+   * Sets the page/column as having a valid non-null value
+   * kind of misnomer here
+   */ 
   protected void markAsNotEmpty() {
-    firstValueAccountedFor = true;
+    hasNonNullValue = true;
   }
 }