From 9eb1f1d5cdc0a27710780c4d079d70cc712460f1 Mon Sep 17 00:00:00 2001 From: Matej Ferencevic Date: Wed, 6 Nov 2019 12:12:08 +0100 Subject: [PATCH] Implement `ClearProperties` for storage v2 Reviewers: teon.banek Reviewed By: teon.banek Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D2537 --- src/query/db_accessor.hpp | 8 +- src/storage/v2/edge_accessor.cpp | 21 +++++ src/storage/v2/edge_accessor.hpp | 5 + src/storage/v2/vertex_accessor.cpp | 21 +++++ src/storage/v2/vertex_accessor.hpp | 5 + tests/unit/storage_v2.cpp | 113 ++++++++++++++++++++++ tests/unit/storage_v2_edge.cpp | 147 +++++++++++++++++++++++++++++ 7 files changed, 318 insertions(+), 2 deletions(-) diff --git a/src/query/db_accessor.hpp b/src/query/db_accessor.hpp index 34b29f6fa..18b7b5c61 100644 --- a/src/query/db_accessor.hpp +++ b/src/query/db_accessor.hpp @@ -52,7 +52,9 @@ class EdgeAccessor final { } utils::BasicResult ClearProperties() { - throw utils::NotYetImplemented("ClearProperties"); + auto ret = impl_.ClearProperties(); + if (ret.HasError()) return ret.GetError(); + return {}; } VertexAccessor To() const; @@ -118,7 +120,9 @@ class VertexAccessor final { } utils::BasicResult ClearProperties() { - throw utils::NotYetImplemented("ClearProperties"); + auto ret = impl_.ClearProperties(); + if (ret.HasError()) return ret.GetError(); + return {}; } auto InEdges(storage::View view, diff --git a/src/storage/v2/edge_accessor.cpp b/src/storage/v2/edge_accessor.cpp index 7d1bc7fd8..71850db3f 100644 --- a/src/storage/v2/edge_accessor.cpp +++ b/src/storage/v2/edge_accessor.cpp @@ -55,6 +55,27 @@ Result EdgeAccessor::SetProperty(PropertyId property, return !existed; } +Result EdgeAccessor::ClearProperties() { + if (!config_.properties_on_edges) return Error::PROPERTIES_DISABLED; + + std::lock_guard guard(edge_.ptr->lock); + + if (!PrepareForWrite(transaction_, edge_.ptr)) + return Error::SERIALIZATION_ERROR; + + if (edge_.ptr->deleted) return Error::DELETED_OBJECT; + + bool removed = !edge_.ptr->properties.empty(); + for (const auto &property : edge_.ptr->properties) { + CreateAndLinkDelta(transaction_, edge_.ptr, Delta::SetPropertyTag(), + property.first, property.second); + } + + edge_.ptr->properties.clear(); + + return removed; +} + Result EdgeAccessor::GetProperty(PropertyId property, View view) const { if (!config_.properties_on_edges) return PropertyValue(); diff --git a/src/storage/v2/edge_accessor.hpp b/src/storage/v2/edge_accessor.hpp index c0cdf47b6..6be8e5af0 100644 --- a/src/storage/v2/edge_accessor.hpp +++ b/src/storage/v2/edge_accessor.hpp @@ -43,6 +43,11 @@ class EdgeAccessor final { /// @throw std::bad_alloc Result SetProperty(PropertyId property, const PropertyValue &value); + /// Remove all properties and return `true` if any removal took place. + /// `false` is returned if there were no properties to remove. + /// @throw std::bad_alloc + Result ClearProperties(); + /// @throw std::bad_alloc Result GetProperty(PropertyId property, View view) const; diff --git a/src/storage/v2/vertex_accessor.cpp b/src/storage/v2/vertex_accessor.cpp index f668262bc..b0ccf4982 100644 --- a/src/storage/v2/vertex_accessor.cpp +++ b/src/storage/v2/vertex_accessor.cpp @@ -221,6 +221,27 @@ Result VertexAccessor::SetProperty(PropertyId property, return !existed; } +Result VertexAccessor::ClearProperties() { + std::lock_guard guard(vertex_->lock); + + if (!PrepareForWrite(transaction_, vertex_)) + return Error::SERIALIZATION_ERROR; + + if (vertex_->deleted) return Error::DELETED_OBJECT; + + bool removed = !vertex_->properties.empty(); + for (const auto &property : vertex_->properties) { + CreateAndLinkDelta(transaction_, vertex_, Delta::SetPropertyTag(), + property.first, property.second); + UpdateOnSetProperty(indices_, property.first, PropertyValue(), vertex_, + *transaction_); + } + + vertex_->properties.clear(); + + return removed; +} + Result VertexAccessor::GetProperty(PropertyId property, View view) const { bool deleted = false; diff --git a/src/storage/v2/vertex_accessor.hpp b/src/storage/v2/vertex_accessor.hpp index bad76e6c0..7cce4a411 100644 --- a/src/storage/v2/vertex_accessor.hpp +++ b/src/storage/v2/vertex_accessor.hpp @@ -54,6 +54,11 @@ class VertexAccessor final { /// @throw std::bad_alloc Result SetProperty(PropertyId property, const PropertyValue &value); + /// Remove all properties and return `true` if any removal took place. + /// `false` is returned if there were no properties to remove. + /// @throw std::bad_alloc + Result ClearProperties(); + /// @throw std::bad_alloc Result GetProperty(PropertyId property, View view) const; diff --git a/tests/unit/storage_v2.cpp b/tests/unit/storage_v2.cpp index f65397151..c4ad1b0b6 100644 --- a/tests/unit/storage_v2.cpp +++ b/tests/unit/storage_v2.cpp @@ -1,9 +1,12 @@ +#include #include #include #include "storage/v2/storage.hpp" +using testing::UnorderedElementsAre; + size_t CountVertices(storage::Storage::Accessor *storage_accessor, storage::View view) { auto vertices = storage_accessor->Vertices(view); @@ -2143,3 +2146,113 @@ TEST(StorageV2, VertexLabelPropertyMixed) { ASSERT_FALSE(acc.Commit().HasError()); } + +TEST(StorageV2, VertexPropertyClear) { + storage::Storage store; + storage::Gid gid; + auto property1 = store.NameToProperty("property1"); + auto property2 = store.NameToProperty("property2"); + { + auto acc = store.Access(); + auto vertex = acc.CreateVertex(); + gid = vertex.Gid(); + + auto res = vertex.SetProperty(property1, storage::PropertyValue("value")); + ASSERT_TRUE(res.HasValue()); + ASSERT_TRUE(res.GetValue()); + + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + + ASSERT_EQ(vertex->GetProperty(property1, storage::View::OLD)->ValueString(), + "value"); + ASSERT_TRUE(vertex->GetProperty(property2, storage::View::OLD)->IsNull()); + ASSERT_THAT(vertex->Properties(storage::View::OLD).GetValue(), + UnorderedElementsAre( + std::pair(property1, storage::PropertyValue("value")))); + + { + auto ret = vertex->ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_TRUE(ret.GetValue()); + } + + ASSERT_TRUE(vertex->GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(vertex->GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(vertex->Properties(storage::View::NEW).GetValue().size(), 0); + + { + auto ret = vertex->ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_FALSE(ret.GetValue()); + } + + ASSERT_TRUE(vertex->GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(vertex->GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(vertex->Properties(storage::View::NEW).GetValue().size(), 0); + + acc.Abort(); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + + auto res = vertex->SetProperty(property2, storage::PropertyValue(42)); + ASSERT_TRUE(res.HasValue()); + ASSERT_TRUE(res.GetValue()); + + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + + ASSERT_EQ(vertex->GetProperty(property1, storage::View::OLD)->ValueString(), + "value"); + ASSERT_EQ(vertex->GetProperty(property2, storage::View::OLD)->ValueInt(), + 42); + ASSERT_THAT(vertex->Properties(storage::View::OLD).GetValue(), + UnorderedElementsAre( + std::pair(property1, storage::PropertyValue("value")), + std::pair(property2, storage::PropertyValue(42)))); + + { + auto ret = vertex->ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_TRUE(ret.GetValue()); + } + + ASSERT_TRUE(vertex->GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(vertex->GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(vertex->Properties(storage::View::NEW).GetValue().size(), 0); + + { + auto ret = vertex->ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_FALSE(ret.GetValue()); + } + + ASSERT_TRUE(vertex->GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(vertex->GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(vertex->Properties(storage::View::NEW).GetValue().size(), 0); + + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + + ASSERT_TRUE(vertex->GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(vertex->GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(vertex->Properties(storage::View::NEW).GetValue().size(), 0); + + acc.Abort(); + } +} diff --git a/tests/unit/storage_v2_edge.cpp b/tests/unit/storage_v2_edge.cpp index 82594d1b9..66fab3436 100644 --- a/tests/unit/storage_v2_edge.cpp +++ b/tests/unit/storage_v2_edge.cpp @@ -5,6 +5,8 @@ #include "storage/v2/storage.hpp" +using testing::UnorderedElementsAre; + class StorageEdgeTest : public ::testing::TestWithParam {}; INSTANTIATE_TEST_CASE_P(EdgesWithProperties, StorageEdgeTest, @@ -5108,6 +5110,124 @@ TEST(StorageWithProperties, EdgePropertySerializationError) { } } +TEST(StorageWithProperties, EdgePropertyClear) { + storage::Storage store({.items = {.properties_on_edges = true}}); + storage::Gid gid; + auto property1 = store.NameToProperty("property1"); + auto property2 = store.NameToProperty("property2"); + { + auto acc = store.Access(); + auto vertex = acc.CreateVertex(); + gid = vertex.Gid(); + auto et = acc.NameToEdgeType("et5"); + auto edge = acc.CreateEdge(&vertex, &vertex, et).GetValue(); + ASSERT_EQ(edge.EdgeType(), et); + ASSERT_EQ(edge.FromVertex(), vertex); + ASSERT_EQ(edge.ToVertex(), vertex); + + auto res = edge.SetProperty(property1, storage::PropertyValue("value")); + ASSERT_TRUE(res.HasValue()); + ASSERT_TRUE(res.GetValue()); + + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + auto edge = vertex->OutEdges({}, storage::View::NEW).GetValue()[0]; + + ASSERT_EQ(edge.GetProperty(property1, storage::View::OLD)->ValueString(), + "value"); + ASSERT_TRUE(edge.GetProperty(property2, storage::View::OLD)->IsNull()); + ASSERT_THAT(edge.Properties(storage::View::OLD).GetValue(), + UnorderedElementsAre( + std::pair(property1, storage::PropertyValue("value")))); + + { + auto ret = edge.ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_TRUE(ret.GetValue()); + } + + ASSERT_TRUE(edge.GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(edge.GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW).GetValue().size(), 0); + + { + auto ret = edge.ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_FALSE(ret.GetValue()); + } + + ASSERT_TRUE(edge.GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(edge.GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW).GetValue().size(), 0); + + acc.Abort(); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + auto edge = vertex->OutEdges({}, storage::View::NEW).GetValue()[0]; + + auto res = edge.SetProperty(property2, storage::PropertyValue(42)); + ASSERT_TRUE(res.HasValue()); + ASSERT_TRUE(res.GetValue()); + + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + auto edge = vertex->OutEdges({}, storage::View::NEW).GetValue()[0]; + + ASSERT_EQ(edge.GetProperty(property1, storage::View::OLD)->ValueString(), + "value"); + ASSERT_EQ(edge.GetProperty(property2, storage::View::OLD)->ValueInt(), 42); + ASSERT_THAT(edge.Properties(storage::View::OLD).GetValue(), + UnorderedElementsAre( + std::pair(property1, storage::PropertyValue("value")), + std::pair(property2, storage::PropertyValue(42)))); + + { + auto ret = edge.ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_TRUE(ret.GetValue()); + } + + ASSERT_TRUE(edge.GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(edge.GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW).GetValue().size(), 0); + + { + auto ret = edge.ClearProperties(); + ASSERT_TRUE(ret.HasValue()); + ASSERT_FALSE(ret.GetValue()); + } + + ASSERT_TRUE(edge.GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(edge.GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW).GetValue().size(), 0); + + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + auto edge = vertex->OutEdges({}, storage::View::NEW).GetValue()[0]; + + ASSERT_TRUE(edge.GetProperty(property1, storage::View::NEW)->IsNull()); + ASSERT_TRUE(edge.GetProperty(property2, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW).GetValue().size(), 0); + + acc.Abort(); + } +} + // NOLINTNEXTLINE(hicpp-special-member-functions) TEST(StorageWithoutProperties, EdgePropertyAbort) { storage::Storage store({.items = {.properties_on_edges = false}}); @@ -5178,3 +5298,30 @@ TEST(StorageWithoutProperties, EdgePropertyAbort) { acc.Abort(); } } + +TEST(StorageWithoutProperties, EdgePropertyClear) { + storage::Storage store({.items = {.properties_on_edges = false}}); + storage::Gid gid; + { + auto acc = store.Access(); + auto vertex = acc.CreateVertex(); + gid = vertex.Gid(); + auto et = acc.NameToEdgeType("et5"); + auto edge = acc.CreateEdge(&vertex, &vertex, et).GetValue(); + ASSERT_EQ(edge.EdgeType(), et); + ASSERT_EQ(edge.FromVertex(), vertex); + ASSERT_EQ(edge.ToVertex(), vertex); + ASSERT_FALSE(acc.Commit().HasError()); + } + { + auto acc = store.Access(); + auto vertex = acc.FindVertex(gid, storage::View::OLD); + ASSERT_TRUE(vertex); + auto edge = vertex->OutEdges({}, storage::View::NEW).GetValue()[0]; + + ASSERT_EQ(edge.ClearProperties().GetError(), + storage::Error::PROPERTIES_DISABLED); + + acc.Abort(); + } +}