From 5d4fde7b1dea4948404e29409bfd864ff9f4999d Mon Sep 17 00:00:00 2001
From: Fokko Driesprong <fokko@apache.org>
Date: Tue, 15 Oct 2019 10:44:14 +0200
Subject: [PATCH 1/3] PARQUET-1496: Update Scala to 2.12

This also includes updating Scrooge because it isn't compiled
anymore against Scala 2.10
---
 parquet-scrooge/pom.xml                              |  6 +++---
 .../apache/parquet/scrooge/ScroogeBinaryTest.java    |  4 ++--
 .../apache/parquet/thrift/ThriftParquetReader.java   | 12 ++++++------
 pom.xml                                              |  5 +++--
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/parquet-scrooge/pom.xml b/parquet-scrooge/pom.xml
index 6da3c0a3e9..e3c6af0d42 100644
--- a/parquet-scrooge/pom.xml
+++ b/parquet-scrooge/pom.xml
@@ -38,7 +38,7 @@
       <url>https://conjars.org/repo</url>
     </repository>
   </repositories>
-  
+
   <dependencies>
     <dependency>
       <groupId>org.apache.parquet</groupId>
@@ -88,7 +88,7 @@
     <dependency>
       <groupId>com.twitter</groupId>
       <artifactId>scrooge-core_${scala.binary.version}</artifactId>
-      <version>4.7.0</version>
+      <version>${scrooge.verion}</version>
     </dependency>
     <dependency>
       <groupId>org.apache.parquet</groupId>
@@ -193,7 +193,7 @@
         <plugin>
             <groupId>com.twitter</groupId>
             <artifactId>scrooge-maven-plugin</artifactId>
-            <version>3.17.0</version>
+            <version>${scrooge.verion}</version>
             <configuration>
                 <outputDirectory>${project.build.directory}/generated-test-sources/scrooge</outputDirectory>
                 <thriftNamespaceMappings>
diff --git a/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java b/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
index bf0d615111..fa016b20ab 100644
--- a/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
+++ b/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
@@ -61,9 +61,9 @@ public void testScroogeBinaryEncoding() throws Exception {
     reader.close();
 
     Assert.assertEquals("String should match after serialization round trip",
-        "test", record.s);
+        "test", record.s());
     Assert.assertEquals("ByteBuffer should match after serialization round trip",
-        ByteBuffer.wrap(new byte[] {-123, 20, 33}), record.b);
+        ByteBuffer.wrap(new byte[] {-123, 20, 33}), record.b());
   }
 
   @Test
diff --git a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
index 602d757e55..5903b0f082 100644
--- a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
+++ b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
@@ -1,4 +1,4 @@
-/* 
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -36,7 +36,7 @@
  * To read a parquet file into thrift objects
  * @param <T> the thrift type
  */
