From 40835b8c9c6106b3c4cbb342eb66c7c48b8ba251 Mon Sep 17 00:00:00 2001
From: gvolfing <gabor.volfinger@memgraph.io>
Date: Mon, 16 Jan 2023 14:54:20 +0100
Subject: [PATCH] General clean-up

---
 src/query/v2/multiframe.hpp                   |   2 +-
 src/query/v2/plan/operator.cpp                |  24 +---
 src/query/v2/request_router.hpp               |  37 +------
 tests/simulation/request_router.cpp           |   2 +-
 tests/simulation/test_cluster.hpp             |   2 +-
 tests/unit/high_density_shard_create_scan.cpp |   2 +-
 tests/unit/machine_manager.cpp                |   2 +-
 tests/unit/query_plan_checker.hpp             | 104 ------------------
 tests/unit/query_plan_checker_v2.hpp          |   4 -
 tests/unit/query_v2_expression_evaluator.cpp  |   5 +-
 tests/unit/query_v2_plan.cpp                  |   6 -
 11 files changed, 12 insertions(+), 178 deletions(-)

diff --git a/src/query/v2/multiframe.hpp b/src/query/v2/multiframe.hpp
index 28c972ac4..a3b1e4d7d 100644
--- a/src/query/v2/multiframe.hpp
+++ b/src/query/v2/multiframe.hpp
@@ -203,7 +203,7 @@ class ValidFramesConsumer {
   ValidFramesConsumer(const ValidFramesConsumer &other) = default;
   ValidFramesConsumer(ValidFramesConsumer &&other) noexcept = default;
   ValidFramesConsumer &operator=(const ValidFramesConsumer &other) = default;
-  ValidFramesConsumer &operator=(ValidFramesConsumer &&other) noexcept = delete;
+  ValidFramesConsumer &operator=(ValidFramesConsumer &&other) noexcept = default;
 
   struct Iterator {
     using iterator_category = std::forward_iterator_tag;
diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp
index ed0b51632..491241635 100644
--- a/src/query/v2/plan/operator.cpp
+++ b/src/query/v2/plan/operator.cpp
@@ -404,7 +404,7 @@ class DistributedScanAllAndFilterCursor : public Cursor {
       if (label_.has_value()) {
         request_label = request_router.LabelToName(*label_);
       }
-      current_batch = request_router.ScanVertices(request_label, std::nullopt);
+      current_batch = request_router.ScanVertices(request_label);
     }
     current_vertex_it = current_batch.begin();
     request_state_ = State::COMPLETED;
@@ -491,7 +491,7 @@ class DistributedScanAllByPrimaryKeyCursor : public Cursor {
       if (label_.has_value()) {
         request_label = request_router.LabelToName(*label_);
       }
-      current_batch_ = request_router.ScanVertices(request_label, std::nullopt);
+      current_batch_ = request_router.ScanVertices(request_label);
     }
     current_vertex_it_ = current_batch_.begin();
     request_state_ = State::COMPLETED;
@@ -516,29 +516,11 @@ class DistributedScanAllByPrimaryKeyCursor : public Cursor {
         pk.push_back(TypedValueToValue(primary_key->Accept(evaluator)));
       }
 
-      // Original
-      // current_batch_ = request_router.ScanVertices(request_label, pk);
-
-      auto asd_debug = request_router.ScanVertices(request_label, pk);
-
-      // VertexAccessor(Vertex v, std::vector<std::pair<PropertyId, Value>> props,
-      //                const RequestRouterInterface *request_router);
-
-      // VertexAccessor(Vertex v, std::map<PropertyId, Value> &&props, const RequestRouterInterface *request_router);
-      // VertexAccessor(Vertex v, const std::map<PropertyId, Value> &props, const RequestRouterInterface
-      // *request_router);
-
-      // struct Vertex {
-      //   VertexId id;
-      //   std::vector<Label> labels;
-      //   friend bool operator==(const Vertex &lhs, const Vertex &rhs) { return lhs.id == rhs.id; }
-      // };
-
       msgs::Label label = {.id = msgs::LabelId::FromUint(label_->AsUint())};
 
       msgs::GetPropertiesRequest req = {.vertex_ids = {std::make_pair(label, pk)}};
       auto get_prop_result = request_router.GetProperties(req);
-      // MG_ASSERT(get_prop_result.size() <= 1);
+      MG_ASSERT(get_prop_result.size() <= 1);
 
       if (get_prop_result.empty()) {
         current_batch_ = std::vector<VertexAccessor>{};
diff --git a/src/query/v2/request_router.hpp b/src/query/v2/request_router.hpp
index d83dd6928..0afee54f0 100644
--- a/src/query/v2/request_router.hpp
+++ b/src/query/v2/request_router.hpp
@@ -97,8 +97,7 @@ class RequestRouterInterface {
 
   virtual void StartTransaction() = 0;
   virtual void Commit() = 0;
-  virtual std::vector<VertexAccessor> ScanVertices(std::optional<std::string> label,
-                                                   std::optional<std::vector<msgs::Value>> primary_key) = 0;
+  virtual std::vector<VertexAccessor> ScanVertices(std::optional<std::string> label) = 0;
   virtual std::vector<msgs::CreateVerticesResponse> CreateVertices(std::vector<msgs::NewVertex> new_vertices) = 0;
   virtual std::vector<msgs::ExpandOneResultRow> ExpandOne(msgs::ExpandOneRequest request) = 0;
   virtual std::vector<msgs::CreateExpandResponse> CreateExpand(std::vector<msgs::NewExpand> new_edges) = 0;
@@ -241,15 +240,9 @@ class RequestRouter : public RequestRouterInterface {
   bool IsPrimaryLabel(storage::v3::LabelId label) const override { return shards_map_.label_spaces.contains(label); }
 
   // TODO(kostasrim) Simplify return result
-  std::vector<VertexAccessor> ScanVertices(std::optional<std::string> label,
-                                           std::optional<std::vector<msgs::Value>> primary_key) override {
+  std::vector<VertexAccessor> ScanVertices(std::optional<std::string> label) override {
     // create requests
-    std::vector<ShardRequestState<msgs::ScanVerticesRequest>> requests_to_be_sent;
-    if (primary_key) {
-      requests_to_be_sent = RequestsForScanVertexByPrimaryKey(label, *primary_key);
-    } else {
-      requests_to_be_sent = RequestsForScanVertices(label);
-    }
+    auto requests_to_be_sent = RequestsForScanVertices(label);
 
     spdlog::trace("created {} ScanVertices requests", requests_to_be_sent.size());
 
@@ -372,7 +365,6 @@ class RequestRouter : public RequestRouterInterface {
   }
 
   std::vector<msgs::GetPropertiesResultRow> GetProperties(msgs::GetPropertiesRequest requests) override {
-    // Add HLC to GetProperties request
     requests.transaction_id = transaction_id_;
     // create requests
     std::vector<ShardRequestState<msgs::GetPropertiesRequest>> requests_to_be_sent =
@@ -517,29 +509,6 @@ class RequestRouter : public RequestRouterInterface {
     return requests;
   }
 
-  std::vector<ShardRequestState<msgs::ScanVerticesRequest>> RequestsForScanVertexByPrimaryKey(
-      const std::optional<std::string> &label, const std::vector<msgs::Value> &primary_key) {
-    const auto label_id = shards_map_.GetLabelId(*label);
-    MG_ASSERT(label_id);
-    MG_ASSERT(IsPrimaryLabel(*label_id));
-    std::vector<ShardRequestState<msgs::ScanVerticesRequest>> requests = {};
-
-    auto pk_containing_shard =
-        shards_map_.GetShardForKey(*label, storage::conversions::ConvertPropertyVector(primary_key));
-
-    msgs::ScanVerticesRequest request;
-    request.transaction_id = transaction_id_;
-    request.batch_limit = 1;
-    request.start_id.second = primary_key;
-
-    ShardRequestState<msgs::ScanVerticesRequest> shard_request_state{
-        .shard = pk_containing_shard,
-        .request = std::move(request),
-    };
-    requests.emplace_back(std::move(shard_request_state));
-    return requests;
-  }
-
   std::vector<ShardRequestState<msgs::ExpandOneRequest>> RequestsForExpandOne(const msgs::ExpandOneRequest &request) {
     std::map<ShardMetadata, msgs::ExpandOneRequest> per_shard_request_table;
     msgs::ExpandOneRequest top_level_rqst_template = request;
diff --git a/tests/simulation/request_router.cpp b/tests/simulation/request_router.cpp
index 10976baa9..037674b66 100644
--- a/tests/simulation/request_router.cpp
+++ b/tests/simulation/request_router.cpp
@@ -154,7 +154,7 @@ void RunStorageRaft(Raft<IoImpl, MockedShardRsm, WriteRequests, WriteResponses,
 }
 
 void TestScanVertices(query::v2::RequestRouterInterface &request_router) {
-  auto result = request_router.ScanVertices("test_label", std::nullopt);
+  auto result = request_router.ScanVertices("test_label");
   MG_ASSERT(result.size() == 2);
   {
     auto prop = result[0].GetProperty(msgs::PropertyId::FromUint(0));
diff --git a/tests/simulation/test_cluster.hpp b/tests/simulation/test_cluster.hpp
index 699d9f8c7..791b45faa 100644
--- a/tests/simulation/test_cluster.hpp
+++ b/tests/simulation/test_cluster.hpp
@@ -182,7 +182,7 @@ void ExecuteOp(query::v2::RequestRouter<SimulatorTransport> &request_router, std
 
 void ExecuteOp(query::v2::RequestRouter<SimulatorTransport> &request_router, std::set<CompoundKey> &correctness_model,
                ScanAll scan_all) {
-  auto results = request_router.ScanVertices("test_label", std::nullopt);
+  auto results = request_router.ScanVertices("test_label");
 
   RC_ASSERT(results.size() == correctness_model.size());
 
diff --git a/tests/unit/high_density_shard_create_scan.cpp b/tests/unit/high_density_shard_create_scan.cpp
index a9647c795..4a90b98b2 100644
--- a/tests/unit/high_density_shard_create_scan.cpp
+++ b/tests/unit/high_density_shard_create_scan.cpp
@@ -192,7 +192,7 @@ void ExecuteOp(query::v2::RequestRouter<LocalTransport> &request_router, std::se
 
 void ExecuteOp(query::v2::RequestRouter<LocalTransport> &request_router, std::set<CompoundKey> &correctness_model,
                ScanAll scan_all) {
-  auto results = request_router.ScanVertices("test_label", std::nullopt);
+  auto results = request_router.ScanVertices("test_label");
 
   spdlog::error("got {} results, model size is {}", results.size(), correctness_model.size());
   EXPECT_EQ(results.size(), correctness_model.size());
diff --git a/tests/unit/machine_manager.cpp b/tests/unit/machine_manager.cpp
index 8d0670e81..6cc6a3ff1 100644
--- a/tests/unit/machine_manager.cpp
+++ b/tests/unit/machine_manager.cpp
@@ -111,7 +111,7 @@ ShardMap TestShardMap() {
 
 template <typename RequestRouter>
 void TestScanAll(RequestRouter &request_router) {
-  auto result = request_router.ScanVertices(kLabelName, std::nullopt);
+  auto result = request_router.ScanVertices(kLabelName);
   EXPECT_EQ(result.size(), 2);
 }
 
diff --git a/tests/unit/query_plan_checker.hpp b/tests/unit/query_plan_checker.hpp
index 12ea2a683..da910ff99 100644
--- a/tests/unit/query_plan_checker.hpp
+++ b/tests/unit/query_plan_checker.hpp
@@ -516,108 +516,4 @@ class FakeDbAccessor {
   std::vector<std::tuple<memgraph::storage::LabelId, memgraph::storage::PropertyId, int64_t>> label_property_index_;
 };
 
-// class FakeDistributedDbAccessor {
-//  public:
-//   int64_t VerticesCount(memgraph::storage::v3::LabelId label) const {
-//     auto found = label_index_.find(label);
-//     if (found != label_index_.end()) return found->second;
-//     return 0;
-//   }
-
-//   int64_t VerticesCount(memgraph::storage::v3::LabelId label, memgraph::storage::PropertyId property) const {
-//     for (auto &index : label_property_index_) {
-//       if (std::get<0>(index) == label && std::get<1>(index) == property) {
-//         return std::get<2>(index);
-//       }
-//     }
-//     return 0;
-//   }
-
-//   bool LabelIndexExists(memgraph::storage::v3::LabelId label) const {
-//     return label_index_.find(label) != label_index_.end();
-//   }
-
-//   bool LabelPropertyIndexExists(memgraph::storage::v3::LabelId label, memgraph::storage::PropertyId property) const {
-//     for (auto &index : label_property_index_) {
-//       if (std::get<0>(index) == label && std::get<1>(index) == property) {
-//         return true;
-//       }
-//     }
-//     return false;
-//   }
-
-//   void SetIndexCount(memgraph::storage::v3::LabelId label, int64_t count) { label_index_[label] = count; }
-
-//   void SetIndexCount(memgraph::storage::v3::LabelId label, memgraph::storage::PropertyId property, int64_t count) {
-//     for (auto &index : label_property_index_) {
-//       if (std::get<0>(index) == label && std::get<1>(index) == property) {
-//         std::get<2>(index) = count;
-//         return;
-//       }
-//     }
-//     label_property_index_.emplace_back(label, property, count);
-//   }
-
-//   memgraph::storage::v3::LabelId NameToLabel(const std::string &name) {
-//     auto found = primary_labels_.find(name);
-//     if (found != primary_labels_.end()) return found->second;
-//     return primary_labels_.emplace(name,
-//     memgraph::storage::v3::LabelId::FromUint(primary_labels_.size())).first->second;
-//   }
-
-//   memgraph::storage::v3::LabelId Label(const std::string &name) { return NameToLabel(name); }
-
-//   memgraph::storage::EdgeTypeId NameToEdgeType(const std::string &name) {
-//     auto found = edge_types_.find(name);
-//     if (found != edge_types_.end()) return found->second;
-//     return edge_types_.emplace(name, memgraph::storage::EdgeTypeId::FromUint(edge_types_.size())).first->second;
-//   }
-
-//   memgraph::storage::PropertyId NameToPrimaryProperty(const std::string &name) {
-//     auto found = primary_properties_.find(name);
-//     if (found != primary_properties_.end()) return found->second;
-//     return primary_properties_.emplace(name,
-//     memgraph::storage::PropertyId::FromUint(primary_properties_.size())).first->second;
-//   }
-
-//   memgraph::storage::PropertyId NameToSecondaryProperty(const std::string &name) {
-//     auto found = secondary_properties_.find(name);
-//     if (found != secondary_properties_.end()) return found->second;
-//     return secondary_properties_.emplace(name,
-//     memgraph::storage::PropertyId::FromUint(secondary_properties_.size())).first->second;
-//   }
-
-//   memgraph::storage::PropertyId PrimaryProperty(const std::string &name) { return NameToPrimaryProperty(name); }
-//   memgraph::storage::PropertyId SecondaryProperty(const std::string &name) { return NameToSecondaryProperty(name); }
-
-//   std::string PrimaryPropertyToName(memgraph::storage::PropertyId property) const {
-//     for (const auto &kv : primary_properties_) {
-//       if (kv.second == property) return kv.first;
-//     }
-//     LOG_FATAL("Unable to find primary property name");
-//   }
-
-//   std::string SecondaryPropertyToName(memgraph::storage::PropertyId property) const {
-//     for (const auto &kv : secondary_properties_) {
-//       if (kv.second == property) return kv.first;
-//     }
-//     LOG_FATAL("Unable to find secondary property name");
-//   }
-
-//   std::string PrimaryPropertyName(memgraph::storage::PropertyId property) const { return
-//   PrimaryPropertyToName(property); } std::string SecondaryPropertyName(memgraph::storage::PropertyId property) const
-//   { return SecondaryPropertyToName(property); }
-
-//  private:
-//   std::unordered_map<std::string, memgraph::storage::v3::LabelId> primary_labels_;
-//   std::unordered_map<std::string, memgraph::storage::v3::LabelId> secondary_labels_;
-//   std::unordered_map<std::string, memgraph::storage::EdgeTypeId> edge_types_;
-//   std::unordered_map<std::string, memgraph::storage::PropertyId> primary_properties_;
-//   std::unordered_map<std::string, memgraph::storage::PropertyId> secondary_properties_;
-
-//   std::unordered_map<memgraph::storage::v3::LabelId, int64_t> label_index_;
-//   std::vector<std::tuple<memgraph::storage::v3::LabelId, memgraph::storage::PropertyId, int64_t>>
-//   label_property_index_;
-// };
-
 }  // namespace memgraph::query::plan
diff --git a/tests/unit/query_plan_checker_v2.hpp b/tests/unit/query_plan_checker_v2.hpp
index 85c625d59..1a971c4f0 100644
--- a/tests/unit/query_plan_checker_v2.hpp
+++ b/tests/unit/query_plan_checker_v2.hpp
@@ -181,10 +181,6 @@ class ExpectScanAllByPrimaryKey : public OpChecker<v2::plan::ScanAllByPrimaryKey
 
   void ExpectOp(v2::plan::ScanAllByPrimaryKey &scan_all, const SymbolTable &) override {
     EXPECT_EQ(scan_all.label_, label_);
-    // EXPECT_EQ(scan_all.property_, property_);
-
-    // TODO(gvolfing) maybe assert the size of the 2 vectors.
-    // TODO(gvolfing) maybe use some std alg if Expression lets us.
 
     bool primary_property_match = true;
     for (const auto &expected_prop : properties_) {
diff --git a/tests/unit/query_v2_expression_evaluator.cpp b/tests/unit/query_v2_expression_evaluator.cpp
index 098a94981..ffbebb4ba 100644
--- a/tests/unit/query_v2_expression_evaluator.cpp
+++ b/tests/unit/query_v2_expression_evaluator.cpp
@@ -84,10 +84,7 @@ class MockedRequestRouter : public RequestRouterInterface {
   }
   void StartTransaction() override {}
   void Commit() override {}
-  std::vector<VertexAccessor> ScanVertices(std::optional<std::string> /* label */,
-                                           std::optional<std::vector<msgs::Value>> /*primary_key*/) override {
-    return {};
-  }
+  std::vector<VertexAccessor> ScanVertices(std::optional<std::string> /* label */) override { return {}; }
 
   std::vector<CreateVerticesResponse> CreateVertices(
       std::vector<memgraph::msgs::NewVertex> /* new_vertices */) override {
diff --git a/tests/unit/query_v2_plan.cpp b/tests/unit/query_v2_plan.cpp
index 56f90a690..215b6b8a3 100644
--- a/tests/unit/query_v2_plan.cpp
+++ b/tests/unit/query_v2_plan.cpp
@@ -86,12 +86,6 @@ class TestPlanner : public ::testing::Test {};
 
 using PlannerTypes = ::testing::Types<Planner>;
 
-// void DeleteListContent(std::list<BaseOpChecker *> *list) {
-//   for (BaseOpChecker *ptr : *list) {
-//     delete ptr;
-//   }
-// }
-
 TYPED_TEST_CASE(TestPlanner, PlannerTypes);
 
 TYPED_TEST(TestPlanner, MatchFilterPropIsNotNull) {