ARROW-8238: [C++] Fix FieldPath type definition

FieldPath inherits from std::vector and causes link error. Refactor by replacing inheritance with composition. Closes apache#6775 from cyb70289/fieldpath Authored-by: Yibo Cai <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
plusplusjiajia · Mar 31, 2020 · f21a551 · f21a551
1 parent 23cff43
commit f21a551
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 16 deletions.
diff --git a/cpp/src/arrow/dataset/projector.cc b/cpp/src/arrow/dataset/projector.cc
@@ -48,7 +48,7 @@ Status RecordBatchProjector::SetDefaultValue(FieldRef ref,
   }
 
   ARROW_ASSIGN_OR_RAISE(auto match, ref.FindOne(*to_));
-  auto index = match[0];
+  auto index = match.indices()[0];
 
   auto field_type = to_->field(index)->type();
   if (!field_type->Equals(scalar->type)) {
@@ -101,7 +101,7 @@ Status RecordBatchProjector::SetInputSchema(std::shared_ptr<Schema> from,
       column_indices_[i] = kNoMatch;
     } else {
       RETURN_NOT_OK(ref.CheckNonMultiple(matches, *from_));
-      int matching_index = matches[0][0];
+      int matching_index = matches[0].indices()[0];
 
       if (!from_->field(matching_index)->Equals(field, /*check_metadata=*/false)) {
         return Status::TypeError("fields had matching names but were not equivalent ",

diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc
@@ -600,12 +600,12 @@ std::string NullType::ToString() const { return name(); }
 // FieldRef
 
 size_t FieldPath::hash() const {
-  return internal::ComputeStringHash<0>(data(), size() * sizeof(int));
+  return internal::ComputeStringHash<0>(indices().data(), indices().size() * sizeof(int));
 }
 
 std::string FieldPath::ToString() const {
   std::string repr = "FieldPath(";
-  for (auto index : *this) {
+  for (auto index : this->indices()) {
     repr += std::to_string(index) + " ";
   }
   repr.resize(repr.size() - 1);
@@ -643,7 +643,7 @@ struct FieldPathGetImpl {
 
     ss << "indices=[ ";
     int depth = 0;
-    for (int i : *path) {
+    for (int i : path->indices()) {
       if (depth != out_of_range_depth) {
         ss << i << " ";
         continue;
@@ -666,13 +666,13 @@ struct FieldPathGetImpl {
   template <typename T, typename GetChildren>
   static Result<T> Get(const FieldPath* path, const std::vector<T>* children,
                        GetChildren&& get_children, int* out_of_range_depth) {
-    if (path->empty()) {
+    if (path->indices().empty()) {
       return Status::Invalid("empty indices cannot be traversed");
     }
 
     int depth = 0;
     const T* out;
-    for (int index : *path) {
+    for (int index : path->indices()) {
       if (index < 0 || static_cast<size_t>(index) >= children->size()) {
         *out_of_range_depth = depth;
         return nullptr;
@@ -766,7 +766,7 @@ Result<std::shared_ptr<ChunkedArray>> FieldPath::Get(const Table& table) const {
 }
 
 FieldRef::FieldRef(FieldPath indices) : impl_(std::move(indices)) {
-  DCHECK_GT(util::get<FieldPath>(impl_).size(), 0);
+  DCHECK_GT(util::get<FieldPath>(impl_).indices().size(), 0);
 }
 
 void FieldRef::Flatten(std::vector<FieldRef> children) {
@@ -973,8 +973,9 @@ std::vector<FieldPath> FieldRef::FindAll(const FieldVector& fields) const {
         auto maybe_field = match.Get(fields);
         DCHECK_OK(maybe_field.status());
 
-        prefix.resize(prefix.size() + match.size());
-        std::copy(match.begin(), match.end(), prefix.end() - match.size());
+        prefix.indices().resize(prefix.indices().size() + match.indices().size());
+        std::copy(match.indices().begin(), match.indices().end(),
+                  prefix.indices().end() - match.indices().size());
         prefixes.push_back(std::move(prefix));
         referents.push_back(std::move(maybe_field).ValueOrDie());
       }

diff --git a/cpp/src/arrow/type.h b/cpp/src/arrow/type.h
@@ -1401,20 +1401,26 @@ class ARROW_EXPORT DictionaryUnifier {
 /// Array (returns a child array), RecordBatch (returns a column), ChunkedArray (returns a
 /// ChunkedArray where each chunk is a child array of the corresponding original chunk)
 /// and Table (returns a column).
-class ARROW_EXPORT FieldPath : public std::vector<int> {
+class ARROW_EXPORT FieldPath {
  public:
-  using std::vector<int>::vector;
-
   FieldPath() = default;
 
   FieldPath(std::vector<int> indices)  // NOLINT runtime/explicit
-      : std::vector<int>(std::move(indices)) {}
+      : indices_(std::move(indices)) {}
+
+  FieldPath(std::initializer_list<int> indices)  // NOLINT runtime/explicit
+      : indices_(std::move(indices)) {}
 
   std::string ToString() const;
 
   size_t hash() const;
 
-  explicit operator bool() const { return !empty(); }
+  explicit operator bool() const { return !indices_.empty(); }
+  bool operator==(const FieldPath& other) const { return indices() == other.indices(); }
+  bool operator!=(const FieldPath& other) const { return !(*this == other); }
+
+  std::vector<int>& indices() { return indices_; }
+  const std::vector<int>& indices() const { return indices_; }
 
   /// \brief Retrieve the referenced child Field from a Schema, Field, or DataType
   Result<std::shared_ptr<Field>> Get(const Schema& schema) const;
@@ -1429,6 +1435,9 @@ class ARROW_EXPORT FieldPath : public std::vector<int> {
   /// \brief Retrieve the referenced child Array from an Array or ChunkedArray
   Result<std::shared_ptr<Array>> Get(const Array& array) const;
   Result<std::shared_ptr<ChunkedArray>> Get(const ChunkedArray& array) const;
+
+ private:
+  std::vector<int> indices_;
 };
 
 /// \class FieldRef
@@ -1521,7 +1530,7 @@ class ARROW_EXPORT FieldRef {
   bool IsName() const { return util::holds_alternative<std::string>(impl_); }
   bool IsNested() const {
     if (IsName()) return false;
-    if (IsFieldPath()) return util::get<FieldPath>(impl_).size() > 1;
+    if (IsFieldPath()) return util::get<FieldPath>(impl_).indices().size() > 1;
     return true;
   }