-public class ThriftParquetReader<T extends TBase<?,?>> extends ParquetReader<T> {
+public class ThriftParquetReader<T> extends ParquetReader<T> {
 
   /**
    * @param file the file to read
@@ -84,11 +84,11 @@ public ThriftParquetReader(Configuration conf, Path file) throws IOException {
     super(conf, file, new ThriftReadSupport<T>());
   }
 
-  public static <T extends TBase<?,?>> Builder<T> build(Path file) {
+  public static <T> Builder<T> build(Path file) {
     return new Builder<T>(file);
   }
 
-  public static class Builder<T extends TBase<?,?>> {
+  public static class Builder<T> {
     private final Path file;
     private Configuration conf;
     private Filter filter;
diff --git a/pom.xml b/pom.xml
index daea6197e6..b12dad03b3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -86,10 +86,11 @@
     <previous.version>1.7.0</previous.version>
     <thrift.executable>thrift</thrift.executable>
     <format.thrift.executable>thrift</format.thrift.executable>
-    <scala.version>2.10.6</scala.version>
+    <scala.version>2.12.8</scala.version>
     <!-- scala.binary.version is used for projects that fetch dependencies that are in scala -->
-    <scala.binary.version>2.10</scala.binary.version>
+    <scala.binary.version>2.12</scala.binary.version>
     <scala.maven.test.skip>false</scala.maven.test.skip>
+    <scrooge.verion>19.10.0</scrooge.verion>
     <pig.version>0.16.0</pig.version>
     <pig.classifier>h2</pig.classifier>
     <thrift-maven-plugin.version>0.10.0</thrift-maven-plugin.version>

From e4e5537ae0dbf068658796cf6827aede46a70560 Mon Sep 17 00:00:00 2001
From: Fokko Driesprong <fokko@apache.org>
Date: Fri, 18 Oct 2019 14:45:26 +0200
Subject: [PATCH 2/3] Fix the tests

---
 parquet-scrooge/pom.xml                       | 16 ++--
 .../parquet/scrooge/ScroogeBinaryTest.java    | 78 ++++++++++---------
 parquet-scrooge/src/test/thrift/test.thrift   | 14 ++--
 .../parquet/thrift/ThriftParquetReader.java   |  6 +-
 .../parquet/hadoop/thrift/TestBinary.java     | 34 ++++----
 parquet-thrift/src/test/thrift/test.thrift    |  2 +-
 6 files changed, 75 insertions(+), 75 deletions(-)

diff --git a/parquet-scrooge/pom.xml b/parquet-scrooge/pom.xml
index e3c6af0d42..304088854d 100644
--- a/parquet-scrooge/pom.xml
+++ b/parquet-scrooge/pom.xml
@@ -202,19 +202,17 @@
                         <to>org.apache.parquet.scrooge.test</to>
                     </thriftNamespaceMapping>
                     <thriftNamespaceMapping>
-                        <from>org.apache.parquet.thrift.test.compat</from>
-                        <to>org.apache.parquet.scrooge.test.compat</to>
+                      <from>org.apache.parquet.thrift.test.compat</from>
+                      <to>org.apache.parquet.scrooge.test.compat</to>
+                    </thriftNamespaceMapping>
+                    <thriftNamespaceMapping>
+                      <from>org.apache.parquet.thrift.test.binary</from>
+                      <to>org.apache.parquet.scrooge.test.binary</to>
                     </thriftNamespaceMapping>
                 </thriftNamespaceMappings>
+                <buildExtractedThrift>false</buildExtractedThrift>
             </configuration>
             <executions>
-                <execution>
-                    <id>thrift-sources</id>
-                    <phase>generate-sources</phase>
-                    <goals>
-                        <goal>compile</goal>
-                    </goals>
-                </execution>
                 <execution>
                     <id>thrift-test-sources</id>
                     <phase>generate-test-sources</phase>
diff --git a/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java b/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
index fa016b20ab..a6f4e326fa 100644
--- a/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
+++ b/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
@@ -1,20 +1,20 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
  */
 package org.apache.parquet.scrooge;
 
@@ -47,30 +47,31 @@ public void testScroogeBinaryEncoding() throws Exception {
 
     Path path = new Path(temp.getPath());
 
-    ParquetWriter<StringAndBinary> writer = new ParquetWriter<StringAndBinary>(
-        path, new Configuration(), new ScroogeWriteSupport<StringAndBinary>(StringAndBinary.class));
-    writer.write(expected);
-    writer.close();
+    try(ParquetWriter<StringAndBinary> writer = new ParquetWriter<StringAndBinary>(
+        path, new Configuration(), new ScroogeWriteSupport<StringAndBinary>(StringAndBinary.class))) {
+      writer.write(expected);
+    }
 
     // read using the parquet-thrift version to isolate the write path
-    ParquetReader<org.apache.parquet.thrift.test.binary.StringAndBinary> reader = ThriftParquetReader.<org.apache.parquet.thrift.test.binary.StringAndBinary>
+    final org.apache.parquet.thrift.test.binary.StringAndBinary record;
+    try (ParquetReader<org.apache.parquet.thrift.test.binary.StringAndBinary> reader = ThriftParquetReader.<org.apache.parquet.thrift.test.binary.StringAndBinary>
         build(path)
         .withThriftClass(org.apache.parquet.thrift.test.binary.StringAndBinary.class)
-        .build();
-    org.apache.parquet.thrift.test.binary.StringAndBinary record = reader.read();
-    reader.close();
+        .build()) {
+      record = reader.read();
+    }
 
     Assert.assertEquals("String should match after serialization round trip",
-        "test", record.s());
+        "test", record.s);
     Assert.assertEquals("ByteBuffer should match after serialization round trip",
-        ByteBuffer.wrap(new byte[] {-123, 20, 33}), record.b());
+        ByteBuffer.wrap(new byte[] {-123, 20, 33}), record.b);
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void testScroogeBinaryDecoding() throws Exception {
     StringAndBinary expected = new StringAndBinary.Immutable("test",
-        ByteBuffer.wrap(new byte[] {-123, 20, 33}));
+      ByteBuffer.wrap(new byte[]{-123, 20, 33}));
 
     File temp = tempDir.newFile(UUID.randomUUID().toString());
     temp.deleteOnExit();
@@ -78,19 +79,20 @@ public void testScroogeBinaryDecoding() throws Exception {
 
     Path path = new Path(temp.getPath());
 
-    ParquetWriter<StringAndBinary> writer = new ParquetWriter<StringAndBinary>(
-        path, new Configuration(), new ScroogeWriteSupport<StringAndBinary>(StringAndBinary.class));
-    writer.write(expected);
-    writer.close();
+    try (ParquetWriter<StringAndBinary> writer = new ParquetWriter<>(
+      path, new Configuration(), new ScroogeWriteSupport<>(StringAndBinary.class))) {
+      writer.write(expected);
+    }
 
     Configuration conf = new Configuration();
     conf.set("parquet.thrift.converter.class", ScroogeRecordConverter.class.getName());
-    ParquetReader<StringAndBinary> reader = ParquetReader.<StringAndBinary>
-        builder(new ScroogeReadSupport(), path)
-        .withConf(conf)
-        .build();
-    StringAndBinary record = reader.read();
-    reader.close();
+    StringAndBinary record;
+    try(ParquetReader<StringAndBinary> reader = ParquetReader.<StringAndBinary>
+      builder(new ScroogeReadSupport(), path)
+      .withConf(conf)
+      .build()) {
+      record = reader.read();
+    }
 
     Assert.assertEquals("String should match after serialization round trip",
         "test", record.s());
diff --git a/parquet-scrooge/src/test/thrift/test.thrift b/parquet-scrooge/src/test/thrift/test.thrift
index 6db7dc19ef..a80dbd924a 100644
--- a/parquet-scrooge/src/test/thrift/test.thrift
+++ b/parquet-scrooge/src/test/thrift/test.thrift
@@ -1,4 +1,4 @@
-/* 
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -28,7 +28,7 @@ struct Name {
 }
 
 struct Address {
-  1: required string street,
+  1: string street,
   2: required string zip
 }
 
@@ -38,7 +38,7 @@ struct AddressWithStreetWithDefaultRequirement {
 }
 
 struct Phone {
-  1: required string mobile
+  1: string mobile
   2: required string work
 }
 
@@ -284,7 +284,7 @@ union UnionV2 {
   3: ABool aNewBool
 }
 
-struct StructWithUnionV2 {  
+struct StructWithUnionV2 {
   1: required string name,
   2: required UnionV2 aUnion
 }
@@ -295,7 +295,7 @@ struct AStructThatLooksLikeUnionV2 {
   3: optional ABool aNewBool
 }
 
-struct StructWithAStructThatLooksLikeUnionV2 {  
+struct StructWithAStructThatLooksLikeUnionV2 {
   1: required string name,
   2: required AStructThatLooksLikeUnionV2 aNotQuiteUnion
 }
diff --git a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
index 5903b0f082..933f0601f3 100644
--- a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
+++ b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
@@ -36,7 +36,7 @@
  * To read a parquet file into thrift objects
  * @param <T> the thrift type
  */
-public class ThriftParquetReader<T> extends ParquetReader<T> {
+public class ThriftParquetReader<T extends TBase<?, ?>> extends ParquetReader<T> {
 
   /**
    * @param file the file to read
@@ -84,11 +84,11 @@ public ThriftParquetReader(Configuration conf, Path file) throws IOException {
     super(conf, file, new ThriftReadSupport<T>());
   }
 
-  public static <T> Builder<T> build(Path file) {
+  public static <T extends TBase<?, ?>> Builder<T> build(Path file) {
     return new Builder<T>(file);
   }
 
-  public static class Builder<T> {
+  public static class Builder<T extends TBase<?, ?>> {
     private final Path file;
     private Configuration conf;
     private Filter filter;
diff --git a/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java b/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java
index ac5a08b979..d38c049f59 100644
--- a/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java
+++ b/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java
@@ -1,20 +1,20 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
  */
 package org.apache.parquet.hadoop.thrift;
 
diff --git a/parquet-thrift/src/test/thrift/test.thrift b/parquet-thrift/src/test/thrift/test.thrift
index d25e540b67..e759144173 100644
--- a/parquet-thrift/src/test/thrift/test.thrift
+++ b/parquet-thrift/src/test/thrift/test.thrift
@@ -35,7 +35,7 @@ struct Address {
 
 struct Phone {
   1: string mobile
-  2: string work
+  2: required string work
 }
 
 struct TestPerson {

From 2d81079ca34788edc913186c2106ea1fb0d723c9 Mon Sep 17 00:00:00 2001
From: Fokko Driesprong <fokko@apache.org>
Date: Fri, 18 Oct 2019 14:56:27 +0200
Subject: [PATCH 3/3] Revert unrelated changes

ScroogeBinaryTest.java
ThriftParquetReader.java
---
 .../parquet/scrooge/ScroogeBinaryTest.java    | 74 +++++++++----------
 .../parquet/thrift/ThriftParquetReader.java   | 12 +--
 .../parquet/hadoop/thrift/TestBinary.java     | 34 ++++-----
 3 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java b/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
index a6f4e326fa..bf0d615111 100644
--- a/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
+++ b/parquet-scrooge/src/test/java/org/apache/parquet/scrooge/ScroogeBinaryTest.java
@@ -1,20 +1,20 @@
-/*
-  Licensed to the Apache Software Foundation (ASF) under one
-  or more contributor license agreements.  See the NOTICE file
-  distributed with this work for additional information
-  regarding copyright ownership.  The ASF licenses this file
-  to you under the Apache License, Version 2.0 (the
-  "License"); you may not use this file except in compliance
-  with the License.  You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing,
-  software distributed under the License is distributed on an
-  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-  KIND, either express or implied.  See the License for the
-  specific language governing permissions and limitations
-  under the License.
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
  */
 package org.apache.parquet.scrooge;
 
@@ -47,19 +47,18 @@ public void testScroogeBinaryEncoding() throws Exception {
 
     Path path = new Path(temp.getPath());
 
-    try(ParquetWriter<StringAndBinary> writer = new ParquetWriter<StringAndBinary>(
-        path, new Configuration(), new ScroogeWriteSupport<StringAndBinary>(StringAndBinary.class))) {
-      writer.write(expected);
-    }
+    ParquetWriter<StringAndBinary> writer = new ParquetWriter<StringAndBinary>(
+        path, new Configuration(), new ScroogeWriteSupport<StringAndBinary>(StringAndBinary.class));
+    writer.write(expected);
+    writer.close();
 
     // read using the parquet-thrift version to isolate the write path
-    final org.apache.parquet.thrift.test.binary.StringAndBinary record;
-    try (ParquetReader<org.apache.parquet.thrift.test.binary.StringAndBinary> reader = ThriftParquetReader.<org.apache.parquet.thrift.test.binary.StringAndBinary>
+    ParquetReader<org.apache.parquet.thrift.test.binary.StringAndBinary> reader = ThriftParquetReader.<org.apache.parquet.thrift.test.binary.StringAndBinary>
         build(path)
         .withThriftClass(org.apache.parquet.thrift.test.binary.StringAndBinary.class)
-        .build()) {
-      record = reader.read();
-    }
+        .build();
+    org.apache.parquet.thrift.test.binary.StringAndBinary record = reader.read();
+    reader.close();
 
     Assert.assertEquals("String should match after serialization round trip",
         "test", record.s);
@@ -71,7 +70,7 @@ record = reader.read();
   @SuppressWarnings("unchecked")
   public void testScroogeBinaryDecoding() throws Exception {
     StringAndBinary expected = new StringAndBinary.Immutable("test",
-      ByteBuffer.wrap(new byte[]{-123, 20, 33}));
+        ByteBuffer.wrap(new byte[] {-123, 20, 33}));
 
     File temp = tempDir.newFile(UUID.randomUUID().toString());
     temp.deleteOnExit();
@@ -79,20 +78,19 @@ public void testScroogeBinaryDecoding() throws Exception {
 
     Path path = new Path(temp.getPath());
 
-    try (ParquetWriter<StringAndBinary> writer = new ParquetWriter<>(
-      path, new Configuration(), new ScroogeWriteSupport<>(StringAndBinary.class))) {
-      writer.write(expected);
-    }
+    ParquetWriter<StringAndBinary> writer = new ParquetWriter<StringAndBinary>(
+        path, new Configuration(), new ScroogeWriteSupport<StringAndBinary>(StringAndBinary.class));
+    writer.write(expected);
+    writer.close();
 
     Configuration conf = new Configuration();
     conf.set("parquet.thrift.converter.class", ScroogeRecordConverter.class.getName());
-    StringAndBinary record;
-    try(ParquetReader<StringAndBinary> reader = ParquetReader.<StringAndBinary>
-      builder(new ScroogeReadSupport(), path)
-      .withConf(conf)
-      .build()) {
-      record = reader.read();
-    }
+    ParquetReader<StringAndBinary> reader = ParquetReader.<StringAndBinary>
+        builder(new ScroogeReadSupport(), path)
+        .withConf(conf)
+        .build();
+    StringAndBinary record = reader.read();
+    reader.close();
 
     Assert.assertEquals("String should match after serialization round trip",
         "test", record.s());
diff --git a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
index 933f0601f3..602d757e55 100644
--- a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
+++ b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ThriftParquetReader.java
@@ -1,4 +1,4 @@
-/*
+/* 
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- *
+ * 
  *   http://www.apache.org/licenses/LICENSE-2.0
- *
+ * 
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -36,7 +36,7 @@
  * To read a parquet file into thrift objects
  * @param <T> the thrift type
  */
-public class ThriftParquetReader<T extends TBase<?, ?>> extends ParquetReader<T> {
+public class ThriftParquetReader<T extends TBase<?,?>> extends ParquetReader<T> {
 
   /**
    * @param file the file to read
@@ -84,11 +84,11 @@ public ThriftParquetReader(Configuration conf, Path file) throws IOException {
     super(conf, file, new ThriftReadSupport<T>());
   }
 
-  public static <T extends TBase<?, ?>> Builder<T> build(Path file) {
+  public static <T extends TBase<?,?>> Builder<T> build(Path file) {
     return new Builder<T>(file);
   }
 
-  public static class Builder<T extends TBase<?, ?>> {
+  public static class Builder<T extends TBase<?,?>> {
     private final Path file;
     private Configuration conf;
     private Filter filter;
diff --git a/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java b/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java
index d38c049f59..ac5a08b979 100644
--- a/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java
+++ b/parquet-thrift/src/test/java/org/apache/parquet/hadoop/thrift/TestBinary.java
@@ -1,20 +1,20 @@
-/*
-  Licensed to the Apache Software Foundation (ASF) under one
-  or more contributor license agreements.  See the NOTICE file
-  distributed with this work for additional information
-  regarding copyright ownership.  The ASF licenses this file
-  to you under the Apache License, Version 2.0 (the
-  "License"); you may not use this file except in compliance
-  with the License.  You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing,
-  software distributed under the License is distributed on an
-  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-  KIND, either express or implied.  See the License for the
-  specific language governing permissions and limitations
-  under the License.
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
  */
 package org.apache.parquet.hadoop.thrift;