From e946eb50d2115777927e4aeeb28e77cf3dfa8310 Mon Sep 17 00:00:00 2001
From: jeremy <jeremy.bailleux@memgraph.io>
Date: Tue, 29 Nov 2022 11:05:11 +0100
Subject: [PATCH] Add version ValidFramesModifier to distinguish between
 reading-only and reading+modifying

---
 src/query/v2/interpreter.cpp   |  2 +-
 src/query/v2/multiframe.cpp    | 11 ++++++
 src/query/v2/multiframe.hpp    | 71 +++++++++++++++++++++++++++++++---
 src/query/v2/plan/operator.cpp |  2 +-
 4 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/src/query/v2/interpreter.cpp b/src/query/v2/interpreter.cpp
index 78a560662..86a967de4 100644
--- a/src/query/v2/interpreter.cpp
+++ b/src/query/v2/interpreter.cpp
@@ -737,7 +737,7 @@ std::optional<plan::ProfilingStatsWithTotalTime> PullPlan::PullMultiple(AnyStrea
     return multi_frame_.HasValidFrame();
   };
 
-  const auto stream_values = [&output_symbols, &stream](Frame &frame) {
+  const auto stream_values = [&output_symbols, &stream](const Frame &frame) {
     // TODO: The streamed values should also probably use the above memory.
     std::vector<TypedValue> values;
     values.reserve(output_symbols.size());
diff --git a/src/query/v2/multiframe.cpp b/src/query/v2/multiframe.cpp
index d07a4deaf..2d0918b9c 100644
--- a/src/query/v2/multiframe.cpp
+++ b/src/query/v2/multiframe.cpp
@@ -80,6 +80,8 @@ void MultiFrame::DefragmentValidFrames() noexcept {
 
 ValidFramesReader MultiFrame::GetValidFramesReader() { return ValidFramesReader(*this); }
 
+ValidFramesModifier MultiFrame::GetValidFramesModifier() { return ValidFramesModifier(*this); }
+
 ValidFramesInvalidator MultiFrame::GetValidFramesInvalidator() { return ValidFramesInvalidator(*this); }
 
 InvalidFramesPopulator MultiFrame::GetInvalidFramesPopulator() { return InvalidFramesPopulator(*this); }
@@ -93,6 +95,15 @@ ValidFramesReader::Iterator ValidFramesReader::end() {
   return Iterator(&multiframe_.frames_[multiframe_.frames_.size()], *this);
 }
 
+ValidFramesModifier::ValidFramesModifier(MultiFrame &multiframe) : multiframe_(multiframe) {}
+
+ValidFramesModifier::~ValidFramesModifier() = default;
+
+ValidFramesModifier::Iterator ValidFramesModifier::begin() { return Iterator(&multiframe_.frames_[0], *this); }
+ValidFramesModifier::Iterator ValidFramesModifier::end() {
+  return Iterator(&multiframe_.frames_[multiframe_.frames_.size()], *this);
+}
+
 ValidFramesInvalidator::ValidFramesInvalidator(MultiFrame &multiframe) : multiframe_(multiframe) {}
 
 ValidFramesInvalidator::~ValidFramesInvalidator() {
diff --git a/src/query/v2/multiframe.hpp b/src/query/v2/multiframe.hpp
index 23d14a9c3..5b4af87e3 100644
--- a/src/query/v2/multiframe.hpp
+++ b/src/query/v2/multiframe.hpp
@@ -18,14 +18,16 @@
 namespace memgraph::query::v2 {
 constexpr unsigned long kNumberOfFramesInMultiframe = 1000;  // #NoCommit have it configurable
 
-class ValidFramesReader;
 class ValidFramesInvalidator;
+class ValidFramesModifier;
+class ValidFramesReader;
 class InvalidFramesPopulator;
 
 class MultiFrame {
  public:
-  friend class ValidFramesReader;
   friend class ValidFramesInvalidator;
+  friend class ValidFramesModifier;
+  friend class ValidFramesReader;
   friend class InvalidFramesPopulator;
 
   MultiFrame(FrameWithValidity default_frame, size_t number_of_frames, utils::MemoryResource *execution_memory);
@@ -40,16 +42,25 @@ class MultiFrame {
   Returns a object on which one can iterate in a for-loop. By doing so, you will only get frames that are in a valid
   state in the multiframe.
   Iteration goes in a deterministic order.
-  One can't modify the validity of the frame with this implementation.
+  One can't modify the validity of the frame nor its content with this implementation.
   */
   ValidFramesReader GetValidFramesReader();
 
+  /*!
+  Returns a object on which one can iterate in a for-loop. By doing so, you will only get frames that are in a valid
+  state in the multiframe.
+  Iteration goes in a deterministic order.
+  One can't modify the validity of the frame with this implementation. One can modify its content.
+  */
+  ValidFramesModifier GetValidFramesModifier();
+
   /*!
   Returns a object on which one can iterate in a for-loop. By doing so, you will only get frames that are in a valid
   state in the multiframe.
   Iteration goes in a deterministic order.
   One can modify the validity of the frame with this implementation.
-  If you do not plan to modify the validity of the frames, use GetValidFramesReader instead as this is faster.
+  If you do not plan to modify the validity of the frames, use GetValidFramesReader/GetValidFramesModifer instead as
+  this is faster.
   */
   ValidFramesInvalidator GetValidFramesInvalidator();
 
@@ -88,9 +99,9 @@ class ValidFramesReader {
   struct Iterator {
     using iterator_category = std::forward_iterator_tag;
     using difference_type = std::ptrdiff_t;
-    using value_type = Frame;
+    using value_type = const Frame;
     using pointer = value_type *;
-    using reference = Frame &;
+    using reference = const Frame &;
     using internal_ptr = FrameWithValidity *;
 
     Iterator(internal_ptr ptr, ValidFramesReader &iterator_wrapper) : ptr_(ptr), iterator_wrapper_(iterator_wrapper) {}
@@ -122,6 +133,54 @@ class ValidFramesReader {
   MultiFrame &multiframe_;
 };
 
+class ValidFramesModifier {
+ public:
+  ValidFramesModifier(MultiFrame &multiframe);
+
+  ~ValidFramesModifier();
+  ValidFramesModifier(const ValidFramesModifier &other) = delete;                 // copy constructor
+  ValidFramesModifier(ValidFramesModifier &&other) noexcept = delete;             // move constructor
+  ValidFramesModifier &operator=(const ValidFramesModifier &other) = delete;      // copy assignment
+  ValidFramesModifier &operator=(ValidFramesModifier &&other) noexcept = delete;  // move assignment
+
+  struct Iterator {
+    using iterator_category = std::forward_iterator_tag;
+    using difference_type = std::ptrdiff_t;
+    using value_type = Frame;
+    using pointer = value_type *;
+    using reference = Frame &;
+    using internal_ptr = FrameWithValidity *;
+
+    Iterator(internal_ptr ptr, ValidFramesModifier &iterator_wrapper)
+        : ptr_(ptr), iterator_wrapper_(iterator_wrapper) {}
+
+    reference operator*() const { return *ptr_; }
+    pointer operator->() { return ptr_; }
+
+    // Prefix increment
+    Iterator &operator++() {
+      do {
+        ptr_++;
+      } while (!this->ptr_->IsValid() && *this != iterator_wrapper_.end());
+
+      return *this;
+    }
+
+    friend bool operator==(const Iterator &a, const Iterator &b) { return a.ptr_ == b.ptr_; };
+    friend bool operator!=(const Iterator &a, const Iterator &b) { return a.ptr_ != b.ptr_; };
+
+   private:
+    internal_ptr ptr_;
+    ValidFramesModifier &iterator_wrapper_;
+  };
+
+  Iterator begin();
+  Iterator end();
+
+ private:
+  MultiFrame &multiframe_;
+};
+
 class ValidFramesInvalidator {
  public:
   ValidFramesInvalidator(MultiFrame &multiframe);
diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp
index 6f4747e2b..c71d70701 100644
--- a/src/query/v2/plan/operator.cpp
+++ b/src/query/v2/plan/operator.cpp
@@ -768,7 +768,7 @@ void Produce::ProduceCursor::PullMultiple(MultiFrame &multi_frame, ExecutionCont
 
   input_cursor_->PullMultiple(multi_frame, context);
 
-  auto iterator_for_valid_frame_only = multi_frame.GetValidFramesReader();
+  auto iterator_for_valid_frame_only = multi_frame.GetValidFramesModifier();
   for (auto &frame : iterator_for_valid_frame_only) {
     // Produce should always yield the latest results.
     ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context,