This is an automated email from the ASF dual-hosted git repository.

mdeepak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/parquet-cpp.git


The following commit(s) were added to refs/heads/master by this push:
     new 0875e43  PARQUET-1265: Segfault on static ApplicationVersion 
initialization
0875e43 is described below

commit 0875e43010af485e1c0b506d77d7e0edc80c66cc
Author: Deepak Majeti <deepak.maj...@hpe.com>
AuthorDate: Thu Apr 12 17:09:37 2018 -0400

    PARQUET-1265: Segfault on static ApplicationVersion initialization
    
    Author: Deepak Majeti <deepak.maj...@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 |  2 +-
 src/parquet/metadata.cc    | 28 ++++++++++++++++++++--------
 src/parquet/metadata.h     |  7 ++++---
 3 files changed, 25 insertions(+), 12 deletions(-)

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 @@ class SerializedRowGroup : public RowGroupReader::Contents {
 
     // 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.
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 @@ std::shared_ptr<const KeyValueMetadata> 
FileMetaData::key_value_metadata() const
 
 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::VersionEq(const 
ApplicationVersion& other_version) cons
 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 @@ bool ApplicationVersion::HasCorrectStatistics(Type::type 
col_type,
   }
 
   // PARQUET-251
-  if (VersionLt(PARQUET_251_FIXED_VERSION)) {
+  if (VersionLt(PARQUET_251_FIXED_VERSION())) {
     return false;
   }
 
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 @@ using KeyValueMetadata = ::arrow::KeyValueMetadata;
 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 @@ class ApplicationVersion {
 
   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;

-- 
To stop receiving notification emails like this one, please contact
mdee...@apache.org.

Reply via email to