PARQUET-2005: Upgrade thrift to 0.14.1 (apache#884)

* PARQUET-2005: Upgrade thrift to 0.14.1 * PARQUET-2005: Update thrift version in CI scripts and README * Update README.md Co-authored-by: Fokko Driesprong <[email protected]> Co-authored-by: Fokko Driesprong <[email protected]>
JunchengMa · Apr 8, 2021 · d23a9a7 · d23a9a7
1 parent 8624081
commit d23a9a7
Show file tree

Hide file tree

Showing 10 changed files with 34 additions and 23 deletions.
diff --git a/README.md b/README.md
@@ -35,19 +35,19 @@ Parquet-MR uses Maven to build and depends on the thrift compiler (protoc is now
 To build and install the thrift compiler, run:
 
 ```
-wget -nv http://archive.apache.org/dist/thrift/0.13.0/thrift-0.13.0.tar.gz
-tar xzf thrift-0.13.0.tar.gz
-cd thrift-0.13.0
+wget -nv http://archive.apache.org/dist/thrift/0.14.1/thrift-0.14.1.tar.gz
+tar xzf thrift-0.14.1.tar.gz
+cd thrift-0.14.1
 chmod +x ./configure
 ./configure --disable-libs
 sudo make install
 ```
 
-If you're on OSX and use homebrew, you can instead install Thrift 0.13.0 with `brew` and ensure that it comes first in your `PATH`.
+If you're on OSX and use homebrew, you can instead install Thrift 0.14.1 with `brew` and ensure that it comes first in your `PATH`.
 
 ```
-brew install thrift@0.13
-export PATH="/usr/local/opt/thrift@0.13.0/bin:$PATH"
+brew install thrift
+export PATH="/usr/local/opt/thrift@0.14.1/bin:$PATH"
 ```
 
 ### Build Parquet with Maven

diff --git a/dev/ci-before_install.sh b/dev/ci-before_install.sh
@@ -20,7 +20,7 @@
 # This script gets invoked by the CI system in a "before install" step
 ################################################################################
 
-export THRIFT_VERSION=0.13.0
+export THRIFT_VERSION=0.14.1
 
 set -e
 date

diff --git a/parquet-format-structures/src/main/java/org/apache/parquet/format/CliUtils.java b/parquet-format-structures/src/main/java/org/apache/parquet/format/CliUtils.java
@@ -40,8 +40,8 @@ public class CliUtils {
    * @throws IOException if any Thrift error occurs during the serialization
    */
   public static String toJson(TBase<?, ?> tbase) throws IOException {
-    TSerializer serializer = new TSerializer(new TSimpleJSONProtocol.Factory());
     try {
+      TSerializer serializer = new TSerializer(new TSimpleJSONProtocol.Factory());
       return serializer.toString(tbase);
     } catch (TException e) {
       // Wrapping the exception the not to expose the shaded Thrift class TException

diff --git a/parquet-format-structures/src/main/java/org/apache/parquet/format/InterningProtocol.java b/parquet-format-structures/src/main/java/org/apache/parquet/format/InterningProtocol.java
@@ -228,4 +228,9 @@ public String toString() {
     return delegate.toString();
   }
 
+  @Override
+  public int getMinSerializedSize(byte type) throws TException {
+    return delegate.getMinSerializedSize(type);
+  }
+
 }
diff --git a/parquet-format-structures/src/main/java/org/apache/parquet/format/Util.java b/parquet-format-structures/src/main/java/org/apache/parquet/format/Util.java
@@ -45,6 +45,7 @@
 import org.apache.thrift.protocol.TProtocol;
 import org.apache.thrift.transport.TIOStreamTransport;
 import org.apache.thrift.transport.TMemoryBuffer;
+import org.apache.thrift.transport.TTransportException;
 import org.apache.parquet.format.event.Consumers.Consumer;
 import org.apache.parquet.format.event.Consumers.DelegatingFieldConsumer;
 import org.apache.parquet.format.event.EventBasedThriftReader;
@@ -336,11 +337,11 @@ public void consume(RowGroup rowGroup) {
     }
   }
 
-  private static TProtocol protocol(OutputStream to) {
+  private static TProtocol protocol(OutputStream to) throws TTransportException {
     return protocol(new TIOStreamTransport(to));
   }
 
-  private static TProtocol protocol(InputStream from) {
+  private static TProtocol protocol(InputStream from) throws TTransportException {
     return protocol(new TIOStreamTransport(from));
   }
 

diff --git a/parquet-thrift/src/main/java/org/apache/parquet/hadoop/thrift/ThriftBytesWriteSupport.java b/parquet-thrift/src/main/java/org/apache/parquet/hadoop/thrift/ThriftBytesWriteSupport.java
@@ -30,6 +30,7 @@
 import org.apache.thrift.protocol.TProtocol;
 import org.apache.thrift.protocol.TProtocolFactory;
 import org.apache.thrift.transport.TIOStreamTransport;
+import org.apache.thrift.transport.TTransportException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.parquet.hadoop.BadConfigurationException;
@@ -156,7 +157,7 @@ public WriteContext init(Configuration configuration) {
     }
   }
 
-  private TProtocol protocol(BytesWritable record) {
+  private TProtocol protocol(BytesWritable record) throws TTransportException {
     TProtocol protocol = protocolFactory.getProtocol(new TIOStreamTransport(new ByteArrayInputStream(record.getBytes())));
 
     /* Reduce the chance of OOM when data is corrupted. When readBinary is called on TBinaryProtocol, it reads the length of the binary first,

diff --git a/parquet-thrift/src/main/java/org/apache/parquet/thrift/BufferedProtocolReadToWrite.java b/parquet-thrift/src/main/java/org/apache/parquet/thrift/BufferedProtocolReadToWrite.java
@@ -728,6 +728,11 @@ public String readString() throws TException {
     public ByteBuffer readBinary() throws TException {
       return null;
     }
+
+    @Override
+    public int getMinSerializedSize(byte type) throws TException {
+      return 0;
+    }
   }
 
 }
diff --git a/parquet-thrift/src/main/java/org/apache/parquet/thrift/ParquetProtocol.java b/parquet-thrift/src/main/java/org/apache/parquet/thrift/ParquetProtocol.java
@@ -274,4 +274,9 @@ public ByteBuffer readBinary() throws TException {
     throw exception();
   }
 
+  @Override
+  public int getMinSerializedSize(byte type) throws TException {
+    throw exception();
+  }
+
 }
diff --git a/parquet-thrift/src/test/java/org/apache/parquet/thrift/TestProtocolReadToWrite.java b/parquet-thrift/src/test/java/org/apache/parquet/thrift/TestProtocolReadToWrite.java
@@ -25,6 +25,7 @@
 import org.apache.thrift.protocol.TCompactProtocol;
 import org.apache.thrift.protocol.TField;
 import org.apache.thrift.transport.TIOStreamTransport;
+import org.apache.thrift.transport.TTransportException;
 import org.junit.Test;
 import org.apache.parquet.thrift.test.Phone;
 import org.apache.parquet.thrift.test.StructWithExtraField;
@@ -358,11 +359,11 @@ public void handleFieldIgnored(TField field) {
     assertEquals(1, countingHandler.fieldIgnoredCount);
   }
 
-  private TCompactProtocol protocol(OutputStream to) {
+  private TCompactProtocol protocol(OutputStream to) throws TTransportException {
     return new TCompactProtocol(new TIOStreamTransport(to));
   }
 
-  private TCompactProtocol protocol(InputStream from) {
+  private TCompactProtocol protocol(InputStream from) throws TTransportException {
     return new TCompactProtocol(new TIOStreamTransport(from));
   }
 

diff --git a/pom.xml b/pom.xml
@@ -85,7 +85,7 @@
     <parquet.format.version>2.8.0</parquet.format.version>
     <previous.version>1.12.0</previous.version>
     <thrift.executable>thrift</thrift.executable>
-    <format.thrift.executable>thrift</format.thrift.executable>
+    <format.thrift.executable>${thrift.executable}</format.thrift.executable>
     <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.12</scala.binary.version>
@@ -94,8 +94,8 @@
     <pig.version>0.16.0</pig.version>
     <pig.classifier>h2</pig.classifier>
     <thrift-maven-plugin.version>0.10.0</thrift-maven-plugin.version>
-    <thrift.version>0.13.0</thrift.version>
-    <format.thrift.version>0.13.0</format.thrift.version>
+    <thrift.version>0.14.1</thrift.version>
+    <format.thrift.version>${thrift.version}</format.thrift.version>
     <fastutil.version>8.4.2</fastutil.version>
     <semver.api.version>0.9.33</semver.api.version>
     <slf4j.version>1.7.22</slf4j.version>
@@ -586,13 +586,6 @@
       </build>
     </profile>
 
-    <profile>
-      <id>thrift9</id>
-      <properties>
-        <thrift.version>0.9.0</thrift.version>
-      </properties>
-    </profile>
-
     <!-- Profile for CI tests to have less output -->
     <profile>
       <id>ci-test</id>