Skip to content

Commit beec0e4

Browse files
sdruzkinNikhilCollooru
sdruzkin
authored and
NikhilCollooru
committed
Fix confusing OrcCorruptionException thrown by ORC metadata reader (#24897)
Summary: Pull Request resolved: #24897 Make ORC ExceptionWrappingMetadataReader throw OrcCorruptionException only in case of protobuf exceptions, and make it re-throw the rest of exceptions. Reviewed By: NikhilCollooru, sathishkrishnaraju Differential Revision: D72724589
1 parent 7820553 commit beec0e4

File tree

2 files changed

+211
-25
lines changed

2 files changed

+211
-25
lines changed

‎presto-orc/src/main/java/com/facebook/presto/orc/metadata/ExceptionWrappingMetadataReader.java

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.facebook.presto.orc.OrcDecompressor;
2222
import com.facebook.presto.orc.metadata.PostScript.HiveWriterVersion;
2323
import com.facebook.presto.orc.metadata.statistics.HiveBloomFilter;
24+
import com.facebook.presto.orc.protobuf.InvalidProtocolBufferException;
2425

2526
import java.io.IOException;
2627
import java.io.InputStream;
@@ -45,25 +46,25 @@ public ExceptionWrappingMetadataReader(OrcDataSourceId orcDataSourceId, Metadata
4546

4647
@Override
4748
public PostScript readPostScript(byte[] data, int offset, int length)
48-
throws OrcCorruptionException
49+
throws OrcCorruptionException, IOException
4950
{
5051
try {
5152
return delegate.readPostScript(data, offset, length);
5253
}
53-
catch (IOException | RuntimeException e) {
54-
throw propagate(e, "Invalid postscript");
54+
catch (InvalidProtocolBufferException e) {
55+
throw new OrcCorruptionException(e, orcDataSourceId, "Invalid postscript");
5556
}
5657
}
5758

5859
@Override
5960
public Metadata readMetadata(HiveWriterVersion hiveWriterVersion, InputStream inputStream)
60-
throws OrcCorruptionException
61+
throws OrcCorruptionException, IOException
6162
{
6263
try {
6364
return delegate.readMetadata(hiveWriterVersion, inputStream);
6465
}
65-
catch (IOException | RuntimeException e) {
66-
throw propagate(e, "Invalid file metadata");
66+
catch (InvalidProtocolBufferException e) {
67+
throw new OrcCorruptionException(e, orcDataSourceId, "Invalid file metadata");
6768
}
6869
}
6970

@@ -74,13 +75,13 @@ public Footer readFooter(HiveWriterVersion hiveWriterVersion,
7475
DwrfKeyProvider dwrfKeyProvider,
7576
OrcDataSource orcDataSource,
7677
Optional<OrcDecompressor> decompressor)
77-
throws OrcCorruptionException
78+
throws OrcCorruptionException, IOException
7879
{
7980
try {
8081
return delegate.readFooter(hiveWriterVersion, inputStream, dwrfEncryptionProvider, dwrfKeyProvider, orcDataSource, decompressor);
8182
}
82-
catch (IOException | RuntimeException e) {
83-
throw propagate(e, "Invalid file footer");
83+
catch (InvalidProtocolBufferException e) {
84+
throw new OrcCorruptionException(e, orcDataSourceId, "Invalid file footer");
8485
}
8586
}
8687

@@ -91,40 +92,32 @@ public StripeFooter readStripeFooter(OrcDataSourceId orcDataSourceId, List<OrcTy
9192
try {
9293
return delegate.readStripeFooter(orcDataSourceId, types, inputStream);
9394
}
94-
catch (IOException e) {
95-
throw propagate(e, "Invalid stripe footer");
95+
catch (InvalidProtocolBufferException e) {
96+
throw new OrcCorruptionException(e, orcDataSourceId, "Invalid stripe footer");
9697
}
9798
}
9899

99100
@Override
100101
public List<RowGroupIndex> readRowIndexes(HiveWriterVersion hiveWriterVersion, InputStream inputStream, List<HiveBloomFilter> bloomFilters)
101-
throws OrcCorruptionException
102+
throws OrcCorruptionException, IOException
102103
{
103104
try {
104105
return delegate.readRowIndexes(hiveWriterVersion, inputStream, bloomFilters);
105106
}
106-
catch (IOException | RuntimeException e) {
107-
throw propagate(e, "Invalid stripe row index");
107+
catch (InvalidProtocolBufferException e) {
108+
throw new OrcCorruptionException(e, orcDataSourceId, "Invalid stripe row index");
108109
}
109110
}
110111

111112
@Override
112113
public List<HiveBloomFilter> readBloomFilterIndexes(InputStream inputStream)
113-
throws OrcCorruptionException
114+
throws OrcCorruptionException, IOException
114115
{
115116
try {
116117
return delegate.readBloomFilterIndexes(inputStream);
117118
}
118-
catch (IOException | RuntimeException e) {
119-
throw propagate(e, "Invalid bloom filter");
119+
catch (InvalidProtocolBufferException e) {
120+
throw new OrcCorruptionException(e, orcDataSourceId, "Invalid bloom filter");
120121
}
121122
}
122-
123-
private OrcCorruptionException propagate(Throwable throwable, String message)
124-
{
125-
if (throwable.getClass().getSimpleName().equals("PrestoException")) {
126-
throw (RuntimeException) throwable;
127-
}
128-
return new OrcCorruptionException(throwable, orcDataSourceId, message);
129-
}
130123
}

‎presto-orc/src/test/java/com/facebook/presto/orc/metadata/TestExceptionWrappingMetadataReader.java

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.facebook.presto.orc.metadata;
15+
16+
import com.facebook.presto.orc.DwrfEncryptionProvider;
17+
import com.facebook.presto.orc.DwrfKeyProvider;
18+
import com.facebook.presto.orc.OrcCorruptionException;
19+
import com.facebook.presto.orc.OrcDataSource;
20+
import com.facebook.presto.orc.OrcDataSourceId;
21+
import com.facebook.presto.orc.OrcDecompressor;
22+
import com.facebook.presto.orc.metadata.statistics.HiveBloomFilter;
23+
import com.facebook.presto.orc.protobuf.InvalidProtocolBufferException;
24+
import com.google.common.collect.ImmutableList;
25+
import org.testng.Assert.ThrowingRunnable;
26+
import org.testng.annotations.Test;
27+
28+
import java.io.IOException;
29+
import java.io.InputStream;
30+
import java.util.List;
31+
import java.util.Optional;
32+
33+
import static com.facebook.presto.orc.metadata.PostScript.HiveWriterVersion.ORIGINAL;
34+
import static com.google.common.base.Preconditions.checkState;
35+
import static org.testng.Assert.expectThrows;
36+
37+
public class TestExceptionWrappingMetadataReader
38+
{
39+
private static final OrcDataSourceId ORC_DATA_SOURCE_ID = new OrcDataSourceId("test");
40+
41+
@Test
42+
public void testReadPostScriptExceptionHandling()
43+
{
44+
byte[] data = new byte[10];
45+
TestMetadataReader delegate = new TestMetadataReader();
46+
ExceptionWrappingMetadataReader metadataReader = new ExceptionWrappingMetadataReader(ORC_DATA_SOURCE_ID, delegate);
47+
assertExceptions(() -> metadataReader.readPostScript(data, 0, 1));
48+
delegate.assertAllExceptionsThrown();
49+
}
50+
51+
@Test
52+
public void testReadMetadataExceptionHandling()
53+
{
54+
TestMetadataReader delegate = new TestMetadataReader();
55+
ExceptionWrappingMetadataReader metadataReader = new ExceptionWrappingMetadataReader(ORC_DATA_SOURCE_ID, delegate);
56+
ThrowingRunnable lambda = () -> metadataReader.readMetadata(ORIGINAL, null);
57+
assertExceptions(lambda);
58+
delegate.assertAllExceptionsThrown();
59+
}
60+
61+
@Test
62+
public void testReadFooterExceptionHandling()
63+
{
64+
TestMetadataReader delegate = new TestMetadataReader();
65+
ExceptionWrappingMetadataReader metadataReader = new ExceptionWrappingMetadataReader(ORC_DATA_SOURCE_ID, delegate);
66+
ThrowingRunnable lambda = () -> metadataReader.readFooter(
67+
ORIGINAL,
68+
null,
69+
null,
70+
null,
71+
null,
72+
null);
73+
assertExceptions(lambda);
74+
delegate.assertAllExceptionsThrown();
75+
}
76+
77+
@Test
78+
public void testReadStripeFooterExceptionHandling()
79+
{
80+
TestMetadataReader delegate = new TestMetadataReader();
81+
ExceptionWrappingMetadataReader metadataReader = new ExceptionWrappingMetadataReader(ORC_DATA_SOURCE_ID, delegate);
82+
ThrowingRunnable lambda = () -> metadataReader.readStripeFooter(ORC_DATA_SOURCE_ID, null, null);
83+
assertExceptions(lambda);
84+
delegate.assertAllExceptionsThrown();
85+
}
86+
87+
@Test
88+
public void testReadRowIndexesExceptionHandling()
89+
{
90+
TestMetadataReader delegate = new TestMetadataReader();
91+
ExceptionWrappingMetadataReader metadataReader = new ExceptionWrappingMetadataReader(ORC_DATA_SOURCE_ID, delegate);
92+
ThrowingRunnable lambda = () -> metadataReader.readRowIndexes(ORIGINAL, null, null);
93+
assertExceptions(lambda);
94+
delegate.assertAllExceptionsThrown();
95+
}
96+
97+
@Test
98+
public void testReadBloomFilterIndexesExceptionHandling()
99+
{
100+
TestMetadataReader delegate = new TestMetadataReader();
101+
ExceptionWrappingMetadataReader metadataReader = new ExceptionWrappingMetadataReader(ORC_DATA_SOURCE_ID, delegate);
102+
ThrowingRunnable lambda = () -> metadataReader.readBloomFilterIndexes(null);
103+
assertExceptions(lambda);
104+
delegate.assertAllExceptionsThrown();
105+
}
106+
107+
private static void assertExceptions(ThrowingRunnable lambda)
108+
{
109+
expectThrows(RuntimeException.class, lambda);
110+
expectThrows(IOException.class, lambda);
111+
expectThrows(OrcCorruptionException.class, lambda);
112+
}
113+
114+
private static class TestMetadataReader
115+
implements MetadataReader
116+
{
117+
private static final List<Exception> EXCEPTIONS = ImmutableList.of(
118+
new RuntimeException("test runtime exception"),
119+
new IOException("test io exception"),
120+
new InvalidProtocolBufferException("test protobuf exception"));
121+
private int currentExceptionIndex;
122+
123+
private void throwNextException()
124+
throws IOException
125+
{
126+
checkState(currentExceptionIndex < EXCEPTIONS.size());
127+
Exception ex = EXCEPTIONS.get(currentExceptionIndex++);
128+
if (ex instanceof IOException) {
129+
throw (IOException) ex;
130+
}
131+
throw (RuntimeException) ex;
132+
}
133+
134+
public void assertAllExceptionsThrown()
135+
{
136+
checkState(currentExceptionIndex == EXCEPTIONS.size());
137+
}
138+
139+
@Override
140+
public PostScript readPostScript(byte[] data, int offset, int length)
141+
throws IOException
142+
{
143+
throwNextException();
144+
return null;
145+
}
146+
147+
@Override
148+
public Metadata readMetadata(PostScript.HiveWriterVersion hiveWriterVersion, InputStream inputStream)
149+
throws IOException
150+
{
151+
throwNextException();
152+
return null;
153+
}
154+
155+
@Override
156+
public Footer readFooter(
157+
PostScript.HiveWriterVersion hiveWriterVersion,
158+
InputStream inputStream,
159+
DwrfEncryptionProvider dwrfEncryptionProvider,
160+
DwrfKeyProvider dwrfKeyProvider,
161+
OrcDataSource orcDataSource,
162+
Optional<OrcDecompressor> decompressor)
163+
throws IOException
164+
{
165+
throwNextException();
166+
return null;
167+
}
168+
169+
@Override
170+
public StripeFooter readStripeFooter(OrcDataSourceId orcDataSourceId, List<OrcType> types, InputStream inputStream)
171+
throws IOException
172+
{
173+
throwNextException();
174+
return null;
175+
}
176+
177+
@Override
178+
public List<RowGroupIndex> readRowIndexes(PostScript.HiveWriterVersion hiveWriterVersion, InputStream inputStream, List<HiveBloomFilter> bloomFilters)
179+
throws IOException
180+
{
181+
throwNextException();
182+
return null;
183+
}
184+
185+
@Override
186+
public List<HiveBloomFilter> readBloomFilterIndexes(InputStream inputStream)
187+
throws IOException
188+
{
189+
throwNextException();
190+
return null;
191+
}
192+
}
193+
}

0 commit comments

Comments
 (0)