fix bug on for_deleted

src/storage/v2/disk/storage.cpp

@@ -883,11 +883,12 @@ Result<std::optional<VertexAccessor>> DiskStorage::DiskAccessor::DeleteVertex(Ve
 
   CreateAndLinkDelta(&transaction_, vertex_ptr, Delta::RecreateObjectTag());
   vertex_ptr->deleted = true;
+  vertex_ptr->for_deleted_ = true;
   vertices_to_delete_.emplace_back(utils::SerializeIdType(vertex_ptr->gid), utils::SerializeVertex(*vertex_ptr));
   transaction_.manyDeltasCache.Invalidate(vertex_ptr);
 
   return std::make_optional<VertexAccessor>(vertex_ptr, &transaction_, &storage_->indices_, &storage_->constraints_,
-                                            config_, true);
+                                            config_);
 }
 
 Result<std::optional<std::pair<VertexAccessor, std::vector<EdgeAccessor>>>>
@@ -934,11 +935,12 @@ DiskStorage::DiskAccessor::DetachDeleteVertex(VertexAccessor *vertex) {
 
   CreateAndLinkDelta(&transaction_, vertex_ptr, Delta::RecreateObjectTag());
   vertex_ptr->deleted = true;
+  vertex_ptr->for_deleted_ = true;
   vertices_to_delete_.emplace_back(utils::SerializeIdType(vertex_ptr->gid), utils::SerializeVertex(*vertex_ptr));
   transaction_.manyDeltasCache.Invalidate(vertex_ptr);
 
   return std::make_optional<ReturnType>(
-      VertexAccessor{vertex_ptr, &transaction_, &storage_->indices_, &storage_->constraints_, config_, true},
+      VertexAccessor{vertex_ptr, &transaction_, &storage_->indices_, &storage_->constraints_, config_},
       std::move(deleted_edges));
 }
 
@@ -1125,6 +1127,7 @@ Result<std::optional<EdgeAccessor>> DiskStorage::DiskAccessor::DeleteEdge(EdgeAc
     auto *edge_ptr = edge_ref.ptr;
     CreateAndLinkDelta(&transaction_, edge_ptr, Delta::RecreateObjectTag());
     edge_ptr->deleted = true;
+    edge_ptr->for_deleted_ = true;
   }
 
   CreateAndLinkDelta(&transaction_, from_vertex, Delta::AddOutEdgeTag(), edge_type, to_vertex, edge_ref);
@@ -1137,7 +1140,7 @@ Result<std::optional<EdgeAccessor>> DiskStorage::DiskAccessor::DeleteEdge(EdgeAc
   storage_->edge_count_.fetch_add(-1, std::memory_order_acq_rel);
 
   return std::make_optional<EdgeAccessor>(edge_ref, edge_type, from_vertex, to_vertex, &transaction_,
-                                          &storage_->indices_, &storage_->constraints_, config_, true);
+                                          &storage_->indices_, &storage_->constraints_, config_);
 }
 
 /// TODO: at which storage naming

src/storage/v2/edge.hpp

@@ -39,6 +39,14 @@ struct Edge {
   // uint8_t PAD;
   // uint16_t PAD;
 
+  // if the accessor was created for a deleted edge.
+  // Accessor behaves differently for some methods based on this
+  // flag.
+  // E.g. If this field is set to true, GetProperty will return the property of the edge
+  // even though the edge is deleted.
+  // All the write operations will still return an error if it's called for a deleted edge.
+  bool for_deleted_{false};
+
   Delta *delta;
 };
 

src/storage/v2/edge_accessor.cpp

@@ -64,7 +64,7 @@ bool EdgeAccessor::IsVisible(const View view) const {
         }
       }
     });
-    return exists && (for_deleted_ || !deleted);
+    return exists && (edge_.ptr->for_deleted_ || !deleted);
   }
 
   Delta *delta = nullptr;
@@ -94,7 +94,7 @@ bool EdgeAccessor::IsVisible(const View view) const {
       }
     }
   });
-  return exists && (for_deleted_ || !deleted);
+  return exists && (edge_.ptr->for_deleted_ || !deleted);
 }
 
 VertexAccessor EdgeAccessor::FromVertex() const {
@@ -225,7 +225,7 @@ Result<PropertyValue> EdgeAccessor::GetProperty(PropertyId property, View view)
     }
   });
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!edge_.ptr->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return std::move(value);
 }
 
@@ -277,7 +277,7 @@ Result<std::map<PropertyId, PropertyValue>> EdgeAccessor::Properties(View view)
     }
   });
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!edge_.ptr->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return std::move(properties);
 }
 

src/storage/v2/edge_accessor.hpp

@@ -34,7 +34,7 @@ class EdgeAccessor final {
 
  public:
   EdgeAccessor(EdgeRef edge, EdgeTypeId edge_type, Vertex *from_vertex, Vertex *to_vertex, Transaction *transaction,
-               Indices *indices, Constraints *constraints, Config::Items config, bool for_deleted = false)
+               Indices *indices, Constraints *constraints, Config::Items config)
       : edge_(edge),
         edge_type_(edge_type),
         from_vertex_(from_vertex),
