ARROW-6606: [C++] Add PathTree tree structure

[fsaintjacques]

Construct tree structure from std::vectorfs::FileStats, following the path directory hierarchy.

[pitrou]

The whole design looks a bit complicated. I would expect a simple DirectoryTree. Non-directories need not be tree entries (but if you want a file to point to its DirectoryTree parent, perhaps you can have a simple struct FileTreeEntry { DirectoryTree*, FileStats }).

[pitrou]

It would be better to give this a less generic name, e.g. StatsForFile.

[fsaintjacques]

The goal is to write succinct test. That's why I kept it short.

[pitrou]

Instead of computing path depth, you can probably sort by string length...

[pitrou]

You can even sort the file stats by string length in a work queue (without distinguishing directories up front) and build up the tree like that. I don't think you need to build directories first explicitly, everything will be done naturally in graph order.

[fsaintjacques]

I iterated a couple of time over the design. I initially implemented the tree with Directory only node
but it made the Visit awkward to use. Here's the current code (in another branch) that walks the PathTree to consume files.

DataFragmentIterator FileSystemBasedDataSource::GetFragments(
    std::shared_ptr<ScanOptions> options) {
  std::vector<std::unique_ptr<fs::FileStats>> files;

  auto visitor = [&files](const fs::FileStats& stats) {
    if (stats.IsFile()) {
      files.emplace_back(new fs::FileStats(stats));
    }
  };
  // The matcher ensures that directories (and their descendants) are not
  // visited.
  auto matcher = [this, options](const fs::FileStats& stats) {
    return this->PartitionMatches(stats, options);
  };

  for (auto tree : forest_) {
    tree->Visit(visitor, matcher);
  }

  auto file_it = MakeVectorIterator(std::move(files));
  auto file_to_fragment = [options, this](std::unique_ptr<fs::FileStats> stats,
                                          std::shared_ptr<DataFragment>* out) {
    std::unique_ptr<DataFragment> fragment;
    FileSource src(stats->path(), filesystem_);
    RETURN_NOT_OK(format_->MakeFragment(src, options, &fragment));
    *out = std::move(fragment);
    return Status::OK();
  };

  return MakeMaybeMapIterator(file_to_fragment, std::move(file_it));
}

[pitrou]

You could have a VisitFiles and/or a VisitFilesRecursive.

[fsaintjacques]

Do you prefer I rewrite it that way?

[pitrou]

You mean as a tree of directories? Yes.

[fsaintjacques]

@pitrou, I'd prefer that we keep it that way for the sake of simplifying consumption, creation and testing. The downside of this version is that the type allow invalid tree as per what is found in a real file system. If the user calls Make(), there can't be any "non-directory" in non-leaves nodes. We could make this class namespace "internal" so users can't call the constructor and build invalid trees. The constructor are useful for simplifying unit tests.

Minus this point, the other comments were addressed.

[pitrou]

(woops, sorry, wrong PR :-))

[pitrou]

Ok, just a couple more questions :-)

[fsaintjacques]

Comments were addressed and I also added missing test for the Visit facility.

[pitrou]

+1, thank you!

[pitrou]

Hmm, this needs to be rebased, sorry.

[fsaintjacques]

All green: https://travis-ci.org/fsaintjacques/arrow/builds/589946998