[GitHub] orc pull request #218: ORC-301. `extractFileTail` should open a file in `try...
Github user prasanthj commented on a diff in the pull request: https://github.com/apache/orc/pull/218#discussion_r168586970 --- Diff: java/core/src/java/org/apache/orc/impl/ReaderImpl.java --- @@ -516,12 +516,13 @@ OrcTail buildEmptyTail() { protected OrcTail extractFileTail(FileSystem fs, Path path, long maxFileLength) throws IOException { -FSDataInputStream file = fs.open(path); +FSDataInputStream file = null; ByteBuffer buffer; OrcProto.PostScript ps; OrcProto.FileTail.Builder fileTailBuilder = OrcProto.FileTail.newBuilder(); long modificationTime; try { --- End diff -- Yeah that's right. ---
[GitHub] orc pull request #218: ORC-301. `extractFileTail` should open a file in `try...
Github user prasanthj commented on a diff in the pull request: https://github.com/apache/orc/pull/218#discussion_r168572505 --- Diff: java/core/src/java/org/apache/orc/impl/ReaderImpl.java --- @@ -516,12 +516,13 @@ OrcTail buildEmptyTail() { protected OrcTail extractFileTail(FileSystem fs, Path path, long maxFileLength) throws IOException { -FSDataInputStream file = fs.open(path); +FSDataInputStream file = null; ByteBuffer buffer; OrcProto.PostScript ps; OrcProto.FileTail.Builder fileTailBuilder = OrcProto.FileTail.newBuilder(); long modificationTime; try { --- End diff -- Line 610 doesn't add much value as close() will throw IOException anyway which should be sufficient to know the reason. ---
[GitHub] orc pull request #218: ORC-301. `extractFileTail` should open a file in `try...
Github user dongjoon-hyun commented on a diff in the pull request: https://github.com/apache/orc/pull/218#discussion_r168372882 --- Diff: java/core/src/java/org/apache/orc/impl/ReaderImpl.java --- @@ -516,12 +516,13 @@ OrcTail buildEmptyTail() { protected OrcTail extractFileTail(FileSystem fs, Path path, long maxFileLength) throws IOException { -FSDataInputStream file = fs.open(path); +FSDataInputStream file = null; ByteBuffer buffer; OrcProto.PostScript ps; OrcProto.FileTail.Builder fileTailBuilder = OrcProto.FileTail.newBuilder(); long modificationTime; try { --- End diff -- Thank you for review, @prasanthj . Ya. I thought to use `try-with-resource` first, but it needs more change to print out the log at [Line 610](https://github.com/apache/orc/pull/218/files#diff-90ceebe6c0bfa3e98b4536455a60b498R610). ---
[GitHub] orc pull request #218: ORC-301. `extractFileTail` should open a file in `try...
Github user dongjoon-hyun commented on a diff in the pull request: https://github.com/apache/orc/pull/218#discussion_r168372943 --- Diff: java/core/src/java/org/apache/orc/impl/ReaderImpl.java --- @@ -516,12 +516,13 @@ OrcTail buildEmptyTail() { protected OrcTail extractFileTail(FileSystem fs, Path path, long maxFileLength) throws IOException { -FSDataInputStream file = fs.open(path); +FSDataInputStream file = null; ByteBuffer buffer; OrcProto.PostScript ps; OrcProto.FileTail.Builder fileTailBuilder = OrcProto.FileTail.newBuilder(); long modificationTime; try { --- End diff -- I think this is the minimal change while keeping the current behavior. ---