Optimize scanning vertices (#1227)

src/query/db_accessor.hpp

@@ -331,17 +331,17 @@ class VerticesIterable final {
         it_;
 
    public:
-    explicit Iterator(storage::VerticesIterable::Iterator it) : it_(it) {}
+    explicit Iterator(storage::VerticesIterable::Iterator it) : it_(std::move(it)) {}
     explicit Iterator(std::unordered_set<VertexAccessor, std::hash<VertexAccessor>, std::equal_to<void>,
                                          utils::Allocator<VertexAccessor>>::iterator it)
         : it_(it) {}
 
     VertexAccessor operator*() const {
-      return std::visit([](auto it_) { return VertexAccessor(*it_); }, it_);
+      return std::visit([](auto &it_) { return VertexAccessor(*it_); }, it_);
     }
 
     Iterator &operator++() {
-      std::visit([this](auto it_) { this->it_ = ++it_; }, it_);
+      std::visit([](auto &it_) { ++it_; }, it_);
       return *this;
     }
 

src/query/plan/operator.cpp

@@ -444,7 +444,7 @@ class ScanAllCursor : public Cursor {
 
     AbortCheck(context);
 
-    while (!vertices_ || vertices_it_.value() == vertices_.value().end()) {
+    while (!vertices_ || vertices_it_.value() == vertices_end_it_.value()) {
       if (!input_cursor_->Pull(frame, context)) return false;
       // We need a getter function, because in case of exhausting a lazy
       // iterable, we cannot simply reset it by calling begin().
@@ -455,6 +455,7 @@ class ScanAllCursor : public Cursor {
       // vertices _ = get_vertices_(frame, context);
       vertices_.emplace(std::move(next_vertices.value()));
       vertices_it_.emplace(vertices_.value().begin());
+      vertices_end_it_.emplace(vertices_.value().end());
     }
 #ifdef MG_ENTERPRISE
     if (license::global_license_checker.IsEnterpriseValidFast() && context.auth_checker && !FindNextVertex(context)) {
@@ -469,7 +470,7 @@ class ScanAllCursor : public Cursor {
 
 #ifdef MG_ENTERPRISE
   bool FindNextVertex(const ExecutionContext &context) {
-    while (vertices_it_.value() != vertices_.value().end()) {
+    while (vertices_it_.value() != vertices_end_it_.value()) {
       if (context.auth_checker->Has(*vertices_it_.value(), view_,
                                     memgraph::query::AuthQuery::FineGrainedPrivilege::READ)) {
         return true;
@@ -486,6 +487,7 @@ class ScanAllCursor : public Cursor {
     input_cursor_->Reset();
     vertices_ = std::nullopt;
     vertices_it_ = std::nullopt;
+    vertices_end_it_ = std::nullopt;
   }
 
  private:
@@ -495,6 +497,7 @@ class ScanAllCursor : public Cursor {
   TVerticesFun get_vertices_;
   std::optional<typename std::result_of<TVerticesFun(Frame &, ExecutionContext &)>::type::value_type> vertices_;
   std::optional<decltype(vertices_.value().begin())> vertices_it_;
+  std::optional<decltype(vertices_.value().end())> vertices_end_it_;
   const char *op_name_;
 };
 

src/storage/v2/all_vertices_iterable.cpp

@@ -17,11 +17,11 @@ auto AdvanceToVisibleVertex(utils::SkipList<Vertex>::Iterator it, utils::SkipLis
                             std::optional<VertexAccessor> *vertex, Transaction *tx, View view, Indices *indices,
                             Constraints *constraints, Config::Items config) {
   while (it != end) {
-    *vertex = VertexAccessor::Create(&*it, tx, indices, constraints, config, view);
-    if (!*vertex) {
+    if (not VertexAccessor::IsVisible(&*it, tx, view)) {
       ++it;
       continue;
     }
+    *vertex = VertexAccessor{&*it, tx, indices, constraints, config};
     break;
   }
   return it;
@@ -32,7 +32,7 @@ AllVerticesIterable::Iterator::Iterator(AllVerticesIterable *self, utils::SkipLi
       it_(AdvanceToVisibleVertex(it, self->vertices_accessor_.end(), &self->vertex_, self->transaction_, self->view_,
                                  self->indices_, self_->constraints_, self->config_)) {}
 
-VertexAccessor AllVerticesIterable::Iterator::operator*() const { return *self_->vertex_; }
+VertexAccessor const &AllVerticesIterable::Iterator::operator*() const { return *self_->vertex_; }
 
 AllVerticesIterable::Iterator &AllVerticesIterable::Iterator::operator++() {
   ++it_;

src/storage/v2/all_vertices_iterable.hpp

@@ -33,7 +33,7 @@ class AllVerticesIterable final {
    public:
     Iterator(AllVerticesIterable *self, utils::SkipList<Vertex>::Iterator it);
 
-    VertexAccessor operator*() const;
+    VertexAccessor const &operator*() const;
 
     Iterator &operator++();
 

src/storage/v2/inmemory/label_index.hpp

@@ -67,7 +67,7 @@ class InMemoryLabelIndex : public storage::LabelIndex {
      public:
       Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator);
 
-      VertexAccessor operator*() const { return current_vertex_accessor_; }
+      VertexAccessor const &operator*() const { return current_vertex_accessor_; }
 
       bool operator==(const Iterator &other) const { return index_iterator_ == other.index_iterator_; }
       bool operator!=(const Iterator &other) const { return index_iterator_ != other.index_iterator_; }

src/storage/v2/inmemory/label_property_index.hpp

@@ -74,7 +74,7 @@ class InMemoryLabelPropertyIndex : public storage::LabelPropertyIndex {
      public:
       Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator);
 
-      VertexAccessor operator*() const { return current_vertex_accessor_; }
+      VertexAccessor const &operator*() const { return current_vertex_accessor_; }
 
       bool operator==(const Iterator &other) const { return index_iterator_ == other.index_iterator_; }
       bool operator!=(const Iterator &other) const { return index_iterator_ != other.index_iterator_; }

src/storage/v2/vertex_accessor.cpp

@@ -84,6 +84,11 @@ std::optional<VertexAccessor> VertexAccessor::Create(Vertex *vertex, Transaction
   return VertexAccessor{vertex, transaction, indices, constraints, config};
 }
 
+bool VertexAccessor::IsVisible(const Vertex *vertex, const Transaction *transaction, View view) {
+  const auto [exists, deleted] = detail::IsVisible(vertex, transaction, view);
+  return exists && !deleted;
+}
+
 bool VertexAccessor::IsVisible(View view) const {
   const auto [exists, deleted] = detail::IsVisible(vertex_, transaction_, view);
   return exists && (for_deleted_ || !deleted);

src/storage/v2/vertex_accessor.hpp

@@ -45,6 +45,8 @@ class VertexAccessor final {
   static std::optional<VertexAccessor> Create(Vertex *vertex, Transaction *transaction, Indices *indices,
                                               Constraints *constraints, Config::Items config, View view);
 
+  static bool IsVisible(Vertex const *vertex, Transaction const *transaction, View view);
+
   /// @return true if the object is visible from the current transaction
   bool IsVisible(View view) const;
 

src/storage/v2/vertices_iterable.cpp

@@ -210,7 +210,7 @@ void VerticesIterable::Iterator::Destroy() noexcept {
   }
 }
 
-VertexAccessor VerticesIterable::Iterator::operator*() const {
+VertexAccessor const &VerticesIterable::Iterator::operator*() const {
   switch (type_) {
     case Type::ALL:
       return *all_it_;

src/storage/v2/vertices_iterable.hpp

@@ -63,7 +63,7 @@ class VerticesIterable final {
 
     ~Iterator();
 
-    VertexAccessor operator*() const;
+    VertexAccessor const &operator*() const;
 
     Iterator &operator++();