You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by GitBox <gi...@apache.org> on 2022/06/06 17:51:23 UTC

[GitHub] [parquet-mr] steveloughran commented on a diff in pull request #951: PARQUET-2134: Fix type checking in HadoopStreams.wrap

steveloughran commented on code in PR #951:
URL: https://github.com/apache/parquet-mr/pull/951#discussion_r890388928


##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/HadoopStreams.java:
##########
@@ -51,7 +52,7 @@ public class HadoopStreams {
   public static SeekableInputStream wrap(FSDataInputStream stream) {
     Objects.requireNonNull(stream, "Cannot wrap a null input stream");
     if (byteBufferReadableClass != null && h2SeekableConstructor != null &&
-        byteBufferReadableClass.isInstance(stream.getWrappedStream())) {
+        isWrappedStreamByteBufferReadable(stream)) {

Review Comment:
   this is really going into the internals of the hadoop classes and potentially tricky if there is any dynamic decision making in the inner class. The good news there is I don't see anything doing that.
   
   there is a way to ask (hadoop 3.2+) if a stream does support the API before calling, using the StreamCapabilities interface.
   https://issues.apache.org/jira/browse/HDFS-14111
   
   ```java
   if (stream.hasCapability( "in:readbytebuffer") {
     // stream is confident it has the api
   ) else {
     // do the checking of the inner class
   }



##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/HadoopStreams.java:
##########
@@ -66,6 +67,15 @@ public static SeekableInputStream wrap(FSDataInputStream stream) {
     }
   }
 
+  private static boolean isWrappedStreamByteBufferReadable(FSDataInputStream stream) {
+    InputStream wrapped = stream.getWrappedStream();
+    if (wrapped instanceof FSDataInputStream) {
+      return isWrappedStreamByteBufferReadable(((FSDataInputStream) wrapped));

Review Comment:
   you can't. the inner stream is set in the constructor, so cannot take the not-yet-constructed class as an argument.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org