From a14c4f1864dac4c164d6e84e17158e6eca13a3c2 Mon Sep 17 00:00:00 2001 From: Vinko Kasljevic <vinko.kasljevic@memgraph.io> Date: Fri, 15 Feb 2019 10:59:20 +0100 Subject: [PATCH] Existence constraints for vertex for single node Summary: Existence constraint ensures that all nodes with certain label have a certain property. `ExistenceRule` defines label -> properties rule and `ExistenceConstraints` manages all constraints. Reviewers: msantl, ipaljak, teon.banek, mferencevic Reviewed By: msantl, teon.banek, mferencevic Subscribers: mferencevic, pullbot Differential Revision: https://phabricator.memgraph.io/D1797 --- src/CMakeLists.txt | 1 + src/database/single_node/graph_db.hpp | 3 +- .../single_node/graph_db_accessor.cpp | 57 ++++++- .../single_node/graph_db_accessor.hpp | 27 ++++ .../constraints/existence_constraints.cpp | 53 +++++++ .../constraints/existence_constraints.hpp | 76 ++++++++++ src/storage/single_node/record_accessor.cpp | 3 +- src/storage/single_node/storage.hpp | 4 + tests/unit/CMakeLists.txt | 3 + tests/unit/existence_constraints.cpp | 140 ++++++++++++++++++ 10 files changed, 362 insertions(+), 5 deletions(-) create mode 100644 src/storage/single_node/constraints/existence_constraints.cpp create mode 100644 src/storage/single_node/constraints/existence_constraints.hpp create mode 100644 tests/unit/existence_constraints.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f7b14922d..b8a32b69c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -50,6 +50,7 @@ set(mg_single_node_sources storage/common/types/property_value_store.cpp storage/single_node/record_accessor.cpp storage/single_node/vertex_accessor.cpp + storage/single_node/constraints/existence_constraints.cpp transactions/single_node/engine.cpp memgraph_init.cpp ) diff --git a/src/database/single_node/graph_db.hpp b/src/database/single_node/graph_db.hpp index 6c46c9fdd..727026dc0 100644 --- a/src/database/single_node/graph_db.hpp +++ b/src/database/single_node/graph_db.hpp @@ -94,7 +94,8 @@ class GraphDb { /// Create a new accessor by starting a new transaction. std::unique_ptr<GraphDbAccessor> Access(); std::unique_ptr<GraphDbAccessor> AccessBlocking( - std::experimental::optional<tx::TransactionId> parent_tx); + std::experimental::optional<tx::TransactionId> parent_tx = + std::experimental::nullopt); /// Create an accessor for a running transaction. std::unique_ptr<GraphDbAccessor> Access(tx::TransactionId); diff --git a/src/database/single_node/graph_db_accessor.cpp b/src/database/single_node/graph_db_accessor.cpp index 2f68526cd..ae3579607 100644 --- a/src/database/single_node/graph_db_accessor.cpp +++ b/src/database/single_node/graph_db_accessor.cpp @@ -184,7 +184,7 @@ void GraphDbAccessor::DeleteIndex(storage::Label label, void GraphDbAccessor::UpdateLabelIndices(storage::Label label, const VertexAccessor &vertex_accessor, - const Vertex *const vertex) { + const Vertex *vertex) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; auto *vlist_ptr = vertex_accessor.address(); @@ -194,19 +194,70 @@ void GraphDbAccessor::UpdateLabelIndices(storage::Label label, "Node couldn't be updated due to index constraint violation!"); } db_.storage().labels_index_.Update(label, vlist_ptr, vertex); + + if (!db_.storage().existence_constraints_.CheckIfSatisfies(vertex)) { + throw IndexConstraintViolationException( + "Node couldn't be updated due to existence constraint violation!"); + } } void GraphDbAccessor::UpdatePropertyIndex( storage::Property property, const RecordAccessor<Vertex> &vertex_accessor, - const Vertex *const vertex) { + const Vertex *vertex) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; if (!db_.storage().label_property_index_.UpdateOnProperty( property, vertex_accessor.address(), vertex)) { throw IndexConstraintViolationException( - "Node couldn't be updated due to index constraint violation!"); + "Node couldn't be updated due to existence constraint violation!"); } } +void GraphDbAccessor::UpdateOnPropertyRemove(storage::Property property, + const Vertex *vertex, + const RecordAccessor<Vertex> &accessor) { + if (!db_.storage().existence_constraints_.CheckIfSatisfies(vertex)) { + throw IndexConstraintViolationException( + "Node couldn't be updated due to existence constraint violation!"); + } +} + +void GraphDbAccessor::BuildExistenceConstraint( + storage::Label label, const std::vector<storage::Property> &properties) { + auto dba = + db_.AccessBlocking(std::experimental::make_optional(transaction().id_)); + ExistenceRule rule{label, properties}; + + for (auto v : dba->Vertices(false)) { + if (!CheckIfSatisfiesExistenceRule(v.GetOld(), rule)) { + throw IndexConstraintViolationException( + "Node couldn't be updated due to existence constraint violation!"); + } + } + + db_.storage().existence_constraints_.AddConstraint(rule); + // TODO + //wal().Emplace(database::StateDelta::BuildExistenceConstraint(); + dba->Commit(); +} + +void GraphDbAccessor::DeleteExistenceConstraint( + storage::Label label, const std::vector<storage::Property> &properties) { + auto dba = + db_.AccessBlocking(std::experimental::make_optional(transaction().id_)); + ExistenceRule rule{label, properties}; + db_.storage().existence_constraints_.RemoveConstraint(rule); + // TODO + //wal().Emplace(database::StateDelta::DeleteExistenceConstraint(); + dba->Commit(); +} + +bool GraphDbAccessor::ExistenceConstraintExists( + storage::Label label, + const std::vector<storage::Property> &properties) const { + ExistenceRule rule{label, properties}; + return db_.storage().existence_constraints_.Exists(rule); +} + int64_t GraphDbAccessor::VerticesCount() const { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; return db_.storage().vertices_.access().size(); diff --git a/src/database/single_node/graph_db_accessor.hpp b/src/database/single_node/graph_db_accessor.hpp index ab92b7638..0aa55da4a 100644 --- a/src/database/single_node/graph_db_accessor.hpp +++ b/src/database/single_node/graph_db_accessor.hpp @@ -469,6 +469,26 @@ class GraphDbAccessor { return db_.storage().label_property_index_.Keys(); } + /** + * Creates new existence constraint. + */ + void BuildExistenceConstraint( + storage::Label label, const std::vector<storage::Property> &properties); + + /** + * Deletes existing existence constraint. + */ + void DeleteExistenceConstraint( + storage::Label label, const std::vector<storage::Property> &properties); + + /** + * Checks whether constraint exists. + */ + bool ExistenceConstraintExists( + storage::Label label, + const std::vector<storage::Property> &properties) const; + + /** * Return approximate number of all vertices in the database. * Note that this is always an over-estimate and never an under-estimate. @@ -639,6 +659,13 @@ class GraphDbAccessor { bool commited_{false}; bool aborted_{false}; + /** + * Notifies storage about change. + */ + void UpdateOnPropertyRemove(storage::Property property, + const Vertex *vertex, + const RecordAccessor<Vertex> &accessor); + /** * Insert this vertex into corresponding any label + 'property' index. * @param property - vertex will be inserted into indexes which contain this diff --git a/src/storage/single_node/constraints/existence_constraints.cpp b/src/storage/single_node/constraints/existence_constraints.cpp new file mode 100644 index 000000000..88d423ff9 --- /dev/null +++ b/src/storage/single_node/constraints/existence_constraints.cpp @@ -0,0 +1,53 @@ +#include "storage/single_node/constraints/existence_constraints.hpp" + +namespace database { +bool Contains(const PropertyValueStore &store, + const std::vector<storage::Property> &properties) { + for (auto &property : properties) { + if (store.at(property).IsNull()) { + return false; + } + } + + return true; +} + +bool CheckIfSatisfiesExistenceRule(const Vertex *vertex, + const database::ExistenceRule &rule) { + if (!utils::Contains(vertex->labels_, rule.label)) return true; + if (!Contains(vertex->properties_, rule.properties)) return false; + + return true; +} + +void ExistenceConstraints::AddConstraint(const ExistenceRule &rule) { + auto found = std::find(constraints_.begin(), constraints_.end(), rule); + if (found != constraints_.end()) return; + + constraints_.push_back(rule); +} + +void ExistenceConstraints::RemoveConstraint(const ExistenceRule &rule) { + auto found = std::find(constraints_.begin(), constraints_.end(), rule); + if (found != constraints_.end()) { + constraints_.erase(found); + } +} + +bool ExistenceConstraints::Exists(const ExistenceRule &rule) const { + auto found = std::find(constraints_.begin(), constraints_.end(), rule); + return found != constraints_.end(); +} + +bool ExistenceConstraints::CheckIfSatisfies(const Vertex *vertex) const { + for (auto &constraint : constraints_) { + if (!CheckIfSatisfiesExistenceRule(vertex, constraint)) return false; + } + + return true; +} + +const std::list<ExistenceRule> &ExistenceConstraints::ListConstraints() const { + return constraints_; +} +} // namespace database diff --git a/src/storage/single_node/constraints/existence_constraints.hpp b/src/storage/single_node/constraints/existence_constraints.hpp new file mode 100644 index 000000000..282973c2f --- /dev/null +++ b/src/storage/single_node/constraints/existence_constraints.hpp @@ -0,0 +1,76 @@ +/// @file +#pragma once + +#include <list> +#include <unordered_map> +#include <vector> + +#include "storage/common/types/property_value.hpp" +#include "storage/common/types/types.hpp" +#include "storage/single_node/vertex.hpp" +#include "transactions/type.hpp" + +namespace database { + +/// Existence rule defines label -> set of properties rule. This means that +/// every vertex with that label has to have every property in the set of +/// properties. This rule doesn't care about the PropertyValues for those +/// properties. +struct ExistenceRule { + storage::Label label; + std::vector<storage::Property> properties; + + bool operator==(const ExistenceRule &rule) const { + return label == rule.label && properties == rule.properties; + } + + bool operator!=(const ExistenceRule &rule) const { return !(*this == rule); } +}; + +bool CheckIfSatisfiesExistenceRule(const Vertex *vertex, + const database::ExistenceRule &rule); + +/// ExistenceConstraints contains all active constrains. Existence constraints +/// are restriction on the vertices and are defined by ExistenceRule. +/// To create and delete constraint, the caller must ensure that there are no +/// other transactions running in parallel. +/// Additionally, for adding constraint caller must check existing vertices for +/// constraint violations before adding that constraint. You may use +/// CheckIfSatisfiesExistenceRule function for that. +/// This is needed to ensure logical correctness of transactions. +/// Once created, the client uses method CheckIfSatisfies to check that updated +/// vertex doesn't violate any of the existing constraints. If it does, that +/// update must be reverted. +class ExistenceConstraints { + public: + ExistenceConstraints() = default; + ExistenceConstraints(const ExistenceConstraints &) = delete; + ExistenceConstraints(ExistenceConstraints &&) = delete; + ExistenceConstraints &operator=(const ExistenceConstraints &) = delete; + ExistenceConstraints &operator=(ExistenceConstraints &&) = delete; + + /// Adds new constraint, if the constraint already exists this method does + /// nothing. This method doesn't check if any of the existing vertices breaks + /// this constraint. Caller must do that instead. Caller must also ensure + /// that no other transaction is running in parallel. + void AddConstraint(const ExistenceRule &rule); + + /// Removes existing constraint, if the constraint doesn't exist this method + /// does nothing. Caller must ensure that no other transaction is running in + /// parallel. + void RemoveConstraint(const ExistenceRule &rule); + + /// Checks whether given constraint is visible. + bool Exists(const ExistenceRule &rule) const; + + /// Check if given vertex satisfies all visible constraints. + // TODO I could check this faster if I knew exactly what changed + bool CheckIfSatisfies(const Vertex *vertex) const; + + /// Returns list of all constraints. + const std::list<ExistenceRule> &ListConstraints() const; + + private: + std::list<ExistenceRule> constraints_; +}; +}; // namespace database diff --git a/src/storage/single_node/record_accessor.cpp b/src/storage/single_node/record_accessor.cpp index 77bc8fcd6..127bca7a2 100644 --- a/src/storage/single_node/record_accessor.cpp +++ b/src/storage/single_node/record_accessor.cpp @@ -48,7 +48,8 @@ void RecordAccessor<Vertex>::PropsErase(storage::Property key) { StateDelta::PropsSetVertex(dba.transaction_id(), gid(), key, dba.PropertyName(key), PropertyValue::Null); update().properties_.set(key, PropertyValue::Null); - db_accessor().wal().Emplace(delta); + dba.UpdateOnPropertyRemove(key, GetNew(), *this); + dba.wal().Emplace(delta); } template <> diff --git a/src/storage/single_node/storage.hpp b/src/storage/single_node/storage.hpp index e940c0001..8d975338c 100644 --- a/src/storage/single_node/storage.hpp +++ b/src/storage/single_node/storage.hpp @@ -8,6 +8,7 @@ #include "storage/common/types/types.hpp" #include "storage/common/kvstore/kvstore.hpp" #include "storage/single_node/edge.hpp" +#include "storage/single_node/constraints/existence_constraints.hpp" #include "storage/single_node/indexes/key_index.hpp" #include "storage/single_node/indexes/label_property_index.hpp" #include "storage/single_node/vertex.hpp" @@ -75,6 +76,9 @@ class Storage { KeyIndex<storage::Label, Vertex> labels_index_; LabelPropertyIndex label_property_index_; + // existence constraints + ExistenceConstraints existence_constraints_; + std::vector<std::string> properties_on_disk_; /// Gets the Vertex/Edge main storage map. diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 26379f40c..722b3b079 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -128,6 +128,9 @@ target_link_libraries(${test_prefix}edges_distributed mg-distributed kvstore_dum add_unit_test(edges_single_node.cpp) target_link_libraries(${test_prefix}edges_single_node mg-single-node kvstore_dummy_lib) +add_unit_test(existence_constraints.cpp) +target_link_libraries(${test_prefix}existence_constraints mg-single-node kvstore_dummy_lib) + add_unit_test(gid.cpp) target_link_libraries(${test_prefix}gid mg-distributed kvstore_dummy_lib) diff --git a/tests/unit/existence_constraints.cpp b/tests/unit/existence_constraints.cpp new file mode 100644 index 000000000..68d84cb2b --- /dev/null +++ b/tests/unit/existence_constraints.cpp @@ -0,0 +1,140 @@ +#include <gflags/gflags.h> +#include <glog/logging.h> +#include <gtest/gtest.h> + +#include "database/single_node/graph_db.hpp" +#include "database/single_node/graph_db_accessor.hpp" +#include "storage/single_node/constraints/existence_constraints.hpp" + +class ExistenceConstraintsTest : public ::testing::Test { + public: + void SetUp() override {} + database::ExistenceConstraints constraints_; + database::GraphDb db_; +}; + +TEST_F(ExistenceConstraintsTest, MultiBuildDrop) { + auto d = db_.Access(); + auto label = d->Label("label"); + auto prop = d->Property("property"); + database::ExistenceRule rule{label, std::vector<storage::Property>{prop}}; + d->Commit(); + + { + auto dba = db_.AccessBlocking(); + constraints_.AddConstraint(rule); + EXPECT_TRUE(constraints_.Exists(rule)); + dba->Commit(); + } + { + auto dba = db_.AccessBlocking(); + constraints_.RemoveConstraint(rule); + EXPECT_FALSE(constraints_.Exists(rule)); + dba->Commit(); + } +} + +TEST_F(ExistenceConstraintsTest, InsertTest) { + auto d = db_.Access(); + auto label = d->Label("label"); + auto prop = d->Property("property"); + database::ExistenceRule rule{label, std::vector<storage::Property>{prop}}; + d->Commit(); + + { + auto dba = db_.Access(); + auto v = dba->InsertVertex(); + v.add_label(label); + EXPECT_TRUE(constraints_.CheckIfSatisfies(v.GetNew())); + dba->Commit(); + } + { + auto dba = db_.Access(); + bool can_add_constraint = true; + for (auto v : dba->Vertices(false)) { + if (!database::CheckIfSatisfiesExistenceRule(v.GetOld(), rule)) { + can_add_constraint = false; + } + } + + EXPECT_FALSE(can_add_constraint); + dba->Commit(); + } + { + auto dba = db_.AccessBlocking(); + constraints_.AddConstraint(rule); + dba->Commit(); + } + { + auto dba = db_.Access(); + auto v1 = dba->InsertVertex(); + v1.add_label(label); + EXPECT_FALSE( + constraints_.CheckIfSatisfies(v1.GetNew())); + auto v2 = dba->InsertVertex(); + v2.PropsSet(prop, PropertyValue(false)); + v2.add_label(label); + EXPECT_TRUE(constraints_.CheckIfSatisfies(v2.GetNew())); + dba->Commit(); + } + { + auto dba = db_.AccessBlocking(); + constraints_.RemoveConstraint(rule); + dba->Commit(); + } + { + auto dba = db_.Access(); + auto v = dba->InsertVertex(); + v.add_label(label); + EXPECT_TRUE(constraints_.CheckIfSatisfies(v.GetNew())); + dba->Commit(); + } +} + +TEST_F(ExistenceConstraintsTest, GraphDbAccessor) { + auto d = db_.Access(); + auto label = d->Label("label"); + auto prop = d->Property("property"); + auto properties = std::vector<storage::Property>{prop}; + d->Commit(); + + { + auto dba = db_.Access(); + dba->BuildExistenceConstraint(label, properties); + // Constraint is not visible because transaction creates blocking + // transaction with different id; + dba->Commit(); + } + { + auto dba = db_.Access(); + EXPECT_TRUE(dba->ExistenceConstraintExists(label, properties)); + auto v1 = dba->InsertVertex(); + EXPECT_THROW(v1.add_label(label), + database::IndexConstraintViolationException); + auto v2 = dba->InsertVertex(); + v2.PropsSet(prop, PropertyValue(false)); + v2.add_label(label); + EXPECT_THROW(v2.PropsErase(prop), + database::IndexConstraintViolationException); + v2.remove_label(label); + dba->Commit(); + } + { + auto dba = db_.Access(); + dba->DeleteExistenceConstraint(label, properties); + dba->Commit(); + } + { + auto dba = db_.Access(); + EXPECT_FALSE(dba->ExistenceConstraintExists(label, properties)); + auto v1 = dba->InsertVertex(); + v1.add_label(label); + dba->Commit(); + } +} + +int main(int argc, char **argv) { + google::InitGoogleLogging(argv[0]); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}