@@ -42,8 +42,7 @@ class EdgeAccessor final {
         transaction_(transaction),
         indices_(indices),
         constraints_(constraints),
-        config_(config),
-        for_deleted_(for_deleted) {}
+        config_(config) {}
 
   /// @return true if the object is visible from the current transaction
   bool IsVisible(View view) const;
@@ -99,14 +98,6 @@ class EdgeAccessor final {
   Indices *indices_;
   Constraints *constraints_;
   Config::Items config_;
-
-  // if the accessor was created for a deleted edge.
-  // Accessor behaves differently for some methods based on this
-  // flag.
-  // E.g. If this field is set to true, GetProperty will return the property of the edge
-  // even though the edge is deleted.
-  // All the write operations will still return an error if it's called for a deleted edge.
-  bool for_deleted_{false};
 };
 
 }  // namespace memgraph::storage

src/storage/v2/inmemory/storage.cpp

@@ -268,6 +268,7 @@ Result<std::optional<VertexAccessor>> InMemoryStorage::InMemoryAccessor::DeleteV
 
   CreateAndLinkDelta(&transaction_, vertex_ptr, Delta::RecreateObjectTag());
   vertex_ptr->deleted = true;
+  vertex_ptr->for_deleted_ = true;
   transaction_.manyDeltasCache.Invalidate(vertex_ptr);
 
   // Need to inform the next CollectGarbage call that there are some
@@ -278,7 +279,7 @@ Result<std::optional<VertexAccessor>> InMemoryStorage::InMemoryAccessor::DeleteV
   }
 
   return std::make_optional<VertexAccessor>(vertex_ptr, &transaction_, &storage_->indices_, &storage_->constraints_,
-                                            config_, true);
+                                            config_);
 }
 
 Result<std::optional<std::pair<VertexAccessor, std::vector<EdgeAccessor>>>>
@@ -346,6 +347,7 @@ InMemoryStorage::InMemoryAccessor::DetachDeleteVertex(VertexAccessor *vertex) {
 
   CreateAndLinkDelta(&transaction_, vertex_ptr, Delta::RecreateObjectTag());
   vertex_ptr->deleted = true;
+  vertex_ptr->for_deleted_ = true;
   transaction_.manyDeltasCache.Invalidate(vertex_ptr);
 
   // Need to inform the next CollectGarbage call that there are some
@@ -356,7 +358,7 @@ InMemoryStorage::InMemoryAccessor::DetachDeleteVertex(VertexAccessor *vertex) {
   }
 
   return std::make_optional<ReturnType>(
-      VertexAccessor{vertex_ptr, &transaction_, &storage_->indices_, &storage_->constraints_, config_, true},
+      VertexAccessor{vertex_ptr, &transaction_, &storage_->indices_, &storage_->constraints_, config_},
       std::move(deleted_edges));
 }
 
@@ -573,6 +575,7 @@ Result<std::optional<EdgeAccessor>> InMemoryStorage::InMemoryAccessor::DeleteEdg
     auto *edge_ptr = edge_ref.ptr;
     CreateAndLinkDelta(&transaction_, edge_ptr, Delta::RecreateObjectTag());
     edge_ptr->deleted = true;
+    edge_ptr->for_deleted_ = true;
 
     // Need to inform the next CollectGarbage call that there are some
     // non-transactional deletions that need to be collected
@@ -592,7 +595,7 @@ Result<std::optional<EdgeAccessor>> InMemoryStorage::InMemoryAccessor::DeleteEdg
   storage_->edge_count_.fetch_add(-1, std::memory_order_acq_rel);
 
   return std::make_optional<EdgeAccessor>(edge_ref, edge_type, from_vertex, to_vertex, &transaction_,
-                                          &storage_->indices_, &storage_->constraints_, config_, true);
+                                          &storage_->indices_, &storage_->constraints_, config_);
 }
 
 // NOLINTNEXTLINE(google-default-arguments)

src/storage/v2/vertex.hpp

@@ -43,6 +43,17 @@ struct Vertex {
   // uint8_t PAD;
   // uint16_t PAD;
 
+  // if the accessor was created for a deleted vertex.
+  // Accessor behaves differently for some methods based on this
+  // flag.
+  // E.g. If this field is set to true, GetProperty will return the property of the node
+  // even though the node is deleted.
+  // All the write operations, and operators used for traversal (e.g. InEdges) will still
+  // return an error if it's called for a deleted vertex.
+  // Needs to be on this level because accessing deleted vertex for which we don't know that was deleted
+  // from deleted edge will throw an error
+  bool for_deleted_{false};
+
   Delta *delta;
 };
 

src/storage/v2/vertex_accessor.cpp

