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 <[email protected]> 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 <[email protected]> --- .../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(); + } }
