From 974a22b7982911ab8141b20b3fd12be179aaaeee Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Wed, 7 Jan 2015 18:25:25 +0530 Subject: [PATCH 01/14] PARQUET-136: In case of all nulls in a binary column, statistics object read from file metadata is empty, and should return true for all nulls check for the column --- .../java/parquet/filter2/statisticslevel/StatisticsFilter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java index 4daed5a2e0..54dcc2c80b 100644 --- a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java +++ b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java @@ -67,8 +67,9 @@ private ColumnChunkMetaData getColumnChunk(ColumnPath columnPath) { } // is this column chunk composed entirely of nulls? + // in case of all nulls, the stats object read from file metadata is empty private boolean isAllNulls(ColumnChunkMetaData column) { - return column.getStatistics().getNumNulls() == column.getValueCount(); + return (column.getStatistics().isEmpty()) || (column.getStatistics().getNumNulls() == column.getValueCount()); } // are there any nulls in this column chunk? From c7c126fe0c250706dc3f2bd1580ef2286777e1a9 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Wed, 7 Jan 2015 18:40:21 +0530 Subject: [PATCH 02/14] PARQUET-136: fix hasNulls to cater to the case where all values are nulls --- .../parquet/filter2/statisticslevel/StatisticsFilter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java index 54dcc2c80b..7db5fb49ba 100644 --- a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java +++ b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java @@ -72,9 +72,9 @@ private boolean isAllNulls(ColumnChunkMetaData column) { return (column.getStatistics().isEmpty()) || (column.getStatistics().getNumNulls() == column.getValueCount()); } - // are there any nulls in this column chunk? + // are there any nulls in this column chunk? private boolean hasNulls(ColumnChunkMetaData column) { - return column.getStatistics().getNumNulls() > 0; + return (column.getStatistics().getNumNulls() > 0) || (isAllNulls(column)); } @Override From 8cc51067263e7b50c8906682c122a5fb5ae85168 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Fri, 9 Jan 2015 17:34:33 +0530 Subject: [PATCH 03/14] Revert "PARQUET-136: fix hasNulls to cater to the case where all values are nulls" This reverts commit c7c126fe0c250706dc3f2bd1580ef2286777e1a9. --- .../parquet/filter2/statisticslevel/StatisticsFilter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java index 7db5fb49ba..54dcc2c80b 100644 --- a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java +++ b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java @@ -72,9 +72,9 @@ private boolean isAllNulls(ColumnChunkMetaData column) { return (column.getStatistics().isEmpty()) || (column.getStatistics().getNumNulls() == column.getValueCount()); } - // are there any nulls in this column chunk? + // are there any nulls in this column chunk? private boolean hasNulls(ColumnChunkMetaData column) { - return (column.getStatistics().getNumNulls() > 0) || (isAllNulls(column)); + return column.getStatistics().getNumNulls() > 0; } @Override From e5e924e7bfe15578b58f096900586be0625cf1fc Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Fri, 9 Jan 2015 17:34:59 +0530 Subject: [PATCH 04/14] Revert "PARQUET-136: In case of all nulls in a binary column, statistics object read from file metadata is empty, and should return true for all nulls check for the column" This reverts commit 974a22b7982911ab8141b20b3fd12be179aaaeee. --- .../java/parquet/filter2/statisticslevel/StatisticsFilter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java index 54dcc2c80b..4daed5a2e0 100644 --- a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java +++ b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java @@ -67,9 +67,8 @@ private ColumnChunkMetaData getColumnChunk(ColumnPath columnPath) { } // is this column chunk composed entirely of nulls? - // in case of all nulls, the stats object read from file metadata is empty private boolean isAllNulls(ColumnChunkMetaData column) { - return (column.getStatistics().isEmpty()) || (column.getStatistics().getNumNulls() == column.getValueCount()); + return column.getStatistics().getNumNulls() == column.getValueCount(); } // are there any nulls in this column chunk? From 1ac919227395bf325fa6072afe992f70868061cf Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Fri, 9 Jan 2015 18:20:25 +0530 Subject: [PATCH 05/14] PARQUET-136: Its better to not filter chunks for which empty statistics object is returned. Empty statistics can be read in case of 1. pre-statistics files, 2. files written from current writer that has a bug, as it does not write the statistics if column has all nulls --- .../statisticslevel/StatisticsFilter.java | 81 ++++++++++++++++--- .../statisticslevel/TestStatisticsFilter.java | 8 +- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java index 4daed5a2e0..4242839ab3 100644 --- a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java +++ b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java @@ -81,6 +81,16 @@ public > Boolean visit(Eq eq) { Column filterColumn = eq.getColumn(); T value = eq.getValue(); ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); + Statistics 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 + return false; + } if (value == null) { // we are looking for records where v eq(null) @@ -94,8 +104,6 @@ public > Boolean visit(Eq eq) { return true; } - Statistics stats = columnChunk.getStatistics(); - // drop if value < min || value > max return value.compareTo(stats.genericGetMin()) < 0 || value.compareTo(stats.genericGetMax()) > 0; } @@ -105,6 +113,16 @@ public > Boolean visit(NotEq notEq) { Column filterColumn = notEq.getColumn(); T value = notEq.getValue(); ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); + Statistics 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 + return false; + } if (value == null) { // we are looking for records where v notEq(null) @@ -118,8 +136,6 @@ public > Boolean visit(NotEq notEq) { return false; } - Statistics stats = columnChunk.getStatistics(); - // drop if this is a column where min = max = value return value.compareTo(stats.genericGetMin()) == 0 && value.compareTo(stats.genericGetMax()) == 0; } @@ -129,6 +145,16 @@ public > Boolean visit(Lt lt) { Column filterColumn = lt.getColumn(); T value = lt.getValue(); ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); + Statistics 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 + return false; + } if (isAllNulls(columnChunk)) { // we are looking for records where v < someValue @@ -136,8 +162,6 @@ public > Boolean visit(Lt lt) { return true; } - Statistics stats = columnChunk.getStatistics(); - // drop if value <= min return value.compareTo(stats.genericGetMin()) <= 0; } @@ -147,6 +171,16 @@ public > Boolean visit(LtEq ltEq) { Column filterColumn = ltEq.getColumn(); T value = ltEq.getValue(); ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); + Statistics 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 + return false; + } if (isAllNulls(columnChunk)) { // we are looking for records where v <= someValue @@ -154,8 +188,6 @@ public > Boolean visit(LtEq ltEq) { return true; } - Statistics stats = columnChunk.getStatistics(); - // drop if value < min return value.compareTo(stats.genericGetMin()) < 0; } @@ -165,6 +197,16 @@ public > Boolean visit(Gt gt) { Column filterColumn = gt.getColumn(); T value = gt.getValue(); ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); + Statistics 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 + return false; + } if (isAllNulls(columnChunk)) { // we are looking for records where v > someValue @@ -172,8 +214,6 @@ public > Boolean visit(Gt gt) { return true; } - Statistics stats = columnChunk.getStatistics(); - // drop if value >= max return value.compareTo(stats.genericGetMax()) >= 0; } @@ -183,6 +223,16 @@ public > Boolean visit(GtEq gtEq) { Column filterColumn = gtEq.getColumn(); T value = gtEq.getValue(); ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); + Statistics 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 + return false; + } if (isAllNulls(columnChunk)) { // we are looking for records where v >= someValue @@ -190,8 +240,6 @@ public > Boolean visit(GtEq gtEq) { return true; } - Statistics stats = columnChunk.getStatistics(); - // drop if value >= max return value.compareTo(stats.genericGetMax()) > 0; } @@ -221,6 +269,15 @@ private , U extends UserDefinedPredicate> Boolean vis ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); U udp = ud.getUserDefinedPredicate(); Statistics 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 + return false; + } + parquet.filter2.predicate.Statistics udpStats = new parquet.filter2.predicate.Statistics(stats.genericGetMin(), stats.genericGetMax()); diff --git a/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java b/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java index 4e75b207ce..b7ac931e96 100644 --- a/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java +++ b/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java @@ -279,10 +279,10 @@ public void testUdp() { @Test public void testClearExceptionForNots() { List columnMetas = Arrays.asList( - getIntColumnMeta(new IntStatistics(), 0L), - getDoubleColumnMeta(new DoubleStatistics(), 0L)); + getDoubleColumnMeta(new DoubleStatistics(), 0L), + getIntColumnMeta(new IntStatistics(), 0L)); - FilterPredicate pred = and(eq(intColumn, 17), not(eq(doubleColumn, 12.0))); + FilterPredicate pred = and(not(eq(doubleColumn, 12.0)), eq(intColumn, 17)); try { canDrop(pred, columnMetas); @@ -297,7 +297,7 @@ public void testClearExceptionForNots() { public void testMissingColumn() { List columnMetas = Arrays.asList(getIntColumnMeta(new IntStatistics(), 0L)); try { - canDrop(and(eq(intColumn, 17), eq(doubleColumn, 12.0)), columnMetas); + canDrop(and(eq(doubleColumn, 12.0), eq(intColumn, 17)), columnMetas); fail("This should throw"); } catch (IllegalArgumentException e) { assertEquals("Column double.column not found in schema!", e.getMessage()); From 0c88be0178fbbaf5ce6375f9aedd0fdabf31cf85 Mon Sep 17 00:00:00 2001 From: Alex Levenson Date: Fri, 9 Jan 2015 14:57:03 -0800 Subject: [PATCH 06/14] revert TestStatisticsFilter.java --- .../filter2/statisticslevel/TestStatisticsFilter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java b/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java index b7ac931e96..4e75b207ce 100644 --- a/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java +++ b/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java @@ -279,10 +279,10 @@ public void testUdp() { @Test public void testClearExceptionForNots() { List columnMetas = Arrays.asList( - getDoubleColumnMeta(new DoubleStatistics(), 0L), - getIntColumnMeta(new IntStatistics(), 0L)); + getIntColumnMeta(new IntStatistics(), 0L), + getDoubleColumnMeta(new DoubleStatistics(), 0L)); - FilterPredicate pred = and(not(eq(doubleColumn, 12.0)), eq(intColumn, 17)); + FilterPredicate pred = and(eq(intColumn, 17), not(eq(doubleColumn, 12.0))); try { canDrop(pred, columnMetas); @@ -297,7 +297,7 @@ public void testClearExceptionForNots() { public void testMissingColumn() { List columnMetas = Arrays.asList(getIntColumnMeta(new IntStatistics(), 0L)); try { - canDrop(and(eq(doubleColumn, 12.0), eq(intColumn, 17)), columnMetas); + canDrop(and(eq(intColumn, 17), eq(doubleColumn, 12.0)), columnMetas); fail("This should throw"); } catch (IllegalArgumentException e) { assertEquals("Column double.column not found in schema!", e.getMessage()); From b1001eb265842e18c8ec5761213d1c99114fe582 Mon Sep 17 00:00:00 2001 From: Alex Levenson Date: Fri, 9 Jan 2015 14:59:14 -0800 Subject: [PATCH 07/14] Fix statistics isEmpty, handle more edge cases in statistics filter --- .../parquet/column/statistics/Statistics.java | 20 +++++-- .../statisticslevel/StatisticsFilter.java | 59 ++++++++----------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/parquet-column/src/main/java/parquet/column/statistics/Statistics.java b/parquet-column/src/main/java/parquet/column/statistics/Statistics.java index b29b76b3c3..ff68c523f0 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/Statistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/Statistics.java @@ -28,11 +28,11 @@ */ public abstract class Statistics> { - 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; } } diff --git a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java b/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java index 4242839ab3..2574b1a5f3 100644 --- a/parquet-hadoop/src/main/java/parquet/filter2/statisticslevel/StatisticsFilter.java +++ b/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 > Boolean visit(Eq eq) { Statistics 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 > Boolean visit(NotEq notEq) { Statistics 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 > Boolean visit(NotEq 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 > Boolean visit(Lt lt) { Statistics 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 > Boolean visit(LtEq ltEq) { Statistics 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 > Boolean visit(Gt gt) { Statistics 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 > Boolean visit(GtEq gtEq) { Statistics 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 , U extends UserDefinedPredicate> Boolean vis ColumnChunkMetaData columnChunk = getColumnChunk(filterColumn.getColumnPath()); U udp = ud.getUserDefinedPredicate(); Statistics 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; } From 97bb5175b368e0a2b5a71d12d0a530ece51466ba Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 11 Jan 2015 12:01:53 +0530 Subject: [PATCH 08/14] Revert "revert TestStatisticsFilter.java" This reverts commit 0c88be0178fbbaf5ce6375f9aedd0fdabf31cf85. --- .../filter2/statisticslevel/TestStatisticsFilter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java b/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java index 4e75b207ce..b7ac931e96 100644 --- a/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java +++ b/parquet-hadoop/src/test/java/parquet/filter2/statisticslevel/TestStatisticsFilter.java @@ -279,10 +279,10 @@ public void testUdp() { @Test public void testClearExceptionForNots() { List columnMetas = Arrays.asList( - getIntColumnMeta(new IntStatistics(), 0L), - getDoubleColumnMeta(new DoubleStatistics(), 0L)); + getDoubleColumnMeta(new DoubleStatistics(), 0L), + getIntColumnMeta(new IntStatistics(), 0L)); - FilterPredicate pred = and(eq(intColumn, 17), not(eq(doubleColumn, 12.0))); + FilterPredicate pred = and(not(eq(doubleColumn, 12.0)), eq(intColumn, 17)); try { canDrop(pred, columnMetas); @@ -297,7 +297,7 @@ public void testClearExceptionForNots() { public void testMissingColumn() { List columnMetas = Arrays.asList(getIntColumnMeta(new IntStatistics(), 0L)); try { - canDrop(and(eq(intColumn, 17), eq(doubleColumn, 12.0)), columnMetas); + canDrop(and(eq(doubleColumn, 12.0), eq(intColumn, 17)), columnMetas); fail("This should throw"); } catch (IllegalArgumentException e) { assertEquals("Column double.column not found in schema!", e.getMessage()); From c2f8d6fee55ebabaeed4f5013bfa0e6456dd3e7b Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 11 Jan 2015 12:51:05 +0530 Subject: [PATCH 09/14] PARQUET-161: Fix the write path to write statistics object in case of only nulls in the column --- .../parquet/column/statistics/BinaryStatistics.java | 12 +++++++----- .../parquet/column/statistics/BooleanStatistics.java | 10 ++++++---- .../parquet/column/statistics/DoubleStatistics.java | 10 ++++++---- .../parquet/column/statistics/FloatStatistics.java | 8 +++++--- .../parquet/column/statistics/IntStatistics.java | 8 +++++--- .../parquet/column/statistics/LongStatistics.java | 10 ++++++---- .../java/parquet/column/statistics/Statistics.java | 6 +++++- .../format/converter/ParquetMetadataConverter.java | 10 +++++++--- 8 files changed, 47 insertions(+), 27 deletions(-) diff --git a/parquet-column/src/main/java/parquet/column/statistics/BinaryStatistics.java b/parquet-column/src/main/java/parquet/column/statistics/BinaryStatistics.java index f125b2f0ed..6ef167835f 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/BinaryStatistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/BinaryStatistics.java @@ -24,7 +24,7 @@ public class BinaryStatistics extends Statistics { @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(); } -} \ No newline at end of file +} diff --git a/parquet-column/src/main/java/parquet/column/statistics/BooleanStatistics.java b/parquet-column/src/main/java/parquet/column/statistics/BooleanStatistics.java index 6741343496..8e5b233fde 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/BooleanStatistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/BooleanStatistics.java @@ -24,7 +24,7 @@ public class BooleanStatistics extends Statistics { @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"; } diff --git a/parquet-column/src/main/java/parquet/column/statistics/DoubleStatistics.java b/parquet-column/src/main/java/parquet/column/statistics/DoubleStatistics.java index c9695f355e..ccbf7009e6 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/DoubleStatistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/DoubleStatistics.java @@ -24,7 +24,7 @@ public class DoubleStatistics extends Statistics { @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(); } -} \ No newline at end of file +} diff --git a/parquet-column/src/main/java/parquet/column/statistics/FloatStatistics.java b/parquet-column/src/main/java/parquet/column/statistics/FloatStatistics.java index b13aafa98f..54a550d175 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/FloatStatistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/FloatStatistics.java @@ -24,7 +24,7 @@ public class FloatStatistics extends Statistics { @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"; } diff --git a/parquet-column/src/main/java/parquet/column/statistics/IntStatistics.java b/parquet-column/src/main/java/parquet/column/statistics/IntStatistics.java index 7bdd6be297..065147235f 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/IntStatistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/IntStatistics.java @@ -24,7 +24,7 @@ public class IntStatistics extends Statistics { @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"; } diff --git a/parquet-column/src/main/java/parquet/column/statistics/LongStatistics.java b/parquet-column/src/main/java/parquet/column/statistics/LongStatistics.java index bae63a9973..70407473ca 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/LongStatistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/LongStatistics.java @@ -24,7 +24,7 @@ public class LongStatistics extends Statistics { @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(); } -} \ No newline at end of file +} diff --git a/parquet-column/src/main/java/parquet/column/statistics/Statistics.java b/parquet-column/src/main/java/parquet/column/statistics/Statistics.java index ff68c523f0..7920e3ea24 100644 --- a/parquet-column/src/main/java/parquet/column/statistics/Statistics.java +++ b/parquet-column/src/main/java/parquet/column/statistics/Statistics.java @@ -232,7 +232,11 @@ public boolean isEmpty() { public boolean hasNonNullValue() { return hasNonNullValue; } - + + /** + * Sets the page/column as having a valid non-null value + * kind of misnomer here + */ protected void markAsNotEmpty() { hasNonNullValue = true; } diff --git a/parquet-hadoop/src/main/java/parquet/format/converter/ParquetMetadataConverter.java b/parquet-hadoop/src/main/java/parquet/format/converter/ParquetMetadataConverter.java index 198b6548ba..b43429bb10 100644 --- a/parquet-hadoop/src/main/java/parquet/format/converter/ParquetMetadataConverter.java +++ b/parquet-hadoop/src/main/java/parquet/format/converter/ParquetMetadataConverter.java @@ -234,9 +234,11 @@ public Encoding getEncoding(parquet.column.Encoding encoding) { public static Statistics toParquetStatistics(parquet.column.statistics.Statistics statistics) { Statistics stats = new Statistics(); if (!statistics.isEmpty()) { - stats.setMax(statistics.getMaxBytes()); - stats.setMin(statistics.getMinBytes()); stats.setNull_count(statistics.getNumNulls()); + if(statistics.hasNonNullValue()) { + stats.setMax(statistics.getMaxBytes()); + stats.setMin(statistics.getMinBytes()); + } } return stats; } @@ -246,7 +248,9 @@ public static parquet.column.statistics.Statistics fromParquetStatistics(Statist parquet.column.statistics.Statistics stats = parquet.column.statistics.Statistics.getStatsBasedOnType(type); // If there was no statistics written to the footer, create an empty Statistics object and return if (statistics != null) { - stats.setMinMaxFromBytes(statistics.min.array(), statistics.max.array()); + if (statistics.isSetMax() && statistics.isSetMin()) { + stats.setMinMaxFromBytes(statistics.min.array(), statistics.max.array()); + } stats.setNumNulls(statistics.null_count); } return stats; From 2217ee24adb9891d7e5b7e7c172065d7357fb9f0 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 11 Jan 2015 20:42:42 +0530 Subject: [PATCH 10/14] PARQUET-161: Add a test case for checking the correct statistics info is recorded in case of all nulls in a column --- .../parquet/hadoop/TestParquetFileWriter.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/parquet-hadoop/src/test/java/parquet/hadoop/TestParquetFileWriter.java b/parquet-hadoop/src/test/java/parquet/hadoop/TestParquetFileWriter.java index 1d45469cc8..9fa673bef7 100644 --- a/parquet-hadoop/src/test/java/parquet/hadoop/TestParquetFileWriter.java +++ b/parquet-hadoop/src/test/java/parquet/hadoop/TestParquetFileWriter.java @@ -62,8 +62,14 @@ import parquet.format.Statistics; import parquet.format.converter.ParquetMetadataConverter; +import parquet.example.data.Group; +import parquet.example.data.simple.SimpleGroup; + +import parquet.hadoop.example.GroupWriteSupport; + public class TestParquetFileWriter { private static final Log LOG = Log.getLog(TestParquetFileWriter.class); + private String writeSchema; @Test public void testWriteRead() throws Exception { @@ -310,6 +316,39 @@ public boolean accept(Path p) { } + @Test + public void testWriteReadStatisticsAllNulls() throws Exception { + + File testFile = new File("target/test/TestParquetFileWriter/testParquetFile").getAbsoluteFile(); + testFile.delete(); + + writeSchema = "message example {\n" + + "required binary content;\n" + + "}"; + + Path path = new Path(testFile.toURI()); + + MessageType schema = MessageTypeParser.parseMessageType(writeSchema); + Configuration configuration = new Configuration(); + GroupWriteSupport.setSchema(schema, configuration); + + ParquetWriter writer = new ParquetWriter(path, configuration, new GroupWriteSupport()); + + Group r1 = new SimpleGroup(schema); + writer.write(r1); + writer.close(); + + ParquetMetadata readFooter = ParquetFileReader.readFooter(configuration, path); + for (BlockMetaData block : readFooter.getBlocks()) { + for (ColumnChunkMetaData col : block.getColumns()) { + col.getPath(); + } + } + { // assert the number of nulls are correct for the first block + assertEquals(1, (readFooter.getBlocks().get(0).getColumns().get(0).getStatistics().getNumNulls())); + } + } + private void validateFooters(final List