From 1f2a15e7c89be8b70c235c64c2bb4a595abb02b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bruno=20Sa=C4=8Dari=C4=87?= <bruno.sacaric@gmail.com>
Date: Fri, 23 Dec 2022 14:47:12 +0100
Subject: [PATCH] Fix MATCH not allowed on replica (#709)

---
 src/query/plan/read_write_type_checker.cpp    | 19 +++++++++----
 src/query/plan/read_write_type_checker.hpp    |  3 +-
 .../unit/query_plan_read_write_typecheck.cpp  | 28 +++++++++++++++++++
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/src/query/plan/read_write_type_checker.cpp b/src/query/plan/read_write_type_checker.cpp
index 94252eb3f..851465c56 100644
--- a/src/query/plan/read_write_type_checker.cpp
+++ b/src/query/plan/read_write_type_checker.cpp
@@ -91,18 +91,25 @@ bool ReadWriteTypeChecker::PreVisit([[maybe_unused]] Foreach &op) {
 bool ReadWriteTypeChecker::Visit(Once &) { return false; }  // NOLINT(hicpp-named-parameter)
 
 void ReadWriteTypeChecker::UpdateType(RWType op_type) {
-  // Update type only if it's not the NONE type and the current operator's type
-  // is different than the one that's currently inferred.
-  if (type != RWType::NONE && type != op_type) {
-    type = RWType::RW;
-  }
   // Stop inference because RW is the most "dominant" type, i.e. it isn't
   // affected by the type of nodes in the plan appearing after the node for
   // which the type is set to RW.
   if (type == RWType::RW) {
     return;
   }
-  if (type == RWType::NONE && op_type != RWType::NONE) {
+
+  // if op_type is NONE, type doesn't change.
+  if (op_type == RWType::NONE) {
+    return;
+  }
+
+  // Update type only if it's not the NONE type and the current operator's type
+  // is different than the one that's currently inferred.
+  if (type != RWType::NONE && type != op_type) {
+    type = RWType::RW;
+  }
+
+  if (type == RWType::NONE) {
     type = op_type;
   }
 }
diff --git a/src/query/plan/read_write_type_checker.hpp b/src/query/plan/read_write_type_checker.hpp
index 42d9569b0..34222c52c 100644
--- a/src/query/plan/read_write_type_checker.hpp
+++ b/src/query/plan/read_write_type_checker.hpp
@@ -15,7 +15,7 @@
 
 namespace memgraph::query::plan {
 
-class ReadWriteTypeChecker : public virtual HierarchicalLogicalOperatorVisitor {
+struct ReadWriteTypeChecker : public virtual HierarchicalLogicalOperatorVisitor {
  public:
   ReadWriteTypeChecker() = default;
 
@@ -89,7 +89,6 @@ class ReadWriteTypeChecker : public virtual HierarchicalLogicalOperatorVisitor {
 
   bool Visit(Once &) override;
 
- private:
   void UpdateType(RWType op_type);
 };
 
diff --git a/tests/unit/query_plan_read_write_typecheck.cpp b/tests/unit/query_plan_read_write_typecheck.cpp
index 673620ff9..7f84e9b16 100644
--- a/tests/unit/query_plan_read_write_typecheck.cpp
+++ b/tests/unit/query_plan_read_write_typecheck.cpp
@@ -253,3 +253,31 @@ TEST_F(ReadWriteTypeCheckTest, Foreach) {
   std::shared_ptr<LogicalOperator> foreach = std::make_shared<plan::Foreach>(nullptr, nullptr, nullptr, x);
   CheckPlanType(foreach.get(), RWType::RW);
 }
+
+TEST_F(ReadWriteTypeCheckTest, CheckUpdateType) {
+  std::array<std::array<RWType, 3>, 16> scenarios = {{
+      {RWType::NONE, RWType::NONE, RWType::NONE},
+      {RWType::NONE, RWType::R, RWType::R},
+      {RWType::NONE, RWType::W, RWType::W},
+      {RWType::NONE, RWType::RW, RWType::RW},
+      {RWType::R, RWType::NONE, RWType::R},
+      {RWType::R, RWType::R, RWType::R},
+      {RWType::R, RWType::W, RWType::RW},
+      {RWType::R, RWType::RW, RWType::RW},
+      {RWType::W, RWType::NONE, RWType::W},
+      {RWType::W, RWType::R, RWType::RW},
+      {RWType::W, RWType::W, RWType::W},
+      {RWType::W, RWType::RW, RWType::RW},
+      {RWType::RW, RWType::NONE, RWType::RW},
+      {RWType::RW, RWType::R, RWType::RW},
+      {RWType::RW, RWType::W, RWType::RW},
+      {RWType::RW, RWType::RW, RWType::RW},
+  }};
+
+  auto rw_type_checker = ReadWriteTypeChecker();
+  for (auto scenario : scenarios) {
+    rw_type_checker.type = scenario[0];
+    rw_type_checker.UpdateType(scenario[1]);
+    EXPECT_EQ(scenario[2], rw_type_checker.type);
+  }
+}