[arrow] branch master updated: ARROW-4255: [C++] Eagerly initialize name_to_index_ to avoid race

[ap...@apache.org]

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

apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new d0e4a08  ARROW-4255: [C++] Eagerly initialize name_to_index_ to avoid race
d0e4a08 is described below

commit d0e4a080ce3cf13a9fecbf934288280400afd79c
Author: Gene Novark <30...@users.noreply.github.com>
AuthorDate: Wed Jan 23 13:43:55 2019 +0100

    ARROW-4255: [C++] Eagerly initialize name_to_index_ to avoid race
    
    Author: Gene Novark <30...@users.noreply.github.com>
    
    Closes #3408 from gnpdt/ARROW-4255 and squashes the following commits:
    
    5ae50cdb <Gene Novark> Deduplicate code, fix linter error
    216bcd7d <Gene Novark> Eagerly initialize name_to_index_ to avoid race
---
 cpp/src/arrow/type.cc | 38 ++++++++++++++++++++++++--------------
 cpp/src/arrow/type.h  | 11 +++--------
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc
index cd57e2d..f240664 100644
--- a/cpp/src/arrow/type.cc
+++ b/cpp/src/arrow/type.cc
@@ -219,6 +219,24 @@ std::string UnionType::ToString() const {
 // ----------------------------------------------------------------------
 // Struct type
 
+namespace {
+
+std::unordered_map<std::string, int> CreateNameToIndexMap(
+    const std::vector<std::shared_ptr<Field>>& fields) {
+  std::unordered_map<std::string, int> name_to_index;
+  for (size_t i = 0; i < fields.size(); ++i) {
+    name_to_index[fields[i]->name()] = static_cast<int>(i);
+  }
+  return name_to_index;
+}
+
+}  // namespace
+
+StructType::StructType(const std::vector<std::shared_ptr<Field>>& fields)
+    : NestedType(Type::STRUCT), name_to_index_(CreateNameToIndexMap(fields)) {
+  children_ = fields;
+}
+
 std::string StructType::ToString() const {
   std::stringstream s;
   s << "struct<";
@@ -239,12 +257,6 @@ std::shared_ptr<Field> StructType::GetFieldByName(const std::string& name) const
 }
 
 int StructType::GetFieldIndex(const std::string& name) const {
-  if (children_.size() > 0 && name_to_index_.size() == 0) {
-    for (size_t i = 0; i < children_.size(); ++i) {
-      name_to_index_[children_[i]->name()] = static_cast<int>(i);
-    }
-  }
-
   if (name_to_index_.size() < children_.size()) {
     // There are duplicate field names. Refuse to guess
     int counts = 0;
@@ -318,11 +330,15 @@ std::string NullType::ToString() const { return name(); }
 
 Schema::Schema(const std::vector<std::shared_ptr<Field>>& fields,
                const std::shared_ptr<const KeyValueMetadata>& metadata)
-    : fields_(fields), metadata_(metadata) {}
+    : fields_(fields),
+      name_to_index_(CreateNameToIndexMap(fields_)),
+      metadata_(metadata) {}
 
 Schema::Schema(std::vector<std::shared_ptr<Field>>&& fields,
                const std::shared_ptr<const KeyValueMetadata>& metadata)
-    : fields_(std::move(fields)), metadata_(metadata) {}
+    : fields_(std::move(fields)),
+      name_to_index_(CreateNameToIndexMap(fields_)),
+      metadata_(metadata) {}
 
 bool Schema::Equals(const Schema& other, bool check_metadata) const {
   if (this == &other) {
@@ -357,12 +373,6 @@ std::shared_ptr<Field> Schema::GetFieldByName(const std::string& name) const {
 }
 
 int64_t Schema::GetFieldIndex(const std::string& name) const {
-  if (fields_.size() > 0 && name_to_index_.size() == 0) {
-    for (size_t i = 0; i < fields_.size(); ++i) {
-      name_to_index_[fields_[i]->name()] = static_cast<int>(i);
-    }
-  }
-
   auto it = name_to_index_.find(name);
   if (it == name_to_index_.end()) {
     return -1;
diff --git a/cpp/src/arrow/type.h b/cpp/src/arrow/type.h
index 6c3643c..752fc85 100644
--- a/cpp/src/arrow/type.h
+++ b/cpp/src/arrow/type.h
@@ -506,10 +506,7 @@ class ARROW_EXPORT StructType : public NestedType {
  public:
   static constexpr Type::type type_id = Type::STRUCT;
 
-  explicit StructType(const std::vector<std::shared_ptr<Field>>& fields)
-      : NestedType(Type::STRUCT) {
-    children_ = fields;
-  }
+  explicit StructType(const std::vector<std::shared_ptr<Field>>& fields);
 
   Status Accept(TypeVisitor* visitor) const override;
   std::string ToString() const override;
@@ -529,8 +526,7 @@ class ARROW_EXPORT StructType : public NestedType {
   int GetChildIndex(const std::string& name) const;
 
  private:
-  /// Lazily initialized mapping
-  mutable std::unordered_map<std::string, int> name_to_index_;
+  std::unordered_map<std::string, int> name_to_index_;
 };
 
 /// \brief Base type class for (fixed-size) decimal data
@@ -866,8 +862,7 @@ class ARROW_EXPORT Schema {
  private:
   std::vector<std::shared_ptr<Field>> fields_;
 
-  /// Lazily initialized mapping
-  mutable std::unordered_map<std::string, int> name_to_index_;
+  std::unordered_map<std::string, int> name_to_index_;
 
   std::shared_ptr<const KeyValueMetadata> metadata_;
 };