Skip to content

Commit cfaea42

Browse files
wesmwesm
committed
PARQUET-1481: [C++] Throw exception when encountering bad Thrift metadata in RecordReader
Author: Wes McKinney <wesm+git@apache.org> Closes apache#3242 from wesm/PARQUET-1481 and squashes the following commits: b074227 <Wes McKinney> Add test case with example corrupt data file 59400a2 <Wes McKinney> Throw exception when encountering bad Thrift metadata in RecordReader
1 parent cd543b9 commit cfaea42

3 files changed

Lines changed: 28 additions & 11 deletions

File tree

‎cpp/src/parquet/arrow/arrow-reader-writer-test.cc‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,21 +2291,34 @@ TEST(TestImpalaConversion, ArrowTimestampToImpalaTimestamp) {
22912291
ASSERT_EQ(expected, calculated);
22922292
}
22932293

2294-
TEST(TestArrowReaderAdHoc, Int96BadMemoryAccess) {
2295-
// PARQUET-995
2294+
void TryReadDataFile(const std::string& testing_file_path, bool should_succeed = true) {
22962295
std::string dir_string(test::get_data_dir());
22972296
std::stringstream ss;
2298-
ss << dir_string << "/"
2299-
<< "alltypes_plain.parquet";
2297+
ss << dir_string << "/" << testing_file_path;
23002298
auto path = ss.str();
23012299

23022300
auto pool = ::arrow::default_memory_pool();
23032301

23042302
std::unique_ptr<FileReader> arrow_reader;
2305-
ASSERT_NO_THROW(
2306-
arrow_reader.reset(new FileReader(pool, ParquetFileReader::OpenFile(path, false))));
2307-
std::shared_ptr<::arrow::Table> table;
2308-
ASSERT_OK_NO_THROW(arrow_reader->ReadTable(&table));
2303+
try {
2304+
arrow_reader.reset(new FileReader(pool, ParquetFileReader::OpenFile(path, false)));
2305+
std::shared_ptr<::arrow::Table> table;
2306+
ASSERT_OK(arrow_reader->ReadTable(&table));
2307+
} catch (const ParquetException& e) {
2308+
if (should_succeed) {
2309+
FAIL() << "Exception thrown when reading file: " << e.what();
2310+
}
2311+
}
2312+
}
2313+
2314+
TEST(TestArrowReaderAdHoc, Int96BadMemoryAccess) {
2315+
// PARQUET-995
2316+
TryReadDataFile("alltypes_plain.parquet");
2317+
}
2318+
2319+
TEST(TestArrowReaderAdHoc, CorruptedSchema) {
2320+
// PARQUET-1481
2321+
TryReadDataFile("bad_data/PARQUET-1481.parquet", false /* should_succeed */);
23092322
}
23102323

23112324
class TestArrowReaderAdHocSparkAndHvr

‎cpp/src/parquet/arrow/record_reader.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -850,8 +850,12 @@ std::shared_ptr<RecordReader> RecordReader::Make(const ColumnDescriptor* descr,
850850
case Type::FIXED_LEN_BYTE_ARRAY:
851851
return std::shared_ptr<RecordReader>(
852852
new RecordReader(new TypedRecordReader<FLBAType>(descr, pool)));
853-
default:
854-
DCHECK(false);
853+
default: {
854+
// PARQUET-1481: This can occur if the file is corrupt
855+
std::stringstream ss;
856+
ss << "Invalid physical column type: " << static_cast<int>(descr->physical_type());
857+
throw ParquetException(ss.str());
858+
}
855859
}
856860
// Unreachable code, but supress compiler warning
857861
return nullptr;

‎cpp/submodules/parquet-testing‎

0 commit comments

Comments
 (0)