commit 0875e43010af485e1c0b506d77d7e0edc80c66cc
author Deepak Majeti <deepak.majeti@hpe.com> Thu Apr 12 17:09:37 2018 -0400
committer Deepak Majeti <deepak.majeti@hpe.com> Thu Apr 12 17:09:37 2018 -0400
tree d41deb33a1aab7551da8215592c576da282eecb1
parent 4e7ef12dccb250074370376dc31a4963e1010447

PARQUET-1265: Segfault on static ApplicationVersion initialization

Author: Deepak Majeti <deepak.majeti@hpe.com>

Closes #452 from majetideepak/PARQUET-1265 and squashes the following commits:

2972197 [Deepak Majeti] fix initialization error
ab07a47 [Deepak Majeti] Simplify construction of static variables
b23d3b3 [Deepak Majeti] PARQUET-1265: Segfault on static ApplicationVersion initialization

src/parquet/file_reader.cc

diff --git a/src/parquet/file_reader.cc b/src/parquet/file_reader.cc
index e3280c6..983d2d0 100644
--- a/src/parquet/file_reader.cc
+++ b/src/parquet/file_reader.cc
@@ -112,7 +112,7 @@
 
     // PARQUET-816 workaround for old files created by older parquet-mr
     const ApplicationVersion& version = file_metadata_->writer_version();
-    if (version.VersionLt(ApplicationVersion::PARQUET_816_FIXED_VERSION)) {
+    if (version.VersionLt(ApplicationVersion::PARQUET_816_FIXED_VERSION())) {
       // The Parquet MR writer had a bug in 1.2.8 and below where it didn't include the
       // dictionary page header size in total_compressed_size and total_uncompressed_size
       // (see IMPALA-694). We add padding to compensate.

src/parquet/metadata.cc

diff --git a/src/parquet/metadata.cc b/src/parquet/metadata.cc
index 91304cf..fc420b0 100644
--- a/src/parquet/metadata.cc
+++ b/src/parquet/metadata.cc
@@ -31,12 +31,20 @@
 
 namespace parquet {
 
-const ApplicationVersion ApplicationVersion::PARQUET_251_FIXED_VERSION =
-    ApplicationVersion("parquet-mr version 1.8.0");
-const ApplicationVersion ApplicationVersion::PARQUET_816_FIXED_VERSION =
-    ApplicationVersion("parquet-mr version 1.2.9");
-const ApplicationVersion ApplicationVersion::PARQUET_CPP_FIXED_STATS_VERSION =
-    ApplicationVersion("parquet-cpp version 1.3.0");
+const ApplicationVersion& ApplicationVersion::PARQUET_251_FIXED_VERSION() {
+  static ApplicationVersion version("parquet-mr", 1, 8, 0);
+  return version;
+}
+
+const ApplicationVersion& ApplicationVersion::PARQUET_816_FIXED_VERSION() {
+  static ApplicationVersion version("parquet-mr", 1, 2, 9);
+  return version;
+}
+
+const ApplicationVersion& ApplicationVersion::PARQUET_CPP_FIXED_STATS_VERSION() {
+  static ApplicationVersion version("parquet-cpp", 1, 3, 0);
+  return version;
+}
 
 template <typename DType>
 static std::shared_ptr<RowGroupStatistics> MakeTypedColumnStats(
@@ -448,6 +456,10 @@
 
 void FileMetaData::WriteTo(OutputStream* dst) { return impl_->WriteTo(dst); }
 
+ApplicationVersion::ApplicationVersion(const std::string& application, int major,
+                                       int minor, int patch)
+    : application_(application), version{major, minor, patch, "", "", ""} {}
+
 ApplicationVersion::ApplicationVersion(const std::string& created_by) {
   boost::regex app_regex{ApplicationVersion::APPLICATION_FORMAT};
   boost::regex ver_regex{ApplicationVersion::VERSION_FORMAT};
@@ -511,7 +523,7 @@
 bool ApplicationVersion::HasCorrectStatistics(Type::type col_type,
                                               SortOrder::type sort_order) const {
   // Parquet cpp version 1.3.0 onwards stats are computed correctly for all types
-  if ((application_ != "parquet-cpp") || (VersionLt(PARQUET_CPP_FIXED_STATS_VERSION))) {
+  if ((application_ != "parquet-cpp") || (VersionLt(PARQUET_CPP_FIXED_STATS_VERSION()))) {
     // Only SIGNED are valid
     if (SortOrder::SIGNED != sort_order) {
       return false;
@@ -534,7 +546,7 @@
   }
 
   // PARQUET-251
-  if (VersionLt(PARQUET_251_FIXED_VERSION)) {
+  if (VersionLt(PARQUET_251_FIXED_VERSION())) {
     return false;
   }
 

src/parquet/metadata.h

diff --git a/src/parquet/metadata.h b/src/parquet/metadata.h
index 7850358..3ba57ac 100644
--- a/src/parquet/metadata.h
+++ b/src/parquet/metadata.h
@@ -38,9 +38,9 @@
 class ApplicationVersion {
  public:
   // Known Versions with Issues
-  static const ApplicationVersion PARQUET_251_FIXED_VERSION;
-  static const ApplicationVersion PARQUET_816_FIXED_VERSION;
-  static const ApplicationVersion PARQUET_CPP_FIXED_STATS_VERSION;
+  static const ApplicationVersion& PARQUET_251_FIXED_VERSION();
+  static const ApplicationVersion& PARQUET_816_FIXED_VERSION();
+  static const ApplicationVersion& PARQUET_CPP_FIXED_STATS_VERSION();
   // Regular expression for the version format
   // major . minor . patch unknown - prerelease.x + build info
   // Eg: 1.5.0ab-cdh5.5.0+cd
@@ -74,6 +74,7 @@
 
   ApplicationVersion() {}
   explicit ApplicationVersion(const std::string& created_by);
+  ApplicationVersion(const std::string& application, int major, int minor, int patch);
 
   // Returns true if version is strictly less than other_version
   bool VersionLt(const ApplicationVersion& other_version) const;