From 3d7e35a0b03cdd127d54eaea50bc9b278a158e3f Mon Sep 17 00:00:00 2001
From: julien <julien@twitter.com>
Date: Wed, 3 Sep 2014 18:16:09 -0700
Subject: [PATCH] adress review feedback

---
 .../converter/ParquetMetadataConverter.java      |  2 +-
 .../java/parquet/hadoop/ParquetFileReader.java   | 16 ++++++++--------
 .../java/parquet/hadoop/ParquetRecordReader.java |  6 ++++++
 pom.xml                                          |  2 +-
 4 files changed, 16 insertions(+), 10 deletions(-)

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 519744d25e..76834d51c5 100644
--- a/parquet-hadoop/src/main/java/parquet/format/converter/ParquetMetadataConverter.java
+++ b/parquet-hadoop/src/main/java/parquet/format/converter/ParquetMetadataConverter.java
@@ -385,7 +385,7 @@ public String toString() {
     }
   }
   /**
-   * [ startOffset, endOffset (
+   * [ startOffset, endOffset )
    * @author Julien Le Dem
    */
   static final class RangeMetadataFilter extends MetadataFilter {
diff --git a/parquet-hadoop/src/main/java/parquet/hadoop/ParquetFileReader.java b/parquet-hadoop/src/main/java/parquet/hadoop/ParquetFileReader.java
index 3f9ee8083f..49f1fabb78 100644
--- a/parquet-hadoop/src/main/java/parquet/hadoop/ParquetFileReader.java
+++ b/parquet-hadoop/src/main/java/parquet/hadoop/ParquetFileReader.java
@@ -317,16 +317,16 @@ public static List<Footer> readSummaryFile(Configuration configuration, FileStat
   }
 
   static ParquetMetadata readSummaryMetadata(Configuration configuration, Path basePath, boolean skipRowGroups) throws IOException {
-    Path summaryFile = new Path(basePath, PARQUET_METADATA_FILE);
-    Path summaryFileLight = new Path(basePath, PARQUET_COMMON_METADATA_FILE);
+    Path metadataFile = new Path(basePath, PARQUET_METADATA_FILE);
+    Path commonMetaDataFile = new Path(basePath, PARQUET_COMMON_METADATA_FILE);
     FileSystem fileSystem = basePath.getFileSystem(configuration);
-    if (skipRowGroups && fileSystem.exists(summaryFileLight)) {
+    if (skipRowGroups && fileSystem.exists(commonMetaDataFile)) {
       // reading the summary file that does not contain the row groups
-      if (Log.INFO) LOG.info("reading summary file: " + summaryFileLight);
-      return readFooter(configuration, summaryFileLight, filter(skipRowGroups));
-    } else if (fileSystem.exists(summaryFile)) {
-      if (Log.INFO) LOG.info("reading summary file: " + summaryFile);
-      return readFooter(configuration, summaryFile, filter(skipRowGroups));
+      if (Log.INFO) LOG.info("reading summary file: " + commonMetaDataFile);
+      return readFooter(configuration, commonMetaDataFile, filter(skipRowGroups));
+    } else if (fileSystem.exists(metadataFile)) {
+      if (Log.INFO) LOG.info("reading summary file: " + metadataFile);
+      return readFooter(configuration, metadataFile, filter(skipRowGroups));
     } else {
       return null;
     }
diff --git a/parquet-hadoop/src/main/java/parquet/hadoop/ParquetRecordReader.java b/parquet-hadoop/src/main/java/parquet/hadoop/ParquetRecordReader.java
index 731dc6ae35..9220c3cc27 100644
--- a/parquet-hadoop/src/main/java/parquet/hadoop/ParquetRecordReader.java
+++ b/parquet-hadoop/src/main/java/parquet/hadoop/ParquetRecordReader.java
@@ -144,10 +144,13 @@ private void initializeInternalReader(ParquetInputSplit split, Configuration con
         configuration, path, range(split.getStart(), split.getEnd()));
     long[] rowGroupOffsets = split.getRowGroupOffsets();
     List<BlockMetaData> filteredBlocks;
+    // if task.side.metadata is set, rowGroupOffsets is null
     if (rowGroupOffsets == null) {
+      // then we need to apply the predicate push down filter
       Filter filter = ParquetInputFormat.getFilter(configuration);
       filteredBlocks = RowGroupFilter.filterRowGroups(filter, footer.getBlocks(), footer.getFileMetaData().getSchema());
     } else {
+      // otherwise we find the row groups that were selected on the client
       Set<Long> offsets = new HashSet<Long>();
       for (long offset : rowGroupOffsets) {
         offsets.add(offset);
@@ -158,11 +161,14 @@ private void initializeInternalReader(ParquetInputSplit split, Configuration con
           filteredBlocks.add(block);
         }
       }
+      // verify we found them all
       if (filteredBlocks.size() != rowGroupOffsets.length) {
         long[] foundRowGroupOffsets = new long[footer.getBlocks().size()];
         for (int i = 0; i < foundRowGroupOffsets.length; i++) {
           foundRowGroupOffsets[i] = footer.getBlocks().get(i).getStartingPos();
         }
+        // this should never happen.
+        // provide a good error message in case there's a bug
         throw new IllegalStateException(
             "All the offsets listed in the split should be found in the file."
             + " expected: " + Arrays.toString(rowGroupOffsets)
diff --git a/pom.xml b/pom.xml
index 0117aa5b31..65969e7438 100644
--- a/pom.xml
+++ b/pom.xml
@@ -85,7 +85,7 @@
     <shade.prefix>parquet</shade.prefix>
     <hadoop.version>1.1.0</hadoop.version>
     <cascading.version>2.5.3</cascading.version>
-    <parquet.format.version>2.1.1-SNAPSHOT</parquet.format.version>
+    <parquet.format.version>2.2.0-rc1</parquet.format.version>
     <log4j.version>1.2.17</log4j.version>
     <previous.version>1.6.0rc1</previous.version>
     <thrift.executable>thrift</thrift.executable>