@@ -85,7 +85,7 @@ std::optional<VertexAccessor> VertexAccessor::Create(Vertex *vertex, Transaction
 
 bool VertexAccessor::IsVisible(View view) const {
   const auto [exists, deleted] = detail::IsVisible(vertex_, transaction_, view);
-  return exists && (for_deleted_ || !deleted);
+  return exists && (vertex_->for_deleted_ || !deleted);
 }
 
 Result<bool> VertexAccessor::AddLabel(LabelId label) {
@@ -149,7 +149,7 @@ Result<bool> VertexAccessor::HasLabel(LabelId label, View view) const {
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resLabel = cache.GetHasLabel(view, vertex_, label); resLabel) return {resLabel.value()};
     }
 
@@ -172,7 +172,7 @@ Result<bool> VertexAccessor::HasLabel(LabelId label, View view) const {
   }
 
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!vertex_->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return has_label;
 }
 
@@ -196,7 +196,7 @@ Result<std::vector<LabelId>> VertexAccessor::Labels(View view) const {
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resLabels = cache.GetLabels(view, vertex_); resLabels) return {*resLabels};
     }
 
@@ -219,7 +219,7 @@ Result<std::vector<LabelId>> VertexAccessor::Labels(View view) const {
   }
 
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!vertex_->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return std::move(labels);
 }
 
@@ -325,7 +325,7 @@ Result<PropertyValue> VertexAccessor::GetProperty(PropertyId property, View view
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resProperty = cache.GetProperty(view, vertex_, property); resProperty) return {*resProperty};
     }
 
@@ -349,7 +349,7 @@ Result<PropertyValue> VertexAccessor::GetProperty(PropertyId property, View view
   }
 
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!vertex_->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return std::move(value);
 }
 
@@ -373,7 +373,7 @@ Result<std::map<PropertyId, PropertyValue>> VertexAccessor::Properties(View view
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resProperties = cache.GetProperties(view, vertex_); resProperties) return {*resProperties};
     }
 
@@ -397,7 +397,7 @@ Result<std::map<PropertyId, PropertyValue>> VertexAccessor::Properties(View view
   }
 
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!vertex_->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return std::move(properties);
 }
 
@@ -448,7 +448,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const std::
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resInEdges = cache.GetInEdges(view, vertex_, destination_vertex, edge_types); resInEdges)
         return {build_result(*resInEdges)};
     }
@@ -523,7 +523,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const std:
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resOutEdges = cache.GetOutEdges(view, vertex_, dst_vertex, edge_types); resOutEdges)
         return {build_result(*resOutEdges)};
     }
@@ -572,7 +572,7 @@ Result<size_t> VertexAccessor::InDegree(View view) const {
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resInDegree = cache.GetInDegree(view, vertex_); resInDegree) return {*resInDegree};
     }
 
@@ -596,7 +596,7 @@ Result<size_t> VertexAccessor::InDegree(View view) const {
   }
 
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!vertex_->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return degree;
 }
 
@@ -620,7 +620,7 @@ Result<size_t> VertexAccessor::OutDegree(View view) const {
     auto const useCache = transaction_->isolation_level == IsolationLevel::SNAPSHOT_ISOLATION;
     if (useCache) {
       auto const &cache = transaction_->manyDeltasCache;
-      if (auto resError = HasError(view, cache, vertex_, for_deleted_); resError) return *resError;
+      if (auto resError = HasError(view, cache, vertex_, vertex_->for_deleted_); resError) return *resError;
       if (auto resOutDegree = cache.GetOutDegree(view, vertex_); resOutDegree) return {*resOutDegree};
     }
 
@@ -644,7 +644,7 @@ Result<size_t> VertexAccessor::OutDegree(View view) const {
   }
 
   if (!exists) return Error::NONEXISTENT_OBJECT;
-  if (!for_deleted_ && deleted) return Error::DELETED_OBJECT;
+  if (!vertex_->for_deleted_ && deleted) return Error::DELETED_OBJECT;
   return degree;
 }
 

src/storage/v2/vertex_accessor.hpp

@@ -33,13 +33,8 @@ class VertexAccessor final {
 
  public:
   VertexAccessor(Vertex *vertex, Transaction *transaction, Indices *indices, Constraints *constraints,
-                 Config::Items config, bool for_deleted = false)
-      : vertex_(vertex),
-        transaction_(transaction),
-        indices_(indices),
-        constraints_(constraints),
-        config_(config),
-        for_deleted_(for_deleted) {}
+                 Config::Items config)
+      : vertex_(vertex), transaction_(transaction), indices_(indices), constraints_(constraints), config_(config) {}
 
   static std::optional<VertexAccessor> Create(Vertex *vertex, Transaction *transaction, Indices *indices,
                                               Constraints *constraints, Config::Items config, View view);
@@ -114,15 +109,6 @@ class VertexAccessor final {
   Indices *indices_;
   Constraints *constraints_;
   Config::Items config_;
-
-  // if the accessor was created for a deleted vertex.
-  // Accessor behaves differently for some methods based on this
-  // flag.
-  // E.g. If this field is set to true, GetProperty will return the property of the node
-  // even though the node is deleted.
-  // All the write operations, and operators used for traversal (e.g. InEdges) will still
-  // return an error if it's called for a deleted vertex.
-  bool for_deleted_{false};
 };
 
 }  // namespace memgraph::storage