PARQUET-1167: [C++] FieldToNode function should return a status when throwing an exception
Author: Phillip Cloud <cpcloud@gmail.com>
Closes #421 from cpcloud/PARQUET-1167 and squashes the following commits:
2ae1953 [Phillip Cloud] Formatting
b465d91 [Phillip Cloud] PARQUET-1167: [C++] FieldToNode function should return a status when throwing an exception
diff --git a/src/parquet/arrow/arrow-schema-test.cc b/src/parquet/arrow/arrow-schema-test.cc
index 129eccf..771b996 100644
--- a/src/parquet/arrow/arrow-schema-test.cc
+++ b/src/parquet/arrow/arrow-schema-test.cc
@@ -62,8 +62,8 @@
for (int i = 0; i < expected_schema->num_fields(); ++i) {
auto lhs = result_schema_->field(i);
auto rhs = expected_schema->field(i);
- EXPECT_TRUE(lhs->Equals(rhs)) << i << " " << lhs->ToString()
- << " != " << rhs->ToString();
+ EXPECT_TRUE(lhs->Equals(rhs))
+ << i << " " << lhs->ToString() << " != " << rhs->ToString();
}
}
@@ -807,5 +807,17 @@
CheckFlatSchema(parquet_fields);
}
+TEST(InvalidSchema, ParquetNegativeDecimalScale) {
+ const auto& type = ::arrow::decimal(23, -2);
+ const auto& field = ::arrow::field("f0", type);
+ const auto& arrow_schema = ::arrow::schema({field});
+ std::shared_ptr<::parquet::WriterProperties> properties =
+ ::parquet::default_writer_properties();
+ std::shared_ptr<SchemaDescriptor> result_schema;
+
+ ASSERT_RAISES(IOError,
+ ToParquetSchema(arrow_schema.get(), *properties.get(), &result_schema));
+}
+
} // namespace arrow
} // namespace parquet
diff --git a/src/parquet/arrow/schema.cc b/src/parquet/arrow/schema.cc
index 41da9fb..321bd20 100644
--- a/src/parquet/arrow/schema.cc
+++ b/src/parquet/arrow/schema.cc
@@ -601,8 +601,9 @@
return Status::NotImplemented(ss.str());
}
}
- *out = PrimitiveNode::Make(field->name(), repetition, type, logical_type, length,
- precision, scale);
+ PARQUET_CATCH_NOT_OK(*out =
+ PrimitiveNode::Make(field->name(), repetition, type,
+ logical_type, length, precision, scale));
return Status::OK();
}
diff --git a/src/parquet/file/file-serialize-test.cc b/src/parquet/file/file-serialize-test.cc
index f9f12be..4d94d2e 100644
--- a/src/parquet/file/file-serialize-test.cc
+++ b/src/parquet/file/file-serialize-test.cc
@@ -209,17 +209,11 @@
this->FileSerializeTest(Compression::BROTLI);
}
-TYPED_TEST(TestSerialize, SmallFileGzip) {
- this->FileSerializeTest(Compression::GZIP);
-}
+TYPED_TEST(TestSerialize, SmallFileGzip) { this->FileSerializeTest(Compression::GZIP); }
-TYPED_TEST(TestSerialize, SmallFileLz4) {
- this->FileSerializeTest(Compression::LZ4);
-}
+TYPED_TEST(TestSerialize, SmallFileLz4) { this->FileSerializeTest(Compression::LZ4); }
-TYPED_TEST(TestSerialize, SmallFileZstd) {
- this->FileSerializeTest(Compression::ZSTD);
-}
+TYPED_TEST(TestSerialize, SmallFileZstd) { this->FileSerializeTest(Compression::ZSTD); }
} // namespace test
diff --git a/src/parquet/file/reader.cc b/src/parquet/file/reader.cc
index 9b9bde9..4ec48a4 100644
--- a/src/parquet/file/reader.cc
+++ b/src/parquet/file/reader.cc
@@ -45,9 +45,9 @@
: contents_(std::move(contents)) {}
std::shared_ptr<ColumnReader> RowGroupReader::Column(int i) {
- DCHECK(i < metadata()->num_columns()) << "The RowGroup only has "
- << metadata()->num_columns()
- << "columns, requested column: " << i;
+ DCHECK(i < metadata()->num_columns())
+ << "The RowGroup only has " << metadata()->num_columns()
+ << "columns, requested column: " << i;
const ColumnDescriptor* descr = metadata()->schema()->Column(i);
std::unique_ptr<PageReader> page_reader = contents_->GetColumnPageReader(i);
@@ -57,9 +57,9 @@
}
std::unique_ptr<PageReader> RowGroupReader::GetColumnPageReader(int i) {
- DCHECK(i < metadata()->num_columns()) << "The RowGroup only has "
- << metadata()->num_columns()
- << "columns, requested column: " << i;
+ DCHECK(i < metadata()->num_columns())
+ << "The RowGroup only has " << metadata()->num_columns()
+ << "columns, requested column: " << i;
return contents_->GetColumnPageReader(i);
}
@@ -127,9 +127,9 @@
}
std::shared_ptr<RowGroupReader> ParquetFileReader::RowGroup(int i) {
- DCHECK(i < metadata()->num_row_groups()) << "The file only has "
- << metadata()->num_row_groups()
- << "row groups, requested reader for: " << i;
+ DCHECK(i < metadata()->num_row_groups())
+ << "The file only has " << metadata()->num_row_groups()
+ << "row groups, requested reader for: " << i;
return contents_->GetRowGroup(i);
}
diff --git a/src/parquet/schema.cc b/src/parquet/schema.cc
index 8168dc4..fa6116f 100644
--- a/src/parquet/schema.cc
+++ b/src/parquet/schema.cc
@@ -137,11 +137,13 @@
throw ParquetException(ss.str());
}
if (precision <= 0) {
- ss << "Invalid DECIMAL precision: " << precision;
+ ss << "Invalid DECIMAL precision: " << precision
+ << ". Precision must be a number between 1 and 38 inclusive";
throw ParquetException(ss.str());
}
if (scale < 0) {
- ss << "Invalid DECIMAL scale: " << scale;
+ ss << "Invalid DECIMAL scale: " << scale
+ << ". Scale must be a number between 0 and precision inclusive";
throw ParquetException(ss.str());
}
if (scale > precision) {
