[orc] 01/01: ORC-419: Ensure to call `close` at RecordReaderImpl constructor exception

[om...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

omalley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/orc.git

commit 786fd3e117c5dfb5767a9e226da14fb80a50ffca
Author: Dongjoon Hyun <do...@apache.org>
AuthorDate: Wed Oct 10 22:55:25 2018 -0700

    ORC-419: Ensure to call `close` at RecordReaderImpl constructor exception
    
    Signed-off-by: Owen O'Malley <om...@apache.org>
---
 .../java/org/apache/orc/impl/RecordReaderImpl.java |  9 +++++-
 .../org/apache/orc/impl/TestRecordReaderImpl.java  | 35 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
index 731b46e..2b8a6bf 100644
--- a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
+++ b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
@@ -269,7 +269,14 @@ public class RecordReaderImpl implements RecordReader {
     indexes = new OrcProto.RowIndex[types.size()];
     bloomFilterIndices = new OrcProto.BloomFilterIndex[types.size()];
     bloomFilterKind = new OrcProto.Stream.Kind[types.size()];
-    advanceToNextRow(reader, 0L, true);
+
+    try {
+      advanceToNextRow(reader, 0L, true);
+    } catch (IOException e) {
+      // Try to close since this happens in constructor.
+      close();
+      throw e;
+    }
   }
 
   public static final class PositionProviderImpl implements PositionProvider {
diff --git a/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java b/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
index 66951ff..529a08b 100644
--- a/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
+++ b/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
@@ -24,10 +24,12 @@ import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -54,6 +56,8 @@ import org.apache.hadoop.fs.PositionedReadable;
 import org.apache.hadoop.fs.Seekable;
 import org.apache.hadoop.hive.common.io.DiskRangeList;
 import org.apache.hadoop.hive.common.type.HiveDecimal;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
 import org.apache.hadoop.hive.ql.io.sarg.SearchArgument;
 import org.apache.hadoop.hive.ql.io.sarg.SearchArgumentFactory;
 import org.apache.hadoop.hive.ql.io.sarg.SearchArgumentImpl;
@@ -2119,4 +2123,35 @@ public class TestRecordReaderImpl {
     RecordReader recordReader = reader.rows(readerOptions);
     recordReader.close();
   }
+
+  @Test
+  public void testCloseAtConstructorException() throws Exception {
+    Configuration conf = new Configuration();
+    Path path = new Path(workDir, "oneRow.orc");
+    FileSystem.get(conf).delete(path, true);
+
+    TypeDescription schema = TypeDescription.createLong();
+    OrcFile.WriterOptions options = OrcFile.writerOptions(conf).setSchema(schema);
+    Writer writer = OrcFile.createWriter(path, options);
+    VectorizedRowBatch writeBatch = schema.createRowBatch();
+    int row = writeBatch.size++;
+    ((LongColumnVector) writeBatch.cols[0]).vector[row] = 0;
+    writer.addRowBatch(writeBatch);
+    writer.close();
+
+    DataReader mockedDataReader = mock(DataReader.class);
+    when(mockedDataReader.clone()).thenReturn(mockedDataReader);
+    doThrow(new IOException()).when(mockedDataReader).readStripeFooter(any());
+
+    Reader reader = OrcFile.createReader(path, OrcFile.readerOptions(conf));
+    Reader.Options readerOptions = reader.options().dataReader(mockedDataReader);
+    boolean isCalled = false;
+    try {
+      reader.rows(readerOptions);
+    } catch (IOException ie) {
+      isCalled = true;
+    }
+    assertTrue(isCalled);
+    verify(mockedDataReader, times(1)).close();
+  }
 }