From 9ad49698e95c6b4930b85919c0bc583c99630da7 Mon Sep 17 00:00:00 2001 From: Matej Ferencevic <matej.ferencevic@memgraph.io> Date: Tue, 24 Sep 2019 16:48:36 +0200 Subject: [PATCH] Add support for disabling properties on edges Reviewers: teon.banek Reviewed By: teon.banek Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D2447 --- src/database/single_node/dump.cpp | 3 + src/memgraph_init.cpp | 2 + src/query/common.hpp | 3 + .../interpret/awesome_memgraph_functions.cpp | 4 + src/query/interpret/eval.hpp | 3 + src/query/plan/operator.cpp | 19 +++ src/storage/v2/config.hpp | 4 + src/storage/v2/delta.hpp | 11 +- src/storage/v2/edge_accessor.cpp | 51 +++--- src/storage/v2/edge_accessor.hpp | 21 ++- src/storage/v2/edge_ref.hpp | 39 +++++ src/storage/v2/indices.cpp | 21 ++- src/storage/v2/indices.hpp | 29 +++- src/storage/v2/result.hpp | 1 + src/storage/v2/storage.cpp | 139 +++++++++------- src/storage/v2/storage.hpp | 2 + src/storage/v2/vertex.hpp | 6 +- src/storage/v2/vertex_accessor.cpp | 20 ++- src/storage/v2/vertex_accessor.hpp | 13 +- tests/unit/storage_v2_edge.cpp | 155 +++++++++++++----- 20 files changed, 385 insertions(+), 161 deletions(-) create mode 100644 src/storage/v2/edge_ref.hpp diff --git a/src/database/single_node/dump.cpp b/src/database/single_node/dump.cpp index f419c906a..04567e7a3 100644 --- a/src/database/single_node/dump.cpp +++ b/src/database/single_node/dump.cpp @@ -106,6 +106,7 @@ void DumpVertex(std::ostream *os, query::DbAccessor *dba, "Trying to get labels from a deleted node."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw query::QueryRuntimeException( "Unexpected error when getting labels."); } @@ -122,6 +123,7 @@ void DumpVertex(std::ostream *os, query::DbAccessor *dba, "Trying to get properties from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw query::QueryRuntimeException( "Unexpected error when getting properties."); } @@ -150,6 +152,7 @@ void DumpEdge(std::ostream *os, query::DbAccessor *dba, "Trying to get properties from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw query::QueryRuntimeException( "Unexpected error when getting properties."); } diff --git a/src/memgraph_init.cpp b/src/memgraph_init.cpp index c3c349b5f..c2ca93523 100644 --- a/src/memgraph_init.cpp +++ b/src/memgraph_init.cpp @@ -96,6 +96,7 @@ std::map<std::string, communication::bolt::Value> BoltSession::PullAll( case storage::Error::DELETED_OBJECT: case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw communication::bolt::ClientError( "Unexpected storage error when streaming summary."); } @@ -150,6 +151,7 @@ void BoltSession::TypedValueResultStream::Result( "Returning a deleted object as a result."); case storage::Error::VERTEX_HAS_EDGES: case storage::Error::SERIALIZATION_ERROR: + case storage::Error::PROPERTIES_DISABLED: throw communication::bolt::ClientError( "Unexpected storage error when streaming results."); } diff --git a/src/query/common.hpp b/src/query/common.hpp index 668b58e87..a497bf5ba 100644 --- a/src/query/common.hpp +++ b/src/query/common.hpp @@ -85,6 +85,9 @@ void PropsSetChecked(TRecordAccessor *record, const storage::Property &key, case storage::Error::DELETED_OBJECT: throw QueryRuntimeException( "Trying to set properties on a deleted object."); + case storage::Error::PROPERTIES_DISABLED: + throw QueryRuntimeException( + "Can't set property because properties are disabled."); case storage::Error::VERTEX_HAS_EDGES: throw QueryRuntimeException( "Unexpected error when setting a property."); diff --git a/src/query/interpret/awesome_memgraph_functions.cpp b/src/query/interpret/awesome_memgraph_functions.cpp index 83ad64cc5..16a8981ea 100644 --- a/src/query/interpret/awesome_memgraph_functions.cpp +++ b/src/query/interpret/awesome_memgraph_functions.cpp @@ -374,6 +374,7 @@ TypedValue Properties(const TypedValue *args, int64_t nargs, "Trying to get properties from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when getting properties."); } @@ -438,6 +439,7 @@ size_t UnwrapDegreeResult(storage::Result<size_t> maybe_degree) { throw QueryRuntimeException("Trying to get degree of a deleted node."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when getting node degree."); } @@ -575,6 +577,7 @@ TypedValue Keys(const TypedValue *args, int64_t nargs, "Trying to get keys from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException("Unexpected error when getting keys."); } } @@ -609,6 +612,7 @@ TypedValue Labels(const TypedValue *args, int64_t nargs, "Trying to get labels from a deleted node."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException("Unexpected error when getting labels."); } } diff --git a/src/query/interpret/eval.hpp b/src/query/interpret/eval.hpp index 1475b2677..5a1551394 100644 --- a/src/query/interpret/eval.hpp +++ b/src/query/interpret/eval.hpp @@ -317,6 +317,7 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> { "Trying to access labels on a deleted node."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when accessing labels."); } @@ -554,6 +555,7 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> { "Trying to get a property from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when getting a property."); } @@ -573,6 +575,7 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> { "Trying to get a property from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when getting a property."); } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 384b48a5b..5d2391c44 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -133,6 +133,7 @@ VertexAccessor &CreateLocalVertex(const NodeCreationInfo &node_info, throw QueryRuntimeException( "Trying to set a label on a deleted node."); case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException("Unexpected error when setting a label."); } } @@ -231,6 +232,7 @@ void CreateEdge(const EdgeCreationInfo &edge_info, DbAccessor *dba, throw QueryRuntimeException( "Trying to create an edge on a deleted node."); case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException("Unexpected error when creating an edge."); } } @@ -490,6 +492,7 @@ auto UnwrapEdgesResult(storage::Result<TEdges> &&result) { "Trying to get relationships of a deleted node."); case storage::Error::VERTEX_HAS_EDGES: case storage::Error::SERIALIZATION_ERROR: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when accessing relationships."); } @@ -695,6 +698,7 @@ size_t UnwrapDegreeResult(storage::Result<size_t> maybe_degree) { throw QueryRuntimeException("Trying to get degree of a deleted node."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when getting node degree."); } @@ -1903,6 +1907,7 @@ bool Delete::DeleteCursor::Pull(Frame &frame, ExecutionContext &context) { "Can't serialize due to concurrent operations."); case storage::Error::DELETED_OBJECT: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when deleting an edge."); } @@ -1925,6 +1930,7 @@ bool Delete::DeleteCursor::Pull(Frame &frame, ExecutionContext &context) { "Can't serialize due to concurrent operations."); case storage::Error::DELETED_OBJECT: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when deleting a node."); } @@ -1939,6 +1945,7 @@ bool Delete::DeleteCursor::Pull(Frame &frame, ExecutionContext &context) { case storage::Error::VERTEX_HAS_EDGES: throw RemoveAttachedVertexException(); case storage::Error::DELETED_OBJECT: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when deleting a node."); } @@ -2063,6 +2070,9 @@ void SetPropertiesOnRecord(DbAccessor *dba, TRecordAccessor *record, case storage::Error::SERIALIZATION_ERROR: throw QueryRuntimeException( "Can't serialize due to concurrent operations."); + case storage::Error::PROPERTIES_DISABLED: + throw QueryRuntimeException( + "Can't set property because properties are disabled."); case storage::Error::VERTEX_HAS_EDGES: throw QueryRuntimeException( "Unexpected error when setting properties."); @@ -2079,6 +2089,7 @@ void SetPropertiesOnRecord(DbAccessor *dba, TRecordAccessor *record, "Trying to get properties from a deleted object."); case storage::Error::SERIALIZATION_ERROR: case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when getting properties."); } @@ -2097,6 +2108,9 @@ void SetPropertiesOnRecord(DbAccessor *dba, TRecordAccessor *record, case storage::Error::SERIALIZATION_ERROR: throw QueryRuntimeException( "Can't serialize due to concurrent operations."); + case storage::Error::PROPERTIES_DISABLED: + throw QueryRuntimeException( + "Can't set property because properties are disabled."); case storage::Error::VERTEX_HAS_EDGES: throw QueryRuntimeException( "Unexpected error when setting properties."); @@ -2205,6 +2219,7 @@ bool SetLabels::SetLabelsCursor::Pull(Frame &frame, ExecutionContext &context) { throw QueryRuntimeException( "Trying to set a label on a deleted node."); case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException("Unexpected error when setting a label."); } } @@ -2258,6 +2273,9 @@ bool RemoveProperty::RemovePropertyCursor::Pull(Frame &frame, case storage::Error::SERIALIZATION_ERROR: throw QueryRuntimeException( "Can't serialize due to concurrent operations."); + case storage::Error::PROPERTIES_DISABLED: + throw QueryRuntimeException( + "Can't remove property because properties are disabled."); case storage::Error::VERTEX_HAS_EDGES: throw QueryRuntimeException( "Unexpected error when removing property."); @@ -2330,6 +2348,7 @@ bool RemoveLabels::RemoveLabelsCursor::Pull(Frame &frame, throw QueryRuntimeException( "Trying to remove labels from a deleted node."); case storage::Error::VERTEX_HAS_EDGES: + case storage::Error::PROPERTIES_DISABLED: throw QueryRuntimeException( "Unexpected error when removing labels from a node."); } diff --git a/src/storage/v2/config.hpp b/src/storage/v2/config.hpp index 2294ecc6e..e0cecf709 100644 --- a/src/storage/v2/config.hpp +++ b/src/storage/v2/config.hpp @@ -13,6 +13,10 @@ struct Config { Type type{Type::PERIODIC}; std::chrono::milliseconds interval{std::chrono::milliseconds(1000)}; } gc; + + struct Items { + bool properties_on_edges{true}; + } items; }; } // namespace storage diff --git a/src/storage/v2/delta.hpp b/src/storage/v2/delta.hpp index 1323d6f96..402dbccdd 100644 --- a/src/storage/v2/delta.hpp +++ b/src/storage/v2/delta.hpp @@ -4,6 +4,7 @@ #include <glog/logging.h> +#include "storage/v2/edge_ref.hpp" #include "storage/v2/id_types.hpp" #include "storage/v2/property_value.hpp" @@ -150,28 +151,28 @@ struct Delta { command_id(command_id), property({key, value}) {} - Delta(AddInEdgeTag, EdgeTypeId edge_type, Vertex *vertex, Edge *edge, + Delta(AddInEdgeTag, EdgeTypeId edge_type, Vertex *vertex, EdgeRef edge, std::atomic<uint64_t> *timestamp, uint64_t command_id) : action(Action::ADD_IN_EDGE), timestamp(timestamp), command_id(command_id), vertex_edge({edge_type, vertex, edge}) {} - Delta(AddOutEdgeTag, EdgeTypeId edge_type, Vertex *vertex, Edge *edge, + Delta(AddOutEdgeTag, EdgeTypeId edge_type, Vertex *vertex, EdgeRef edge, std::atomic<uint64_t> *timestamp, uint64_t command_id) : action(Action::ADD_OUT_EDGE), timestamp(timestamp), command_id(command_id), vertex_edge({edge_type, vertex, edge}) {} - Delta(RemoveInEdgeTag, EdgeTypeId edge_type, Vertex *vertex, Edge *edge, + Delta(RemoveInEdgeTag, EdgeTypeId edge_type, Vertex *vertex, EdgeRef edge, std::atomic<uint64_t> *timestamp, uint64_t command_id) : action(Action::REMOVE_IN_EDGE), timestamp(timestamp), command_id(command_id), vertex_edge({edge_type, vertex, edge}) {} - Delta(RemoveOutEdgeTag, EdgeTypeId edge_type, Vertex *vertex, Edge *edge, + Delta(RemoveOutEdgeTag, EdgeTypeId edge_type, Vertex *vertex, EdgeRef edge, std::atomic<uint64_t> *timestamp, uint64_t command_id) : action(Action::REMOVE_OUT_EDGE), timestamp(timestamp), @@ -232,7 +233,7 @@ struct Delta { struct { EdgeTypeId edge_type; Vertex *vertex; - Edge *edge; + EdgeRef edge; } vertex_edge; }; diff --git a/src/storage/v2/edge_accessor.cpp b/src/storage/v2/edge_accessor.cpp index 04f03a5ea..7d1bc7fd8 100644 --- a/src/storage/v2/edge_accessor.cpp +++ b/src/storage/v2/edge_accessor.cpp @@ -8,45 +8,47 @@ namespace storage { VertexAccessor EdgeAccessor::FromVertex() const { - return VertexAccessor{from_vertex_, transaction_, indices_}; + return VertexAccessor{from_vertex_, transaction_, indices_, config_}; } VertexAccessor EdgeAccessor::ToVertex() const { - return VertexAccessor{to_vertex_, transaction_, indices_}; + return VertexAccessor{to_vertex_, transaction_, indices_, config_}; } Result<bool> EdgeAccessor::SetProperty(PropertyId property, const PropertyValue &value) { - std::lock_guard<utils::SpinLock> guard(edge_->lock); + if (!config_.properties_on_edges) return Error::PROPERTIES_DISABLED; - if (!PrepareForWrite(transaction_, edge_)) + std::lock_guard<utils::SpinLock> guard(edge_.ptr->lock); + + if (!PrepareForWrite(transaction_, edge_.ptr)) return Error::SERIALIZATION_ERROR; - if (edge_->deleted) return Error::DELETED_OBJECT; + if (edge_.ptr->deleted) return Error::DELETED_OBJECT; - auto it = edge_->properties.find(property); - bool existed = it != edge_->properties.end(); + auto it = edge_.ptr->properties.find(property); + bool existed = it != edge_.ptr->properties.end(); // We could skip setting the value if the previous one is the same to the new // one. This would save some memory as a delta would not be created as well as // avoid copying the value. The reason we are not doing that is because the // current code always follows the logical pattern of "create a delta" and // "modify in-place". Additionally, the created delta will make other // transactions get a SERIALIZATION_ERROR. - if (it != edge_->properties.end()) { - CreateAndLinkDelta(transaction_, edge_, Delta::SetPropertyTag(), property, - it->second); + if (it != edge_.ptr->properties.end()) { + CreateAndLinkDelta(transaction_, edge_.ptr, Delta::SetPropertyTag(), + property, it->second); if (value.IsNull()) { // remove the property - edge_->properties.erase(it); + edge_.ptr->properties.erase(it); } else { // set the value it->second = value; } } else { - CreateAndLinkDelta(transaction_, edge_, Delta::SetPropertyTag(), property, - PropertyValue()); + CreateAndLinkDelta(transaction_, edge_.ptr, Delta::SetPropertyTag(), + property, PropertyValue()); if (!value.IsNull()) { - edge_->properties.emplace(property, value); + edge_.ptr->properties.emplace(property, value); } } @@ -55,17 +57,18 @@ Result<bool> EdgeAccessor::SetProperty(PropertyId property, Result<PropertyValue> EdgeAccessor::GetProperty(PropertyId property, View view) const { + if (!config_.properties_on_edges) return PropertyValue(); bool deleted = false; PropertyValue value; Delta *delta = nullptr; { - std::lock_guard<utils::SpinLock> guard(edge_->lock); - deleted = edge_->deleted; - auto it = edge_->properties.find(property); - if (it != edge_->properties.end()) { + std::lock_guard<utils::SpinLock> guard(edge_.ptr->lock); + deleted = edge_.ptr->deleted; + auto it = edge_.ptr->properties.find(property); + if (it != edge_.ptr->properties.end()) { value = it->second; } - delta = edge_->delta; + delta = edge_.ptr->delta; } ApplyDeltasForRead(transaction_, delta, view, [&deleted, &value, property](const Delta &delta) { @@ -99,14 +102,16 @@ Result<PropertyValue> EdgeAccessor::GetProperty(PropertyId property, Result<std::map<PropertyId, PropertyValue>> EdgeAccessor::Properties( View view) const { + if (!config_.properties_on_edges) + return std::map<PropertyId, PropertyValue>{}; std::map<PropertyId, PropertyValue> properties; bool deleted = false; Delta *delta = nullptr; { - std::lock_guard<utils::SpinLock> guard(edge_->lock); - deleted = edge_->deleted; - properties = edge_->properties; - delta = edge_->delta; + std::lock_guard<utils::SpinLock> guard(edge_.ptr->lock); + deleted = edge_.ptr->deleted; + properties = edge_.ptr->properties; + delta = edge_.ptr->delta; } ApplyDeltasForRead( transaction_, delta, view, [&deleted, &properties](const Delta &delta) { diff --git a/src/storage/v2/edge_accessor.hpp b/src/storage/v2/edge_accessor.hpp index 9facf0e7d..c0cdf47b6 100644 --- a/src/storage/v2/edge_accessor.hpp +++ b/src/storage/v2/edge_accessor.hpp @@ -3,7 +3,9 @@ #include <optional> #include "storage/v2/edge.hpp" +#include "storage/v2/edge_ref.hpp" +#include "storage/v2/config.hpp" #include "storage/v2/result.hpp" #include "storage/v2/transaction.hpp" #include "storage/v2/view.hpp" @@ -19,14 +21,16 @@ class EdgeAccessor final { friend class Storage; public: - EdgeAccessor(Edge *edge, EdgeTypeId edge_type, Vertex *from_vertex, - Vertex *to_vertex, Transaction *transaction, Indices *indices) + EdgeAccessor(EdgeRef edge, EdgeTypeId edge_type, Vertex *from_vertex, + Vertex *to_vertex, Transaction *transaction, Indices *indices, + Config::Items config) : edge_(edge), edge_type_(edge_type), from_vertex_(from_vertex), to_vertex_(to_vertex), transaction_(transaction), - indices_(indices) {} + indices_(indices), + config_(config) {} VertexAccessor FromVertex() const; @@ -45,7 +49,13 @@ class EdgeAccessor final { /// @throw std::bad_alloc Result<std::map<PropertyId, PropertyValue>> Properties(View view) const; - Gid Gid() const { return edge_->gid; } + Gid Gid() const { + if (config_.properties_on_edges) { + return edge_.ptr->gid; + } else { + return edge_.gid; + } + } bool IsCycle() const { return from_vertex_ == to_vertex_; } @@ -55,12 +65,13 @@ class EdgeAccessor final { bool operator!=(const EdgeAccessor &other) const { return !(*this == other); } private: - Edge *edge_; + EdgeRef edge_; EdgeTypeId edge_type_; Vertex *from_vertex_; Vertex *to_vertex_; Transaction *transaction_; Indices *indices_; + Config::Items config_; }; } // namespace storage diff --git a/src/storage/v2/edge_ref.hpp b/src/storage/v2/edge_ref.hpp new file mode 100644 index 000000000..ea2220d27 --- /dev/null +++ b/src/storage/v2/edge_ref.hpp @@ -0,0 +1,39 @@ +#pragma once + +#include <type_traits> + +#include "storage/v2/id_types.hpp" + +namespace storage { + +// Forward declaration because we only store a pointer here. +struct Edge; + +struct EdgeRef { + explicit EdgeRef(Gid gid) : gid(gid) {} + explicit EdgeRef(Edge *ptr) : ptr(ptr) {} + + union { + Gid gid; + Edge *ptr; + }; +}; + +static_assert(sizeof(Gid) == sizeof(Edge *), + "The Gid should be the same size as an Edge *!"); +static_assert(std::is_standard_layout_v<Gid>, + "The Gid must have a standard layout!"); +static_assert(std::is_standard_layout_v<Edge *>, + "The Edge * must have a standard layout!"); +static_assert(std::is_standard_layout_v<EdgeRef>, + "The EdgeRef must have a standard layout!"); + +inline bool operator==(const EdgeRef &a, const EdgeRef &b) { + return a.gid == b.gid; +} + +inline bool operator!=(const EdgeRef &a, const EdgeRef &b) { + return a.gid != b.gid; +} + +} // namespace storage diff --git a/src/storage/v2/indices.cpp b/src/storage/v2/indices.cpp index 6a25cec64..d991e4d77 100644 --- a/src/storage/v2/indices.cpp +++ b/src/storage/v2/indices.cpp @@ -355,7 +355,7 @@ LabelIndex::Iterable::Iterator::Iterator( Iterable *self, utils::SkipList<Entry>::Iterator index_iterator) : self_(self), index_iterator_(index_iterator), - current_vertex_accessor_(nullptr, nullptr, nullptr), + current_vertex_accessor_(nullptr, nullptr, nullptr, self_->config_), current_vertex_(nullptr) { AdvanceUntilValid(); } @@ -375,7 +375,8 @@ void LabelIndex::Iterable::Iterator::AdvanceUntilValid() { self_->transaction_, self_->view_)) { current_vertex_ = index_iterator_->vertex; current_vertex_accessor_ = - VertexAccessor{current_vertex_, self_->transaction_, self_->indices_}; + VertexAccessor{current_vertex_, self_->transaction_, self_->indices_, + self_->config_}; break; } } @@ -383,12 +384,14 @@ void LabelIndex::Iterable::Iterator::AdvanceUntilValid() { LabelIndex::Iterable::Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label, View view, - Transaction *transaction, Indices *indices) + Transaction *transaction, Indices *indices, + Config::Items config) : index_accessor_(std::move(index_accessor)), label_(label), view_(view), transaction_(transaction), - indices_(indices) {} + indices_(indices), + config_(config) {} bool LabelPropertyIndex::Entry::operator<(const Entry &rhs) { if (PropertyValueLess(value, rhs.value)) { @@ -519,7 +522,7 @@ LabelPropertyIndex::Iterable::Iterator::Iterator( Iterable *self, utils::SkipList<Entry>::Iterator index_iterator) : self_(self), index_iterator_(index_iterator), - current_vertex_accessor_(nullptr, nullptr, nullptr), + current_vertex_accessor_(nullptr, nullptr, nullptr, self_->config_), current_vertex_(nullptr) { AdvanceUntilValid(); } @@ -567,7 +570,8 @@ void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() { self_->transaction_, self_->view_)) { current_vertex_ = index_iterator_->vertex; current_vertex_accessor_ = - VertexAccessor(current_vertex_, self_->transaction_, self_->indices_); + VertexAccessor(current_vertex_, self_->transaction_, self_->indices_, + self_->config_); break; } } @@ -578,7 +582,7 @@ LabelPropertyIndex::Iterable::Iterable( PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view, - Transaction *transaction, Indices *indices) + Transaction *transaction, Indices *indices, Config::Items config) : index_accessor_(std::move(index_accessor)), label_(label), property_(property), @@ -586,7 +590,8 @@ LabelPropertyIndex::Iterable::Iterable( upper_bound_(upper_bound), view_(view), transaction_(transaction), - indices_(indices) {} + indices_(indices), + config_(config) {} LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::begin() { auto index_iterator = index_accessor_.begin(); diff --git a/src/storage/v2/indices.hpp b/src/storage/v2/indices.hpp index 3b3e5cc73..52db5c884 100644 --- a/src/storage/v2/indices.hpp +++ b/src/storage/v2/indices.hpp @@ -4,6 +4,7 @@ #include <tuple> #include <utility> +#include "storage/v2/config.hpp" #include "storage/v2/property_value.hpp" #include "storage/v2/transaction.hpp" #include "storage/v2/vertex_accessor.hpp" @@ -40,7 +41,8 @@ class LabelIndex { }; public: - explicit LabelIndex(Indices *indices) : indices_(indices) {} + LabelIndex(Indices *indices, Config::Items config) + : indices_(indices), config_(config) {} /// @throw std::bad_alloc void UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx); @@ -61,7 +63,8 @@ class LabelIndex { class Iterable { public: Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label, - View view, Transaction *transaction, Indices *indices); + View view, Transaction *transaction, Indices *indices, + Config::Items config); class Iterator { public: @@ -96,6 +99,7 @@ class LabelIndex { View view_; Transaction *transaction_; Indices *indices_; + Config::Items config_; }; /// Returns an self with vertices visible from the given transaction. @@ -103,7 +107,8 @@ class LabelIndex { auto it = index_.find(label); CHECK(it != index_.end()) << "Index for label " << label.AsUint() << " doesn't exist"; - return Iterable(it->second.access(), label, view, transaction, indices_); + return Iterable(it->second.access(), label, view, transaction, indices_, + config_); } int64_t ApproximateVertexCount(LabelId label) { @@ -114,8 +119,9 @@ class LabelIndex { } private: - Indices *indices_; std::map<LabelId, utils::SkipList<Entry>> index_; + Indices *indices_; + Config::Items config_; }; class LabelPropertyIndex { @@ -133,7 +139,8 @@ class LabelPropertyIndex { }; public: - explicit LabelPropertyIndex(Indices *indices) : indices_(indices) {} + LabelPropertyIndex(Indices *indices, Config::Items config) + : indices_(indices), config_(config) {} /// @throw std::bad_alloc void UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx); @@ -166,7 +173,8 @@ class LabelPropertyIndex { PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, - View view, Transaction *transaction, Indices *indices); + View view, Transaction *transaction, Indices *indices, + Config::Items config); class Iterator { public: @@ -207,6 +215,7 @@ class LabelPropertyIndex { View view_; Transaction *transaction_; Indices *indices_; + Config::Items config_; }; /// @throw std::bad_alloc if unable to copy a PropertyValue @@ -220,7 +229,7 @@ class LabelPropertyIndex { << "Index for label " << label.AsUint() << " and property " << property.AsUint() << " doesn't exist"; return Iterable(it->second.access(), label, property, lower_bound, - upper_bound, view, transaction, indices_); + upper_bound, view, transaction, indices_, config_); } int64_t ApproximateVertexCount(LabelId label, PropertyId property) const { @@ -240,12 +249,14 @@ class LabelPropertyIndex { const std::optional<utils::Bound<PropertyValue>> &upper) const; private: - Indices *indices_; std::map<std::pair<LabelId, PropertyId>, utils::SkipList<Entry>> index_; + Indices *indices_; + Config::Items config_; }; struct Indices { - Indices() : label_index(this), label_property_index(this) {} + explicit Indices(Config::Items config) + : label_index(this, config), label_property_index(this, config) {} // Disable copy and move because members hold pointer to `this`. Indices(const Indices &) = delete; diff --git a/src/storage/v2/result.hpp b/src/storage/v2/result.hpp index 273680271..5642e719b 100644 --- a/src/storage/v2/result.hpp +++ b/src/storage/v2/result.hpp @@ -12,6 +12,7 @@ enum class Error : uint8_t { SERIALIZATION_ERROR, DELETED_OBJECT, VERTEX_HAS_EDGES, + PROPERTIES_DISABLED, }; template <class TValue> diff --git a/src/storage/v2/storage.cpp b/src/storage/v2/storage.cpp index 77f26dc9d..66764bc75 100644 --- a/src/storage/v2/storage.cpp +++ b/src/storage/v2/storage.cpp @@ -15,9 +15,10 @@ namespace storage { auto AdvanceToVisibleVertex(utils::SkipList<Vertex>::Iterator it, utils::SkipList<Vertex>::Iterator end, - Transaction *tx, View view, Indices *indices) { + Transaction *tx, View view, Indices *indices, + Config::Items config) { while (it != end) { - auto maybe_vertex = VertexAccessor::Create(&*it, tx, indices, view); + auto maybe_vertex = VertexAccessor::Create(&*it, tx, indices, config, view); if (!maybe_vertex) { ++it; continue; @@ -32,20 +33,20 @@ AllVerticesIterable::Iterator::Iterator(AllVerticesIterable *self, : self_(self), it_(AdvanceToVisibleVertex(it, self->vertices_accessor_.end(), self->transaction_, self->view_, - self->indices_)) {} + self->indices_, self->config_)) {} VertexAccessor AllVerticesIterable::Iterator::operator*() const { // TODO: current vertex accessor could be cached to avoid reconstructing every // time return *VertexAccessor::Create(&*it_, self_->transaction_, self_->indices_, - self_->view_); + self_->config_, self_->view_); } AllVerticesIterable::Iterator &AllVerticesIterable::Iterator::operator++() { ++it_; it_ = AdvanceToVisibleVertex(it_, self_->vertices_accessor_.end(), self_->transaction_, self_->view_, - self_->indices_); + self_->indices_, self_->config_); return *this; } @@ -290,7 +291,7 @@ bool VerticesIterable::Iterator::operator==(const Iterator &other) const { } } -Storage::Storage(Config config) : config_(config) { +Storage::Storage(Config config) : indices_(config.items), config_(config) { if (config_.gc.type == Config::Gc::Type::PERIODIC) { gc_runner_.Run("Storage GC", config_.gc.interval, [this] { this->CollectGarbage(); }); @@ -310,12 +311,14 @@ Storage::Accessor::Accessor(Storage *storage) // during exclusive operations. storage_guard_(storage_->main_lock_), transaction_(storage->CreateTransaction()), - is_transaction_active_(true) {} + is_transaction_active_(true), + config_(storage->config_.items) {} Storage::Accessor::Accessor(Accessor &&other) noexcept : storage_(other.storage_), transaction_(std::move(other.transaction_)), - is_transaction_active_(other.is_transaction_active_) { + is_transaction_active_(other.is_transaction_active_), + config_(other.config_) { // Don't allow the other accessor to abort our transaction in destructor. other.is_transaction_active_ = false; } @@ -334,7 +337,7 @@ VertexAccessor Storage::Accessor::CreateVertex() { CHECK(inserted) << "The vertex must be inserted here!"; CHECK(it != acc.end()) << "Invalid Vertex accessor!"; delta->prev.Set(&*it); - return VertexAccessor{&*it, &transaction_, &storage_->indices_}; + return VertexAccessor(&*it, &transaction_, &storage_->indices_, config_); } std::optional<VertexAccessor> Storage::Accessor::FindVertex(Gid gid, @@ -342,7 +345,8 @@ std::optional<VertexAccessor> Storage::Accessor::FindVertex(Gid gid, auto acc = storage_->vertices_.access(); auto it = acc.find(gid); if (it == acc.end()) return std::nullopt; - return VertexAccessor::Create(&*it, &transaction_, &storage_->indices_, view); + return VertexAccessor::Create(&*it, &transaction_, &storage_->indices_, + config_, view); } Result<bool> Storage::Accessor::DeleteVertex(VertexAccessor *vertex) { @@ -373,8 +377,8 @@ Result<bool> Storage::Accessor::DetachDeleteVertex(VertexAccessor *vertex) { "accessor when deleting a vertex!"; auto vertex_ptr = vertex->vertex_; - std::vector<std::tuple<EdgeTypeId, Vertex *, Edge *>> in_edges; - std::vector<std::tuple<EdgeTypeId, Vertex *, Edge *>> out_edges; + std::vector<std::tuple<EdgeTypeId, Vertex *, EdgeRef>> in_edges; + std::vector<std::tuple<EdgeTypeId, Vertex *, EdgeRef>> out_edges; { std::lock_guard<utils::SpinLock> guard(vertex_ptr->lock); @@ -390,8 +394,8 @@ Result<bool> Storage::Accessor::DetachDeleteVertex(VertexAccessor *vertex) { for (const auto &item : in_edges) { auto [edge_type, from_vertex, edge] = item; - EdgeAccessor e{edge, edge_type, from_vertex, - vertex_ptr, &transaction_, &storage_->indices_}; + EdgeAccessor e(edge, edge_type, from_vertex, vertex_ptr, &transaction_, + &storage_->indices_, config_); auto ret = DeleteEdge(&e); if (ret.HasError()) { CHECK(ret.GetError() == Error::SERIALIZATION_ERROR) @@ -401,8 +405,8 @@ Result<bool> Storage::Accessor::DetachDeleteVertex(VertexAccessor *vertex) { } for (const auto &item : out_edges) { auto [edge_type, to_vertex, edge] = item; - EdgeAccessor e{edge, edge_type, vertex_ptr, - to_vertex, &transaction_, &storage_->indices_}; + EdgeAccessor e(edge, edge_type, vertex_ptr, to_vertex, &transaction_, + &storage_->indices_, config_); auto ret = DeleteEdge(&e); if (ret.HasError()) { CHECK(ret.GetError() == Error::SERIALIZATION_ERROR) @@ -466,14 +470,18 @@ Result<EdgeAccessor> Storage::Accessor::CreateEdge(VertexAccessor *from, if (to_vertex->deleted) return Error::DELETED_OBJECT; } - auto gid = storage_->edge_id_.fetch_add(1, std::memory_order_acq_rel); - auto acc = storage_->edges_.access(); - auto delta = CreateDeleteObjectDelta(&transaction_); - auto [it, inserted] = acc.insert(Edge{storage::Gid::FromUint(gid), delta}); - CHECK(inserted) << "The edge must be inserted here!"; - CHECK(it != acc.end()) << "Invalid Edge accessor!"; - auto edge = &*it; - delta->prev.Set(&*it); + auto gid = storage::Gid::FromUint( + storage_->edge_id_.fetch_add(1, std::memory_order_acq_rel)); + EdgeRef edge(gid); + if (config_.properties_on_edges) { + auto acc = storage_->edges_.access(); + auto delta = CreateDeleteObjectDelta(&transaction_); + auto [it, inserted] = acc.insert(Edge(gid, delta)); + CHECK(inserted) << "The edge must be inserted here!"; + CHECK(it != acc.end()) << "Invalid Edge accessor!"; + edge = EdgeRef(&*it); + delta->prev.Set(&*it); + } CreateAndLinkDelta(&transaction_, from_vertex, Delta::RemoveOutEdgeTag(), edge_type, to_vertex, edge); @@ -483,23 +491,27 @@ Result<EdgeAccessor> Storage::Accessor::CreateEdge(VertexAccessor *from, edge_type, from_vertex, edge); to_vertex->in_edges.emplace_back(edge_type, from_vertex, edge); - return EdgeAccessor{edge, edge_type, from_vertex, - to_vertex, &transaction_, &storage_->indices_}; + return EdgeAccessor(edge, edge_type, from_vertex, to_vertex, &transaction_, + &storage_->indices_, config_); } Result<bool> Storage::Accessor::DeleteEdge(EdgeAccessor *edge) { CHECK(edge->transaction_ == &transaction_) << "EdgeAccessor must be from the same transaction as the storage " "accessor when deleting an edge!"; - auto edge_ptr = edge->edge_; + auto edge_ref = edge->edge_; auto edge_type = edge->edge_type_; - std::lock_guard<utils::SpinLock> guard(edge_ptr->lock); + std::unique_lock<utils::SpinLock> guard; + if (config_.properties_on_edges) { + auto edge_ptr = edge_ref.ptr; + guard = std::unique_lock<utils::SpinLock>(edge_ptr->lock); - if (!PrepareForWrite(&transaction_, edge_ptr)) - return Error::SERIALIZATION_ERROR; + if (!PrepareForWrite(&transaction_, edge_ptr)) + return Error::SERIALIZATION_ERROR; - if (edge_ptr->deleted) return false; + if (edge_ptr->deleted) return false; + } auto from_vertex = edge->from_vertex_; auto to_vertex = edge->to_vertex_; @@ -529,32 +541,43 @@ Result<bool> Storage::Accessor::DeleteEdge(EdgeAccessor *edge) { CHECK(!to_vertex->deleted) << "Invalid database state!"; } - CreateAndLinkDelta(&transaction_, edge_ptr, Delta::RecreateObjectTag()); - edge_ptr->deleted = true; + auto delete_edge_from_storage = [&edge_type, &edge_ref, this](auto *vertex, + auto *edges) { + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link(edge_type, vertex, edge_ref); + auto it = std::find(edges->begin(), edges->end(), link); + if (config_.properties_on_edges) { + CHECK(it != edges->end()) << "Invalid database state!"; + } else if (it == edges->end()) { + return false; + } + std::swap(*it, *edges->rbegin()); + edges->pop_back(); + return true; + }; + + auto op1 = delete_edge_from_storage(to_vertex, &from_vertex->out_edges); + auto op2 = delete_edge_from_storage(from_vertex, &to_vertex->in_edges); + + if (config_.properties_on_edges) { + CHECK((op1 && op2)) << "Invalid database state!"; + } else { + CHECK((op1 && op2) || (!op1 && !op2)) << "Invalid database state!"; + if (!op1 && !op2) { + // The edge is already deleted. + return false; + } + } + + if (config_.properties_on_edges) { + auto edge_ptr = edge_ref.ptr; + CreateAndLinkDelta(&transaction_, edge_ptr, Delta::RecreateObjectTag()); + edge_ptr->deleted = true; + } CreateAndLinkDelta(&transaction_, from_vertex, Delta::AddOutEdgeTag(), - edge_type, to_vertex, edge_ptr); - { - std::tuple<EdgeTypeId, Vertex *, Edge *> link{edge_type, to_vertex, - edge_ptr}; - auto it = std::find(from_vertex->out_edges.begin(), - from_vertex->out_edges.end(), link); - CHECK(it != from_vertex->out_edges.end()) << "Invalid database state!"; - std::swap(*it, *from_vertex->out_edges.rbegin()); - from_vertex->out_edges.pop_back(); - } - + edge_type, to_vertex, edge_ref); CreateAndLinkDelta(&transaction_, to_vertex, Delta::AddInEdgeTag(), edge_type, - from_vertex, edge_ptr); - { - std::tuple<EdgeTypeId, Vertex *, Edge *> link{edge_type, from_vertex, - edge_ptr}; - auto it = - std::find(to_vertex->in_edges.begin(), to_vertex->in_edges.end(), link); - CHECK(it != to_vertex->in_edges.end()) << "Invalid database state!"; - std::swap(*it, *to_vertex->in_edges.rbegin()); - to_vertex->in_edges.pop_back(); - } + from_vertex, edge_ref); return true; } @@ -700,7 +723,7 @@ void Storage::Accessor::Abort() { break; } case Delta::Action::ADD_IN_EDGE: { - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ current->vertex_edge.edge_type, current->vertex_edge.vertex, current->vertex_edge.edge}; auto it = std::find(vertex->in_edges.begin(), @@ -710,7 +733,7 @@ void Storage::Accessor::Abort() { break; } case Delta::Action::ADD_OUT_EDGE: { - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ current->vertex_edge.edge_type, current->vertex_edge.vertex, current->vertex_edge.edge}; auto it = std::find(vertex->out_edges.begin(), @@ -720,7 +743,7 @@ void Storage::Accessor::Abort() { break; } case Delta::Action::REMOVE_IN_EDGE: { - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ current->vertex_edge.edge_type, current->vertex_edge.vertex, current->vertex_edge.edge}; auto it = std::find(vertex->in_edges.begin(), @@ -731,7 +754,7 @@ void Storage::Accessor::Abort() { break; } case Delta::Action::REMOVE_OUT_EDGE: { - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ current->vertex_edge.edge_type, current->vertex_edge.vertex, current->vertex_edge.edge}; auto it = std::find(vertex->out_edges.begin(), diff --git a/src/storage/v2/storage.hpp b/src/storage/v2/storage.hpp index 4e2f60e1f..80fee4478 100644 --- a/src/storage/v2/storage.hpp +++ b/src/storage/v2/storage.hpp @@ -39,6 +39,7 @@ class AllVerticesIterable final { Transaction *transaction_; View view_; Indices *indices_; + Config::Items config_; public: class Iterator final { @@ -279,6 +280,7 @@ class Storage final { std::shared_lock<utils::RWLock> storage_guard_; Transaction transaction_; bool is_transaction_active_; + Config::Items config_; }; Accessor Access() { return Accessor{this}; } diff --git a/src/storage/v2/vertex.hpp b/src/storage/v2/vertex.hpp index 3c606f53f..ddb1e331e 100644 --- a/src/storage/v2/vertex.hpp +++ b/src/storage/v2/vertex.hpp @@ -8,7 +8,7 @@ #include "utils/spin_lock.hpp" #include "storage/v2/delta.hpp" -#include "storage/v2/edge.hpp" +#include "storage/v2/edge_ref.hpp" #include "storage/v2/id_types.hpp" namespace storage { @@ -24,8 +24,8 @@ struct Vertex { std::vector<LabelId> labels; std::map<PropertyId, storage::PropertyValue> properties; - std::vector<std::tuple<EdgeTypeId, Vertex *, Edge *>> in_edges; - std::vector<std::tuple<EdgeTypeId, Vertex *, Edge *>> out_edges; + std::vector<std::tuple<EdgeTypeId, Vertex *, EdgeRef>> in_edges; + std::vector<std::tuple<EdgeTypeId, Vertex *, EdgeRef>> out_edges; utils::SpinLock lock; bool deleted; diff --git a/src/storage/v2/vertex_accessor.cpp b/src/storage/v2/vertex_accessor.cpp index a516c7870..f668262bc 100644 --- a/src/storage/v2/vertex_accessor.cpp +++ b/src/storage/v2/vertex_accessor.cpp @@ -3,6 +3,7 @@ #include <memory> #include "storage/v2/edge_accessor.hpp" +#include "storage/v2/id_types.hpp" #include "storage/v2/indices.hpp" #include "storage/v2/mvcc.hpp" @@ -11,6 +12,7 @@ namespace storage { std::optional<VertexAccessor> VertexAccessor::Create(Vertex *vertex, Transaction *transaction, Indices *indices, + Config::Items config, View view) { bool is_visible = true; Delta *delta = nullptr; @@ -41,7 +43,7 @@ std::optional<VertexAccessor> VertexAccessor::Create(Vertex *vertex, } }); if (!is_visible) return std::nullopt; - return VertexAccessor{vertex, transaction, indices}; + return VertexAccessor{vertex, transaction, indices, config}; } Result<bool> VertexAccessor::AddLabel(LabelId label) { @@ -317,7 +319,7 @@ Result<std::map<PropertyId, PropertyValue>> VertexAccessor::Properties( Result<std::vector<EdgeAccessor>> VertexAccessor::InEdges( const std::vector<EdgeTypeId> &edge_types, View view) const { - std::vector<std::tuple<EdgeTypeId, Vertex *, Edge *>> in_edges; + std::vector<std::tuple<EdgeTypeId, Vertex *, EdgeRef>> in_edges; bool deleted = false; Delta *delta = nullptr; { @@ -331,7 +333,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::InEdges( switch (delta.action) { case Delta::Action::ADD_IN_EDGE: { // Add the edge because we don't see the removal. - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ delta.vertex_edge.edge_type, delta.vertex_edge.vertex, delta.vertex_edge.edge}; auto it = std::find(in_edges.begin(), in_edges.end(), link); @@ -341,7 +343,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::InEdges( } case Delta::Action::REMOVE_IN_EDGE: { // Remove the label because we don't see the addition. - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ delta.vertex_edge.edge_type, delta.vertex_edge.vertex, delta.vertex_edge.edge}; auto it = std::find(in_edges.begin(), in_edges.end(), link); @@ -376,7 +378,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::InEdges( if (edge_types.empty() || std::find(edge_types.begin(), edge_types.end(), edge_type) != edge_types.end()) { ret.emplace_back(edge, edge_type, from_vertex, vertex_, transaction_, - indices_); + indices_, config_); } } return std::move(ret); @@ -384,7 +386,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::InEdges( Result<std::vector<EdgeAccessor>> VertexAccessor::OutEdges( const std::vector<EdgeTypeId> &edge_types, View view) const { - std::vector<std::tuple<EdgeTypeId, Vertex *, Edge *>> out_edges; + std::vector<std::tuple<EdgeTypeId, Vertex *, EdgeRef>> out_edges; bool deleted = false; Delta *delta = nullptr; { @@ -398,7 +400,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::OutEdges( switch (delta.action) { case Delta::Action::ADD_OUT_EDGE: { // Add the edge because we don't see the removal. - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ delta.vertex_edge.edge_type, delta.vertex_edge.vertex, delta.vertex_edge.edge}; auto it = std::find(out_edges.begin(), out_edges.end(), link); @@ -408,7 +410,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::OutEdges( } case Delta::Action::REMOVE_OUT_EDGE: { // Remove the label because we don't see the addition. - std::tuple<EdgeTypeId, Vertex *, Edge *> link{ + std::tuple<EdgeTypeId, Vertex *, EdgeRef> link{ delta.vertex_edge.edge_type, delta.vertex_edge.vertex, delta.vertex_edge.edge}; auto it = std::find(out_edges.begin(), out_edges.end(), link); @@ -443,7 +445,7 @@ Result<std::vector<EdgeAccessor>> VertexAccessor::OutEdges( if (edge_types.empty() || std::find(edge_types.begin(), edge_types.end(), edge_type) != edge_types.end()) { ret.emplace_back(edge, edge_type, vertex_, to_vertex, transaction_, - indices_); + indices_, config_); } } return std::move(ret); diff --git a/src/storage/v2/vertex_accessor.hpp b/src/storage/v2/vertex_accessor.hpp index bce7f468a..bad76e6c0 100644 --- a/src/storage/v2/vertex_accessor.hpp +++ b/src/storage/v2/vertex_accessor.hpp @@ -4,6 +4,7 @@ #include "storage/v2/vertex.hpp" +#include "storage/v2/config.hpp" #include "storage/v2/result.hpp" #include "storage/v2/transaction.hpp" #include "storage/v2/view.hpp" @@ -19,12 +20,17 @@ class VertexAccessor final { friend class Storage; public: - VertexAccessor(Vertex *vertex, Transaction *transaction, Indices *indices) - : vertex_(vertex), transaction_(transaction), indices_(indices) {} + VertexAccessor(Vertex *vertex, Transaction *transaction, Indices *indices, + Config::Items config) + : vertex_(vertex), + transaction_(transaction), + indices_(indices), + config_(config) {} static std::optional<VertexAccessor> Create(Vertex *vertex, Transaction *transaction, - Indices *indices, View view); + Indices *indices, + Config::Items config, View view); /// Add a label and return `true` if insertion took place. /// `false` is returned if the label already existed. @@ -84,6 +90,7 @@ class VertexAccessor final { Vertex *vertex_; Transaction *transaction_; Indices *indices_; + Config::Items config_; }; } // namespace storage diff --git a/tests/unit/storage_v2_edge.cpp b/tests/unit/storage_v2_edge.cpp index 2f0a90f2b..82594d1b9 100644 --- a/tests/unit/storage_v2_edge.cpp +++ b/tests/unit/storage_v2_edge.cpp @@ -1,12 +1,20 @@ +#include <gmock/gmock.h> #include <gtest/gtest.h> #include <limits> #include "storage/v2/storage.hpp" +class StorageEdgeTest : public ::testing::TestWithParam<bool> {}; + +INSTANTIATE_TEST_CASE_P(EdgesWithProperties, StorageEdgeTest, + ::testing::Values(true)); +INSTANTIATE_TEST_CASE_P(EdgesWithoutProperties, StorageEdgeTest, + ::testing::Values(false)); + // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeCreateFromSmallerCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeCreateFromSmallerCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -173,8 +181,8 @@ TEST(StorageV2, EdgeCreateFromSmallerCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeCreateFromLargerCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeCreateFromLargerCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -341,8 +349,8 @@ TEST(StorageV2, EdgeCreateFromLargerCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeCreateFromSameCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeCreateFromSameCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_vertex = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); @@ -479,8 +487,8 @@ TEST(StorageV2, EdgeCreateFromSameCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeCreateFromSmallerAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeCreateFromSmallerAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -742,8 +750,8 @@ TEST(StorageV2, EdgeCreateFromSmallerAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeCreateFromLargerAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeCreateFromLargerAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -1005,8 +1013,8 @@ TEST(StorageV2, EdgeCreateFromLargerAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeCreateFromSameAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeCreateFromSameAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_vertex = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); @@ -1216,8 +1224,8 @@ TEST(StorageV2, EdgeCreateFromSameAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeDeleteFromSmallerCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeDeleteFromSmallerCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -1478,8 +1486,8 @@ TEST(StorageV2, EdgeDeleteFromSmallerCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeDeleteFromLargerCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeDeleteFromLargerCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -1740,8 +1748,8 @@ TEST(StorageV2, EdgeDeleteFromLargerCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeDeleteFromSameCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeDeleteFromSameCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_vertex = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); @@ -1950,8 +1958,8 @@ TEST(StorageV2, EdgeDeleteFromSameCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeDeleteFromSmallerAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeDeleteFromSmallerAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -2360,8 +2368,8 @@ TEST(StorageV2, EdgeDeleteFromSmallerAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeDeleteFromLargerAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeDeleteFromLargerAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -2771,8 +2779,8 @@ TEST(StorageV2, EdgeDeleteFromLargerAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgeDeleteFromSameAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, EdgeDeleteFromSameAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_vertex = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); @@ -3103,8 +3111,8 @@ TEST(StorageV2, EdgeDeleteFromSameAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, VertexDetachDeleteSingleCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, VertexDetachDeleteSingleCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -3247,8 +3255,8 @@ TEST(StorageV2, VertexDetachDeleteSingleCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, VertexDetachDeleteMultipleCommit) { - storage::Storage store; +TEST_P(StorageEdgeTest, VertexDetachDeleteMultipleCommit) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_vertex1 = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_vertex2 = @@ -3598,8 +3606,8 @@ TEST(StorageV2, VertexDetachDeleteMultipleCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, VertexDetachDeleteSingleAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, VertexDetachDeleteSingleAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_from = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_to = @@ -3850,8 +3858,8 @@ TEST(StorageV2, VertexDetachDeleteSingleAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, VertexDetachDeleteMultipleAbort) { - storage::Storage store; +TEST_P(StorageEdgeTest, VertexDetachDeleteMultipleAbort) { + storage::Storage store({.items = {.properties_on_edges = GetParam()}}); storage::Gid gid_vertex1 = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); storage::Gid gid_vertex2 = @@ -4545,8 +4553,8 @@ TEST(StorageV2, VertexDetachDeleteMultipleAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgePropertyCommit) { - storage::Storage store; +TEST(StorageWithProperties, EdgePropertyCommit) { + storage::Storage store({.items = {.properties_on_edges = true}}); storage::Gid gid = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); { @@ -4682,8 +4690,8 @@ TEST(StorageV2, EdgePropertyCommit) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgePropertyAbort) { - storage::Storage store; +TEST(StorageWithProperties, EdgePropertyAbort) { + storage::Storage store({.items = {.properties_on_edges = true}}); storage::Gid gid = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); @@ -4990,8 +4998,8 @@ TEST(StorageV2, EdgePropertyAbort) { } // NOLINTNEXTLINE(hicpp-special-member-functions) -TEST(StorageV2, EdgePropertySerializationError) { - storage::Storage store; +TEST(StorageWithProperties, EdgePropertySerializationError) { + storage::Storage store({.items = {.properties_on_edges = true}}); storage::Gid gid = storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); { @@ -5099,3 +5107,74 @@ TEST(StorageV2, EdgePropertySerializationError) { acc.Abort(); } } + +// NOLINTNEXTLINE(hicpp-special-member-functions) +TEST(StorageWithoutProperties, EdgePropertyAbort) { + storage::Storage store({.items = {.properties_on_edges = false}}); + storage::Gid gid = + storage::Gid::FromUint(std::numeric_limits<uint64_t>::max()); + { + 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]; + + auto property = acc.NameToProperty("property5"); + + ASSERT_TRUE(edge.GetProperty(property, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW)->size(), 0); + + { + auto res = + edge.SetProperty(property, storage::PropertyValue("temporary")); + ASSERT_TRUE(res.HasError()); + ASSERT_EQ(res.GetError(), storage::Error::PROPERTIES_DISABLED); + } + + ASSERT_TRUE(edge.GetProperty(property, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW)->size(), 0); + + { + auto res = edge.SetProperty(property, storage::PropertyValue("nandare")); + ASSERT_TRUE(res.HasError()); + ASSERT_EQ(res.GetError(), storage::Error::PROPERTIES_DISABLED); + } + + ASSERT_TRUE(edge.GetProperty(property, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW)->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 property = acc.NameToProperty("property5"); + + ASSERT_TRUE(edge.GetProperty(property, storage::View::OLD)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::OLD)->size(), 0); + + ASSERT_TRUE(edge.GetProperty(property, storage::View::NEW)->IsNull()); + ASSERT_EQ(edge.Properties(storage::View::NEW)->size(), 0); + + auto other_property = acc.NameToProperty("other"); + + ASSERT_TRUE(edge.GetProperty(other_property, storage::View::OLD)->IsNull()); + ASSERT_TRUE(edge.GetProperty(other_property, storage::View::NEW)->IsNull()); + + acc.Abort(); + } +}