Skip to content

Commit

Permalink
PARQUET-828: Do not implicitly cast ParquetVersion enum to int
Browse files Browse the repository at this point in the history
See https://github.com/apache/parquet-mr/blob/df9d8e415436292ae33e1ca0b8da256640de9710/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileWriter.java#L86, this number should be 1 for Parquet 1.0 files, I believe.

Author: Wes McKinney <wes.mckinney@twosigma.com>

Closes apache#216 from wesm/PARQUET-828 and squashes the following commits:

ab6773c [Wes McKinney] Do not implicitly cast ParquetVersion enum to int. Set 1.0 to 1, 2.0 to 2
  • Loading branch information
wesm committed Sep 2, 2018
1 parent 0daa871 commit c92a229
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 10 deletions.
15 changes: 9 additions & 6 deletions cpp/src/parquet/file/file-metadata-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ TEST(Metadata, TestBuildAccess) {
parquet::schema::NodePtr root;
parquet::SchemaDescriptor schema;

std::shared_ptr<WriterProperties> props = WriterProperties::Builder().build();
WriterProperties::Builder prop_builder;

std::shared_ptr<WriterProperties> props =
prop_builder.version(ParquetVersion::PARQUET_2_0)->build();

fields.push_back(parquet::schema::Int32("int_col", Repetition::REQUIRED));
fields.push_back(parquet::schema::Float("float_col", Repetition::REQUIRED));
Expand Down Expand Up @@ -84,7 +87,7 @@ TEST(Metadata, TestBuildAccess) {
ASSERT_EQ(nrows, f_accessor->num_rows());
ASSERT_LE(0, f_accessor->size());
ASSERT_EQ(2, f_accessor->num_row_groups());
ASSERT_EQ(DEFAULT_WRITER_VERSION, f_accessor->version());
ASSERT_EQ(ParquetVersion::PARQUET_2_0, f_accessor->version());
ASSERT_EQ(DEFAULT_CREATED_BY, f_accessor->created_by());
ASSERT_EQ(3, f_accessor->num_schema_elements());

Expand All @@ -110,8 +113,8 @@ TEST(Metadata, TestBuildAccess) {
ASSERT_EQ(DEFAULT_COMPRESSION_TYPE, rg1_column2->compression());
ASSERT_EQ(nrows / 2, rg1_column1->num_values());
ASSERT_EQ(nrows / 2, rg1_column2->num_values());
ASSERT_EQ(2, rg1_column1->encodings().size());
ASSERT_EQ(2, rg1_column2->encodings().size());
ASSERT_EQ(3, rg1_column1->encodings().size());
ASSERT_EQ(3, rg1_column2->encodings().size());
ASSERT_EQ(512, rg1_column1->total_compressed_size());
ASSERT_EQ(512, rg1_column2->total_compressed_size());
ASSERT_EQ(600, rg1_column1->total_uncompressed_size());
Expand Down Expand Up @@ -142,8 +145,8 @@ TEST(Metadata, TestBuildAccess) {
ASSERT_EQ(nrows / 2, rg2_column2->num_values());
ASSERT_EQ(DEFAULT_COMPRESSION_TYPE, rg2_column1->compression());
ASSERT_EQ(DEFAULT_COMPRESSION_TYPE, rg2_column2->compression());
ASSERT_EQ(2, rg2_column1->encodings().size());
ASSERT_EQ(2, rg2_column2->encodings().size());
ASSERT_EQ(3, rg2_column1->encodings().size());
ASSERT_EQ(3, rg2_column2->encodings().size());
ASSERT_EQ(512, rg2_column1->total_compressed_size());
ASSERT_EQ(512, rg2_column2->total_compressed_size());
ASSERT_EQ(600, rg2_column1->total_uncompressed_size());
Expand Down
25 changes: 22 additions & 3 deletions cpp/src/parquet/file/metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,17 @@ int FileMetaData::num_row_groups() const {
return impl_->num_row_groups();
}

int32_t FileMetaData::version() const {
return impl_->version();
ParquetVersion::type FileMetaData::version() const {
switch (impl_->version()) {
case 1:
return ParquetVersion::PARQUET_1_0;
case 2:
return ParquetVersion::PARQUET_2_0;
default:
// Improperly set version, assuming Parquet 1.0
break;
}
return ParquetVersion::PARQUET_1_0;
}

const FileMetaData::Version& FileMetaData::writer_version() const {
Expand Down Expand Up @@ -656,7 +665,17 @@ class FileMetaDataBuilder::FileMetaDataBuilderImpl {
}
metadata_->__set_num_rows(total_rows);
metadata_->__set_row_groups(row_groups);
metadata_->__set_version(properties_->version());

int32_t file_version = 0;
switch (properties_->version()) {
case ParquetVersion::PARQUET_1_0:
file_version = 1;
case ParquetVersion::PARQUET_2_0:
file_version = 2;
default:
break;
}
metadata_->__set_version(file_version);
metadata_->__set_created_by(properties_->created_by());
parquet::schema::SchemaFlattener flattener(
static_cast<parquet::schema::GroupNode*>(schema_->schema_root().get()),
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/parquet/file/metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class PARQUET_EXPORT FileMetaData {
int num_columns() const;
int64_t num_rows() const;
int num_row_groups() const;
int32_t version() const;
ParquetVersion::type version() const;
const std::string& created_by() const;
int num_schema_elements() const;
std::unique_ptr<RowGroupMetaData> RowGroup(int i) const;
Expand Down

0 comments on commit c92a229

Please sign in to comment.