From f6e78ce6da7872fe35b1c9e7b78e101d05afa985 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= <antaljanosbenjamin@users.noreply.github.com> Date: Wed, 26 Oct 2022 10:40:35 +0200 Subject: [PATCH] Fix TODOs in storage engine (#614) Fixed various TODOs that were easy to fix to improve the code quality of the newly implemented storage. --- src/storage/v3/bindings/db_accessor.hpp | 14 ---- src/storage/v3/key_store.hpp | 2 - src/storage/v3/schema_validator.cpp | 53 ++----------- src/storage/v3/schema_validator.hpp | 5 -- src/storage/v3/shard.cpp | 48 ------------ src/storage/v3/shard.hpp | 8 -- src/storage/v3/shard_rsm.cpp | 50 ++++-------- src/storage/v3/value_conversions.hpp | 2 - src/storage/v3/vertex_accessor.cpp | 2 +- tests/unit/storage_v3.cpp | 91 ++++++++++------------ tests/unit/storage_v3_edge.cpp | 30 ++++--- tests/unit/storage_v3_expr.cpp | 10 +-- tests/unit/storage_v3_indices.cpp | 23 +++--- tests/unit/storage_v3_isolation_level.cpp | 3 +- tests/unit/storage_v3_schema.cpp | 53 ++++--------- tests/unit/storage_v3_vertex_accessors.cpp | 35 ++++----- 16 files changed, 121 insertions(+), 308 deletions(-) diff --git a/src/storage/v3/bindings/db_accessor.hpp b/src/storage/v3/bindings/db_accessor.hpp index 1424c1542..186a1b5c5 100644 --- a/src/storage/v3/bindings/db_accessor.hpp +++ b/src/storage/v3/bindings/db_accessor.hpp @@ -51,10 +51,6 @@ class DbAccessor final { public: explicit DbAccessor(storage::v3::Shard::Accessor *accessor) : accessor_(accessor) {} - // TODO(jbajic) Fix Remove Gid - // NOLINTNEXTLINE(readability-convert-member-functions-to-static) - std::optional<VertexAccessor> FindVertex(uint64_t /*unused*/) { return std::nullopt; } - std::optional<VertexAccessor> FindVertex(storage::v3::PrimaryKey &primary_key, storage::v3::View view) { auto maybe_vertex = accessor_->FindVertex(primary_key, view); if (maybe_vertex) return VertexAccessor(*maybe_vertex); @@ -82,16 +78,6 @@ class DbAccessor final { return VerticesIterable(accessor_->Vertices(label, property, lower, upper, view)); } - storage::v3::ResultSchema<VertexAccessor> InsertVertexAndValidate( - const storage::v3::LabelId primary_label, const std::vector<storage::v3::LabelId> &labels, - const std::vector<std::pair<storage::v3::PropertyId, storage::v3::PropertyValue>> &properties) { - auto maybe_vertex_acc = accessor_->CreateVertexAndValidate(primary_label, labels, properties); - if (maybe_vertex_acc.HasError()) { - return {std::move(maybe_vertex_acc.GetError())}; - } - return maybe_vertex_acc.GetValue(); - } - storage::v3::Result<EdgeAccessor> InsertEdge(VertexAccessor *from, VertexAccessor *to, const storage::v3::EdgeTypeId &edge_type) { static constexpr auto kDummyGid = storage::v3::Gid::FromUint(0); diff --git a/src/storage/v3/key_store.hpp b/src/storage/v3/key_store.hpp index c65132d9a..4bc3c25e3 100644 --- a/src/storage/v3/key_store.hpp +++ b/src/storage/v3/key_store.hpp @@ -42,7 +42,6 @@ class KeyStore { PrimaryKey Keys() const; friend bool operator<(const KeyStore &lhs, const KeyStore &rhs) { - // TODO(antaljanosbenjamin): also compare the schema return std::ranges::lexicographical_compare(lhs.Keys(), rhs.Keys(), std::less<PropertyValue>{}); } @@ -51,7 +50,6 @@ class KeyStore { } friend bool operator<(const KeyStore &lhs, const PrimaryKey &rhs) { - // TODO(antaljanosbenjamin): also compare the schema return std::ranges::lexicographical_compare(lhs.Keys(), rhs, std::less<PropertyValue>{}); } diff --git a/src/storage/v3/schema_validator.cpp b/src/storage/v3/schema_validator.cpp index 4eaa626ec..748eda28d 100644 --- a/src/storage/v3/schema_validator.cpp +++ b/src/storage/v3/schema_validator.cpp @@ -40,46 +40,7 @@ SchemaViolation::SchemaViolation(ValidationStatus status, LabelId label, SchemaP SchemaValidator::SchemaValidator(Schemas &schemas) : schemas_{schemas} {} -[[nodiscard]] std::optional<SchemaViolation> SchemaValidator::ValidateVertexCreate( - LabelId primary_label, const std::vector<LabelId> &labels, - const std::vector<std::pair<PropertyId, PropertyValue>> &properties) const { - // Schema on primary label - const auto *schema = schemas_.GetSchema(primary_label); - if (schema == nullptr) { - return SchemaViolation(SchemaViolation::ValidationStatus::NO_SCHEMA_DEFINED_FOR_LABEL, primary_label); - } - - // Is there another primary label among secondary labels - for (const auto &secondary_label : labels) { - if (schemas_.GetSchema(secondary_label)) { - return SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_SECONDARY_LABEL_IS_PRIMARY, secondary_label); - } - } - - // Check only properties defined by schema - for (const auto &schema_type : schema->second) { - // Check schema property existence - auto property_pair = std::ranges::find_if( - properties, [schema_property_id = schema_type.property_id](const auto &property_type_value) { - return property_type_value.first == schema_property_id; - }); - if (property_pair == properties.end()) { - return SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_HAS_NO_PRIMARY_PROPERTY, primary_label, - schema_type); - } - - // Check schema property type - if (auto property_schema_type = PropertyTypeToSchemaType(property_pair->second); - property_schema_type && *property_schema_type != schema_type.type) { - return SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_PROPERTY_WRONG_TYPE, primary_label, schema_type, - property_pair->second); - } - } - - return std::nullopt; -} - -[[nodiscard]] std::optional<SchemaViolation> SchemaValidator::ValidateVertexCreate( +std::optional<SchemaViolation> SchemaValidator::ValidateVertexCreate( LabelId primary_label, const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties) const { // Schema on primary label @@ -112,8 +73,8 @@ SchemaValidator::SchemaValidator(Schemas &schemas) : schemas_{schemas} {} return std::nullopt; } -[[nodiscard]] std::optional<SchemaViolation> SchemaValidator::ValidatePropertyUpdate( - const LabelId primary_label, const PropertyId property_id) const { +std::optional<SchemaViolation> SchemaValidator::ValidatePropertyUpdate(const LabelId primary_label, + const PropertyId property_id) const { // Verify existence of schema on primary label const auto *schema = schemas_.GetSchema(primary_label); MG_ASSERT(schema, "Cannot validate against non existing schema!"); @@ -129,7 +90,7 @@ SchemaValidator::SchemaValidator(Schemas &schemas) : schemas_{schemas} {} return std::nullopt; } -[[nodiscard]] std::optional<SchemaViolation> SchemaValidator::ValidateLabelUpdate(const LabelId label) const { +std::optional<SchemaViolation> SchemaValidator::ValidateLabelUpdate(const LabelId label) const { const auto *schema = schemas_.GetSchema(label); if (schema) { return SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_UPDATE_PRIMARY_LABEL, label); @@ -142,15 +103,15 @@ const Schemas::Schema *SchemaValidator::GetSchema(LabelId label) const { return VertexValidator::VertexValidator(const SchemaValidator &schema_validator, const LabelId primary_label) : schema_validator{&schema_validator}, primary_label_{primary_label} {} -[[nodiscard]] std::optional<SchemaViolation> VertexValidator::ValidatePropertyUpdate(PropertyId property_id) const { +std::optional<SchemaViolation> VertexValidator::ValidatePropertyUpdate(PropertyId property_id) const { return schema_validator->ValidatePropertyUpdate(primary_label_, property_id); }; -[[nodiscard]] std::optional<SchemaViolation> VertexValidator::ValidateAddLabel(LabelId label) const { +std::optional<SchemaViolation> VertexValidator::ValidateAddLabel(LabelId label) const { return schema_validator->ValidateLabelUpdate(label); } -[[nodiscard]] std::optional<SchemaViolation> VertexValidator::ValidateRemoveLabel(LabelId label) const { +std::optional<SchemaViolation> VertexValidator::ValidateRemoveLabel(LabelId label) const { return schema_validator->ValidateLabelUpdate(label); } diff --git a/src/storage/v3/schema_validator.hpp b/src/storage/v3/schema_validator.hpp index 6cfdc4c11..d57dfae2e 100644 --- a/src/storage/v3/schema_validator.hpp +++ b/src/storage/v3/schema_validator.hpp @@ -23,7 +23,6 @@ namespace memgraph::storage::v3 { struct SchemaViolation { enum class ValidationStatus : uint8_t { - VERTEX_HAS_NO_PRIMARY_PROPERTY, NO_SCHEMA_DEFINED_FOR_LABEL, VERTEX_PROPERTY_WRONG_TYPE, VERTEX_UPDATE_PRIMARY_KEY, @@ -51,10 +50,6 @@ class SchemaValidator { public: explicit SchemaValidator(Schemas &schemas); - [[deprecated]] std::optional<SchemaViolation> ValidateVertexCreate( - LabelId primary_label, const std::vector<LabelId> &labels, - const std::vector<std::pair<PropertyId, PropertyValue>> &properties) const; - [[nodiscard]] std::optional<SchemaViolation> ValidateVertexCreate( LabelId primary_label, const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties) const; diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index c95fa9cad..ebc571333 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -341,53 +341,6 @@ Shard::~Shard() {} Shard::Accessor::Accessor(Shard &shard, Transaction &transaction) : shard_(&shard), transaction_(&transaction), config_(shard_->config_.items) {} -// TODO(jbajic) Remove with next PR -ResultSchema<VertexAccessor> Shard::Accessor::CreateVertexAndValidate( - LabelId primary_label, const std::vector<LabelId> &labels, - const std::vector<std::pair<PropertyId, PropertyValue>> &properties) { - if (primary_label != shard_->primary_label_) { - throw utils::BasicException("Cannot add vertex to shard which does not hold the given primary label!"); - } - auto maybe_schema_violation = GetSchemaValidator().ValidateVertexCreate(primary_label, labels, properties); - if (maybe_schema_violation) { - return {std::move(*maybe_schema_violation)}; - } - OOMExceptionEnabler oom_exception; - // Extract key properties - std::vector<PropertyValue> primary_properties; - for ([[maybe_unused]] const auto &[property_id, property_type] : shard_->GetSchema(primary_label)->second) { - // We know there definitely is key in properties since we have validated - primary_properties.push_back( - std::ranges::find_if(properties, [property_id = property_id](const auto &property_pair) { - return property_pair.first == property_id; - })->second); - } - auto acc = shard_->vertices_.access(); - auto *delta = CreateDeleteObjectDelta(transaction_); - auto [it, inserted] = acc.insert({Vertex{delta, primary_properties}}); - delta->prev.Set(&it->vertex); - - VertexAccessor vertex_acc{&it->vertex, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; - MG_ASSERT(inserted, "The vertex must be inserted here!"); - MG_ASSERT(it != acc.end(), "Invalid Vertex accessor!"); - - // TODO(jbajic) Improve, maybe delay index update - for (const auto &[property_id, property_value] : properties) { - if (!shard_->schemas_.IsPropertyKey(primary_label, property_id)) { - if (const auto err = vertex_acc.SetProperty(property_id, property_value); err.HasError()) { - return {err.GetError()}; - } - } - } - // Set secondary labels - for (auto label : labels) { - if (const auto err = vertex_acc.AddLabel(label); err.HasError()) { - return {err.GetError()}; - } - } - return vertex_acc; -} - ResultSchema<VertexAccessor> Shard::Accessor::CreateVertexAndValidate( const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties, const std::vector<std::pair<PropertyId, PropertyValue>> &properties) { @@ -1078,7 +1031,6 @@ void Shard::CollectGarbage(const io::Time current_time) { } Transaction &Shard::GetTransaction(const coordinator::Hlc start_timestamp, IsolationLevel isolation_level) { - // TODO(antaljanosbenjamin) if (const auto it = start_logical_id_to_transaction_.find(start_timestamp.logical_id); it != start_logical_id_to_transaction_.end()) { MG_ASSERT(it->second->start_timestamp.coordinator_wall_clock == start_timestamp.coordinator_wall_clock, diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index e866311de..268d350ae 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -71,7 +71,6 @@ class AllVerticesIterable final { Indices *indices_; Config::Items config_; const VertexValidator *vertex_validator_; - const Schemas *schemas_; std::optional<VertexAccessor> vertex_; public: @@ -205,13 +204,6 @@ class Shard final { Accessor(Shard &shard, Transaction &transaction); public: - // TODO(gvolfing) this is just a workaround for stitching remove this later. - LabelId GetPrimaryLabel() const noexcept { return shard_->primary_label_; } - - ResultSchema<VertexAccessor> CreateVertexAndValidate( - LabelId primary_label, const std::vector<LabelId> &labels, - const std::vector<std::pair<PropertyId, PropertyValue>> &properties); - /// @throw std::bad_alloc ResultSchema<VertexAccessor> CreateVertexAndValidate( const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties, diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 679f95172..a875e0164 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -167,16 +167,6 @@ std::vector<TypedValue> EvaluateVertexExpressions(DbAccessor &dba, const VertexA return evaluated_expressions; } -bool DoesEdgeTypeMatch(const std::vector<msgs::EdgeType> &edge_types, const EdgeAccessor &edge) { - // TODO(gvolfing) This should be checked only once and handled accordingly. - if (edge_types.empty()) { - return true; - } - - return std::ranges::any_of(edge_types.begin(), edge_types.end(), - [&edge](const msgs::EdgeType &edge_type) { return edge_type.id == edge.EdgeType(); }); -} - struct LocalError {}; std::optional<msgs::Vertex> FillUpSourceVertex(const std::optional<VertexAccessor> &v_acc, @@ -235,12 +225,17 @@ std::optional<std::map<PropertyId, Value>> FillUpSourceVertexProperties(const st std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( const std::optional<VertexAccessor> &v_acc, const msgs::ExpandOneRequest &req, const EdgeUniqunessFunction &maybe_filter_based_on_edge_uniquness) { + std::vector<EdgeTypeId> edge_types{}; + edge_types.reserve(req.edge_types.size()); + std::transform(req.edge_types.begin(), req.edge_types.end(), std::back_inserter(edge_types), + [](const msgs::EdgeType &edge_type) { return edge_type.id; }); + std::vector<EdgeAccessor> in_edges; std::vector<EdgeAccessor> out_edges; switch (req.direction) { case msgs::EdgeDirection::OUT: { - auto out_edges_result = v_acc->OutEdges(View::NEW); + auto out_edges_result = v_acc->OutEdges(View::NEW, edge_types); if (out_edges_result.HasError()) { spdlog::debug("Encountered an error while trying to get out-going EdgeAccessors. Transaction id: {}", req.transaction_id.logical_id); @@ -251,7 +246,7 @@ std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( break; } case msgs::EdgeDirection::IN: { - auto in_edges_result = v_acc->InEdges(View::NEW); + auto in_edges_result = v_acc->InEdges(View::NEW, edge_types); if (in_edges_result.HasError()) { spdlog::debug( "Encountered an error while trying to get in-going EdgeAccessors. Transaction id: {}"[req.transaction_id @@ -262,7 +257,7 @@ std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( break; } case msgs::EdgeDirection::BOTH: { - auto in_edges_result = v_acc->InEdges(View::NEW); + auto in_edges_result = v_acc->InEdges(View::NEW, edge_types); if (in_edges_result.HasError()) { spdlog::debug("Encountered an error while trying to get in-going EdgeAccessors. Transaction id: {}", req.transaction_id.logical_id); @@ -270,7 +265,7 @@ std::optional<std::array<std::vector<EdgeAccessor>, 2>> FillUpConnectingEdges( } in_edges = maybe_filter_based_on_edge_uniquness(std::move(in_edges_result.GetValue()), msgs::EdgeDirection::IN); - auto out_edges_result = v_acc->OutEdges(View::NEW); + auto out_edges_result = v_acc->OutEdges(View::NEW, edge_types); if (out_edges_result.HasError()) { spdlog::debug("Encountered an error while trying to get out-going EdgeAccessors. Transaction id: {}", req.transaction_id.logical_id); @@ -296,13 +291,8 @@ using AllEdgePropertiesVector = std::vector<AllEdgeProperties>; using EdgeFiller = std::function<bool(const EdgeAccessor &edge, bool is_in_edge, msgs::ExpandOneResultRow &result_row)>; template <bool are_in_edges> -bool FillEdges(const std::vector<EdgeAccessor> &edges, const msgs::ExpandOneRequest &req, msgs::ExpandOneResultRow &row, - const EdgeFiller &edge_filler) { +bool FillEdges(const std::vector<EdgeAccessor> &edges, msgs::ExpandOneResultRow &row, const EdgeFiller &edge_filler) { for (const auto &edge : edges) { - if (!DoesEdgeTypeMatch(req.edge_types, edge)) { - continue; - } - if (!edge_filler(edge, are_in_edges, row)) { return false; } @@ -342,10 +332,10 @@ std::optional<msgs::ExpandOneResultRow> GetExpandOneResult( result_row.src_vertex_properties = std::move(*src_vertex_properties); static constexpr bool kInEdges = true; static constexpr bool kOutEdges = false; - if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, req, result_row, edge_filler)) { + if (!in_edges.empty() && !FillEdges<kInEdges>(in_edges, result_row, edge_filler)) { return std::nullopt; } - if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, req, result_row, edge_filler)) { + if (!out_edges.empty() && !FillEdges<kOutEdges>(out_edges, result_row, edge_filler)) { return std::nullopt; } @@ -464,10 +454,6 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { bool action_successful = true; for (auto &new_vertex : req.new_vertices) { - /// TODO(gvolfing) Remove this. In the new implementation each shard - /// should have a predetermined primary label, so there is no point in - /// specifying it in the accessor functions. Their signature will - /// change. /// TODO(gvolfing) Consider other methods than converting. Change either /// the way that the property map is stored in the messages, or the /// signature of CreateVertexAndValidate. @@ -480,11 +466,9 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { std::transform(new_vertex.label_ids.begin(), new_vertex.label_ids.end(), std::back_inserter(converted_label_ids), [](const auto &label_id) { return label_id.id; }); - // TODO(jbajic) sending primary key as vector breaks validation on storage side - // cannot map id -> value PrimaryKey transformed_pk; std::transform(new_vertex.primary_key.begin(), new_vertex.primary_key.end(), std::back_inserter(transformed_pk), - [](const auto &val) { return ToPropertyValue(val); }); + [](msgs::Value &val) { return ToPropertyValue(std::move(val)); }); auto result_schema = acc.CreateVertexAndValidate(converted_label_ids, transformed_pk, converted_property_map); if (result_schema.HasError()) { @@ -530,8 +514,6 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::UpdateVerticesRequest &&req) { } for (auto &update_prop : vertex.property_updates) { - // TODO(gvolfing) Maybe check if the setting is valid if SetPropertyAndValidate() - // does not do that alreaedy. auto result_schema = vertex_to_update->SetPropertyAndValidate(update_prop.first, ToPropertyValue(std::move(update_prop.second))); if (result_schema.HasError()) { @@ -758,12 +740,6 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::ScanVerticesRequest &&req) { auto dba = DbAccessor{&acc}; const auto emplace_scan_result = [&](const VertexAccessor &vertex) { std::vector<Value> expression_results; - // TODO(gvolfing) it should be enough to check these only once. - if (vertex.Properties(View(req.storage_view)).HasError()) { - action_successful = false; - spdlog::debug("Could not retrieve properties from VertexAccessor. Transaction id: {}", - req.transaction_id.logical_id); - } if (!req.filter_expressions.empty()) { // NOTE - DbAccessor might get removed in the future. const bool eval = FilterOnVertex(dba, vertex, req.filter_expressions, expr::identifier_node_symbol); diff --git a/src/storage/v3/value_conversions.hpp b/src/storage/v3/value_conversions.hpp index 5649a6dde..05fd1394b 100644 --- a/src/storage/v3/value_conversions.hpp +++ b/src/storage/v3/value_conversions.hpp @@ -20,8 +20,6 @@ #pragma once -// TODO(kostasrim) Think about long term sustainability - // This should not be put under v3 because ADL will mess that up. namespace memgraph::storage::conversions { diff --git a/src/storage/v3/vertex_accessor.cpp b/src/storage/v3/vertex_accessor.cpp index 9d4e94c68..696c4657a 100644 --- a/src/storage/v3/vertex_accessor.cpp +++ b/src/storage/v3/vertex_accessor.cpp @@ -453,7 +453,7 @@ Result<std::map<PropertyId, PropertyValue>> VertexAccessor::Properties(View view Delta *delta = nullptr; { deleted = vertex_->deleted; - // TODO(antaljanosbenjamin): This should also return the primary key + // TODO(antaljanosbenjamin): Should this also return the primary key? properties = vertex_->properties.Properties(); delta = vertex_->delta; } diff --git a/tests/unit/storage_v3.cpp b/tests/unit/storage_v3.cpp index 9c5c9b9bf..b62606a8e 100644 --- a/tests/unit/storage_v3.cpp +++ b/tests/unit/storage_v3.cpp @@ -41,10 +41,10 @@ class StorageV3 : public ::testing::TestWithParam<bool> { void TearDown() override { CleanupHlc(last_hlc); } - VertexAccessor CreateVertexAndValidate(Shard::Accessor &acc, LabelId primary_label, - const std::vector<LabelId> &labels, + VertexAccessor CreateVertexAndValidate(Shard::Accessor &acc, const std::vector<LabelId> &labels, + const PropertyValue &primary_key, const std::vector<std::pair<PropertyId, PropertyValue>> &properties) { - auto vtx = acc.CreateVertexAndValidate(primary_label, labels, properties); + auto vtx = acc.CreateVertexAndValidate(labels, {primary_key}, properties); EXPECT_TRUE(vtx.HasValue()); return *vtx; } @@ -104,7 +104,7 @@ TEST_P(StorageV3, Commit) { const auto create_start_hlc = GetNextHlc(); { auto acc = store.Access(create_start_hlc); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk, View::NEW).has_value()); @@ -153,7 +153,7 @@ TEST_P(StorageV3, Commit) { TEST_P(StorageV3, Abort) { { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk, View::NEW).has_value()); @@ -178,7 +178,7 @@ TEST_P(StorageV3, AbortByGc) { { const auto hlc = GetNextHlc(); auto acc = store.Access(hlc); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk, View::NEW).has_value()); @@ -203,7 +203,7 @@ TEST_P(StorageV3, AdvanceCommandCommit) { { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk1, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk1, View::NEW).has_value()); @@ -211,7 +211,7 @@ TEST_P(StorageV3, AdvanceCommandCommit) { acc.AdvanceCommand(); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(2)}}); + CreateVertexAndValidate(acc, {}, PropertyValue{2}, {}); ASSERT_FALSE(acc.FindVertex(pk2, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 1U); ASSERT_TRUE(acc.FindVertex(pk2, View::NEW).has_value()); @@ -242,7 +242,7 @@ TEST_P(StorageV3, AdvanceCommandAbort) { { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk1, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk1, View::NEW).has_value()); @@ -250,7 +250,7 @@ TEST_P(StorageV3, AdvanceCommandAbort) { acc.AdvanceCommand(); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(2)}}); + CreateVertexAndValidate(acc, {}, PropertyValue{2}, {}); ASSERT_FALSE(acc.FindVertex(pk2, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 1U); ASSERT_TRUE(acc.FindVertex(pk2, View::NEW).has_value()); @@ -282,7 +282,7 @@ TEST_P(StorageV3, SnapshotIsolation) { auto acc1 = store.Access(start_hlc1); auto acc2 = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc1, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc1, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc2.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc1, View::OLD), 0U); @@ -313,7 +313,7 @@ TEST_P(StorageV3, SnapshotIsolation) { TEST_P(StorageV3, AccessorMove) { { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); @@ -346,7 +346,7 @@ TEST_P(StorageV3, VertexDeleteCommit) { // Create the vertex in transaction 2 { - CreateVertexAndValidate(acc2, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc2, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc2.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc2, View::OLD), 0U); ASSERT_TRUE(acc2.FindVertex(pk, View::NEW).has_value()); @@ -416,7 +416,7 @@ TEST_P(StorageV3, VertexDeleteAbort) { // Create the vertex in transaction 2 { - CreateVertexAndValidate(acc2, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc2, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc2.FindVertex(pk, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc2, View::OLD), 0U); ASSERT_TRUE(acc2.FindVertex(pk, View::NEW).has_value()); @@ -536,7 +536,7 @@ TEST_P(StorageV3, VertexDeleteSerializationError) { // Create vertex { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); acc.Commit(GetNextHlc()); } @@ -612,7 +612,7 @@ TEST_P(StorageV3, VertexDeleteSpecialCases) { // transaction { auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk1, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk1, View::NEW).has_value()); @@ -631,7 +631,7 @@ TEST_P(StorageV3, VertexDeleteSpecialCases) { // Create vertex and delete it in the same transaction { auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(2)}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{2}, {}); ASSERT_FALSE(acc.FindVertex(pk2, View::OLD).has_value()); EXPECT_EQ(CountVertices(acc, View::OLD), 0U); ASSERT_TRUE(acc.FindVertex(pk2, View::NEW).has_value()); @@ -673,7 +673,7 @@ TEST_P(StorageV3, VertexDeleteLabel) { // Create the vertex { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk, View::OLD).has_value()); ASSERT_TRUE(acc.FindVertex(pk, View::NEW).has_value()); acc.Commit(GetNextHlc()); @@ -817,7 +817,7 @@ TEST_P(StorageV3, VertexDeleteProperty) { // Create the vertex { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(acc.FindVertex(pk, View::OLD).has_value()); ASSERT_TRUE(acc.FindVertex(pk, View::NEW).has_value()); acc.Commit(GetNextHlc()); @@ -950,7 +950,7 @@ TEST_P(StorageV3, VertexLabelCommit) { store.StoreMapping({{1, "label"}, {2, "property"}, {3, "label5"}, {4, "other"}}); { auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); auto label = NameToLabelId("label5"); @@ -1064,7 +1064,7 @@ TEST_P(StorageV3, VertexLabelAbort) { // Create the vertex. { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); acc.Commit(GetNextHlc()); } @@ -1308,7 +1308,7 @@ TEST_P(StorageV3, VertexLabelSerializationError) { store.StoreMapping({{1, "label"}, {2, "property"}, {3, "label1"}, {4, "label2"}}); { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); acc.Commit(GetNextHlc()); } @@ -1413,7 +1413,7 @@ TEST_P(StorageV3, VertexPropertyCommit) { store.StoreMapping({{1, "label"}, {2, "property"}, {3, "property5"}, {4, "other"}}); { auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); auto property = NameToPropertyId("property5"); @@ -1534,7 +1534,7 @@ TEST_P(StorageV3, VertexPropertyAbort) { // Create the vertex. { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); acc.Commit(GetNextHlc()); } @@ -1808,7 +1808,7 @@ TEST_P(StorageV3, VertexPropertySerializationError) { store.StoreMapping({{1, "label"}, {2, "property"}, {3, "property1"}, {4, "property2"}}); { auto acc = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); acc.Commit(GetNextHlc()); } @@ -1906,7 +1906,7 @@ TEST_P(StorageV3, VertexPropertySerializationError) { TEST_P(StorageV3, VertexLabelPropertyMixed) { store.StoreMapping({{1, "label"}, {2, "property"}, {3, "label5"}, {4, "property5"}}); auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); auto label = NameToLabelId("label5"); auto property = NameToPropertyId("property5"); @@ -2148,7 +2148,7 @@ TEST_P(StorageV3, VertexPropertyClear) { auto property2 = NameToPropertyId("property2"); { auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); auto old_value = vertex.SetPropertyAndValidate(property1, PropertyValue("value")); ASSERT_TRUE(old_value.HasValue()); @@ -2252,7 +2252,7 @@ TEST_P(StorageV3, VertexNonexistentLabelPropertyEdgeAPI) { auto property1 = NameToPropertyId("property1"); auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); // Check state before (OLD view). ASSERT_EQ(vertex.Labels(View::OLD).GetError(), Error::NONEXISTENT_OBJECT); @@ -2308,7 +2308,7 @@ TEST_P(StorageV3, VertexVisibilitySingleTransaction) { auto acc1 = store.Access(GetNextHlc()); auto acc2 = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc1, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc1, {}, PropertyValue{0}, {}); EXPECT_FALSE(acc1.FindVertex(pk, View::OLD)); EXPECT_TRUE(acc1.FindVertex(pk, View::NEW)); @@ -2362,7 +2362,7 @@ TEST_P(StorageV3, VertexVisibilityMultipleTransactions) { auto acc1 = store.Access(GetNextHlc()); auto acc2 = store.Access(GetNextHlc()); - CreateVertexAndValidate(acc1, primary_label, {}, {{primary_property, PropertyValue{0}}}); + CreateVertexAndValidate(acc1, {}, PropertyValue{0}, {}); EXPECT_FALSE(acc1.FindVertex(pk, View::OLD)); EXPECT_TRUE(acc1.FindVertex(pk, View::NEW)); @@ -2602,7 +2602,7 @@ TEST_P(StorageV3, DeletedVertexAccessor) { // Create the vertex { auto acc = store.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue{0}}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}, {}); ASSERT_FALSE(vertex.SetPropertyAndValidate(property1, property_value).HasError()); acc.Commit(GetNextHlc()); } @@ -2639,8 +2639,7 @@ TEST_P(StorageV3, TestCreateVertexAndValidate) { auto acc = store.Access(GetNextHlc()); const auto label1 = NameToLabelId("label1"); const auto prop1 = NameToPropertyId("prop1"); - auto vertex = acc.CreateVertexAndValidate(primary_label, {label1}, - {{primary_property, PropertyValue(0)}, {prop1, PropertyValue(111)}}); + auto vertex = acc.CreateVertexAndValidate({label1}, {PropertyValue(0)}, {{prop1, PropertyValue(111)}}); ASSERT_TRUE(vertex.HasValue()); ASSERT_TRUE(vertex->PrimaryLabel(View::NEW).HasValue()); EXPECT_EQ(vertex->PrimaryLabel(View::NEW).GetValue(), primary_label); @@ -2650,30 +2649,19 @@ TEST_P(StorageV3, TestCreateVertexAndValidate) { EXPECT_EQ(vertex->Properties(View::NEW).GetValue(), (std::map<PropertyId, PropertyValue>{{prop1, PropertyValue(111)}})); } - { - auto acc = store.Access(GetNextHlc()); - const auto label1 = NameToLabelId("label1"); - const auto prop1 = NameToPropertyId("prop1"); - EXPECT_THROW( - { - auto vertex = acc.CreateVertexAndValidate( - label1, {}, {{primary_property, PropertyValue(0)}, {prop1, PropertyValue(111)}}); - }, - utils::BasicException); - } { ASSERT_DEATH( { Shard store(primary_label, min_pk, std::nullopt /*max_primary_key*/, schema_property_vector); auto acc = store.Access(GetNextHlc()); - auto vertex1 = acc.CreateVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(0)}}); - auto vertex2 = acc.CreateVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(0)}}); + auto vertex1 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); + auto vertex2 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); }, ""); } { auto acc = store.Access(GetNextHlc()); - auto vertex = acc.CreateVertexAndValidate(primary_label, {primary_label}, {{primary_property, PropertyValue(0)}}); + auto vertex = acc.CreateVertexAndValidate({primary_label}, {PropertyValue{0}}, {}); ASSERT_TRUE(vertex.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(vertex.GetError())); EXPECT_EQ(std::get<SchemaViolation>(vertex.GetError()), @@ -2681,7 +2669,7 @@ TEST_P(StorageV3, TestCreateVertexAndValidate) { } { auto acc = store.Access(GetNextHlc()); - auto vertex = acc.CreateVertexAndValidate(primary_label, {primary_label}, {{primary_property, PropertyValue(0)}}); + auto vertex = acc.CreateVertexAndValidate({primary_label}, {PropertyValue{0}}, {}); ASSERT_TRUE(vertex.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(vertex.GetError())); EXPECT_EQ(std::get<SchemaViolation>(vertex.GetError()), @@ -2689,16 +2677,15 @@ TEST_P(StorageV3, TestCreateVertexAndValidate) { } { auto acc = store.Access(GetNextHlc()); - auto vertex = acc.CreateVertexAndValidate(primary_label, {}, {}); + auto vertex = acc.CreateVertexAndValidate({}, {}, {}); ASSERT_TRUE(vertex.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(vertex.GetError())); EXPECT_EQ(std::get<SchemaViolation>(vertex.GetError()), - SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_HAS_NO_PRIMARY_PROPERTY, primary_label, - {primary_property, common::SchemaType::INT})); + SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_PRIMARY_PROPERTIES_UNDEFINED, primary_label)); } { auto acc = store.Access(GetNextHlc()); - auto vertex = acc.CreateVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue("test")}}); + auto vertex = acc.CreateVertexAndValidate({}, {PropertyValue{"test"}}, {}); ASSERT_TRUE(vertex.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(vertex.GetError())); EXPECT_EQ(std::get<SchemaViolation>(vertex.GetError()), diff --git a/tests/unit/storage_v3_edge.cpp b/tests/unit/storage_v3_edge.cpp index efa963aba..293f561df 100644 --- a/tests/unit/storage_v3_edge.cpp +++ b/tests/unit/storage_v3_edge.cpp @@ -38,8 +38,8 @@ class StorageEdgeTest : public ::testing::TestWithParam<bool> { return store.NameToEdgeType(edge_type_name); } - ResultSchema<VertexAccessor> CreateVertex(Shard::Accessor &acc, const PropertyValue &key) { - return acc.CreateVertexAndValidate(primary_label, {}, {{primary_property, key}}); + static ResultSchema<VertexAccessor> CreateVertex(Shard::Accessor &acc, const PropertyValue &key) { + return acc.CreateVertexAndValidate({}, {key}, {}); } coordinator::Hlc GetNextHlc() { @@ -72,12 +72,11 @@ TEST_P(StorageEdgeTest, EdgeCreateFromSmallerCommit) { const auto et = NameToEdgeTypeId("et5"); const auto edge_id = Gid::FromUint(0U); auto acc = store.Access(GetNextHlc()); - const auto [from_id, to_id] = - std::invoke([this, &from_key, &to_key, &acc]() mutable -> std::pair<VertexId, VertexId> { - auto from_id = CreateVertex(acc, from_key)->Id(View::NEW).GetValue(); - auto to_id = CreateVertex(acc, to_key)->Id(View::NEW).GetValue(); - return std::make_pair(std::move(from_id), std::move(to_id)); - }); + const auto [from_id, to_id] = std::invoke([&from_key, &to_key, &acc]() mutable -> std::pair<VertexId, VertexId> { + auto from_id = CreateVertex(acc, from_key)->Id(View::NEW).GetValue(); + auto to_id = CreateVertex(acc, to_key)->Id(View::NEW).GetValue(); + return std::make_pair(std::move(from_id), std::move(to_id)); + }); const auto other_et = NameToEdgeTypeId("other"); const VertexId from_id_with_different_label{NameToLabelId("different_label"), from_id.primary_key}; const VertexId to_id_with_different_label{NameToLabelId("different_label"), to_id.primary_key}; @@ -872,12 +871,11 @@ TEST_P(StorageEdgeTest, EdgeCreateFromLargerAbort) { const PropertyValue to_key{1}; const PropertyValue non_existing_key{2}; auto acc = store.Access(GetNextHlc()); - const auto [from_id, to_id] = - std::invoke([this, &from_key, &to_key, &acc]() mutable -> std::pair<VertexId, VertexId> { - auto from_id = CreateVertex(acc, from_key)->Id(View::NEW).GetValue(); - auto to_id = CreateVertex(acc, to_key)->Id(View::NEW).GetValue(); - return std::make_pair(std::move(from_id), std::move(to_id)); - }); + const auto [from_id, to_id] = std::invoke([&from_key, &to_key, &acc]() mutable -> std::pair<VertexId, VertexId> { + auto from_id = CreateVertex(acc, from_key)->Id(View::NEW).GetValue(); + auto to_id = CreateVertex(acc, to_key)->Id(View::NEW).GetValue(); + return std::make_pair(std::move(from_id), std::move(to_id)); + }); const auto et = NameToEdgeTypeId("et5"); const auto other_et = NameToEdgeTypeId("other"); @@ -1409,7 +1407,7 @@ TEST_P(StorageEdgeTest, EdgeDeleteFromSmallerCommit) { const PropertyValue non_existing_key{2}; auto acc = store.Access(GetNextHlc()); const auto from_id = std::invoke( - [this, &from_key, &acc]() mutable -> VertexId { return CreateVertex(acc, from_key)->Id(View::NEW).GetValue(); }); + [&from_key, &acc]() mutable -> VertexId { return CreateVertex(acc, from_key)->Id(View::NEW).GetValue(); }); const VertexId to_id{primary_label, {to_key}}; const auto et = NameToEdgeTypeId("et5"); const auto edge_id = Gid::FromUint(1U); @@ -2514,7 +2512,7 @@ TEST_P(StorageEdgeTest, EdgeDeleteFromLargerAbort) { const PropertyValue non_existing_key{2}; auto acc = store.Access(GetNextHlc()); const auto to_id = std::invoke( - [this, &to_key, &acc]() mutable -> VertexId { return CreateVertex(acc, to_key)->Id(View::NEW).GetValue(); }); + [&to_key, &acc]() mutable -> VertexId { return CreateVertex(acc, to_key)->Id(View::NEW).GetValue(); }); const VertexId from_id{primary_label, {from_key}}; const auto et = NameToEdgeTypeId("et5"); const auto edge_id = Gid::FromUint(1U); diff --git a/tests/unit/storage_v3_expr.cpp b/tests/unit/storage_v3_expr.cpp index 5b59d37e0..d238ed3a0 100644 --- a/tests/unit/storage_v3_expr.cpp +++ b/tests/unit/storage_v3_expr.cpp @@ -502,7 +502,7 @@ TEST_F(ExpressionEvaluatorTest, VertexAndEdgeIndexing) { db.StoreMapping({{1, "label"}, {2, "property"}, {3, "edge_type"}, {4, "prop"}}); auto edge_type = dba.NameToEdgeType("edge_type"); auto prop = dba.NameToProperty("prop"); - auto v1 = *dba.InsertVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(0)}}); + auto v1 = storage_dba.CreateVertexAndValidate({}, {PropertyValue(0)}, {}).GetValue(); auto e11 = dba.InsertEdge(&v1, &v1, edge_type); ASSERT_TRUE(e11.HasValue()); ASSERT_TRUE(v1.SetPropertyAndValidate(prop, memgraph::storage::v3::PropertyValue(42)).HasValue()); @@ -690,7 +690,7 @@ TEST_F(ExpressionEvaluatorTest, IsNullOperator) { TEST_F(ExpressionEvaluatorTest, LabelsTest) { db.StoreMapping({{1, "label"}, {2, "property"}, {3, "ANIMAL"}, {4, "DOG"}, {5, "NICE_DOG"}, {6, "BAD_DOG"}}); - auto v1 = *dba.InsertVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(1)}}); + auto v1 = storage_dba.CreateVertexAndValidate({}, {PropertyValue(1)}, {}).GetValue(); ASSERT_TRUE(v1.AddLabelAndValidate(dba.NameToLabel("ANIMAL")).HasValue()); ASSERT_TRUE(v1.AddLabelAndValidate(dba.NameToLabel("DOG")).HasValue()); ASSERT_TRUE(v1.AddLabelAndValidate(dba.NameToLabel("NICE_DOG")).HasValue()); @@ -1116,7 +1116,7 @@ class ExpressionEvaluatorPropertyLookup : public ExpressionEvaluatorTest { }; TEST_F(ExpressionEvaluatorPropertyLookup, Vertex) { - auto v1 = *dba.InsertVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(3)}}); + auto v1 = storage_dba.CreateVertexAndValidate({}, {PropertyValue(3)}, {}).GetValue(); ASSERT_TRUE(v1.SetPropertyAndValidate(prop_age.second, PropertyValue(10)).HasValue()); dba.AdvanceCommand(); frame[symbol] = TypedValue(v1); @@ -1277,8 +1277,8 @@ TEST_F(ExpressionEvaluatorPropertyLookup, LocalDateTime) { TEST_F(ExpressionEvaluatorPropertyLookup, Edge) { db.StoreMapping({{1, "label"}, {2, "property"}, {3, "age"}, {4, "height"}, {5, "edge_type"}}); - auto v1 = *dba.InsertVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(1)}}); - auto v2 = *dba.InsertVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(2)}}); + auto v1 = storage_dba.CreateVertexAndValidate({}, {PropertyValue(1)}, {}).GetValue(); + auto v2 = storage_dba.CreateVertexAndValidate({}, {PropertyValue(2)}, {}).GetValue(); auto e12 = dba.InsertEdge(&v1, &v2, dba.NameToEdgeType("edge_type")); ASSERT_TRUE(e12.HasValue()); ASSERT_TRUE(e12->SetProperty(prop_age.second, PropertyValue(10)).HasValue()); diff --git a/tests/unit/storage_v3_indices.cpp b/tests/unit/storage_v3_indices.cpp index a9b408833..7ff3fa43b 100644 --- a/tests/unit/storage_v3_indices.cpp +++ b/tests/unit/storage_v3_indices.cpp @@ -62,9 +62,8 @@ class IndexTest : public testing::Test { PropertyId NameToPropertyId(std::string_view property_name) { return storage.NameToProperty(property_name); } VertexAccessor CreateVertex(Shard::Accessor &accessor) { - auto vertex = *accessor.CreateVertexAndValidate( - primary_label, {}, - {{primary_property, PropertyValue(primary_key_id++)}, {prop_id, PropertyValue(vertex_id++)}}); + auto vertex = *accessor.CreateVertexAndValidate({}, {PropertyValue{primary_key_id++}}, + {{prop_id, PropertyValue{vertex_id++}}}); return vertex; } @@ -702,7 +701,7 @@ TEST_F(IndexTest, LabelPropertyIndexMixedIteration) { { auto acc = storage.Access(GetNextHlc()); for (const auto &value : values) { - auto v = acc.CreateVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(primary_key_id++)}}); + auto v = acc.CreateVertexAndValidate({}, {PropertyValue{primary_key_id++}}, {}); ASSERT_TRUE(v->AddLabelAndValidate(label1).HasValue()); ASSERT_TRUE(v->SetPropertyAndValidate(prop_val, value).HasValue()); } @@ -908,8 +907,7 @@ TEST_F(IndexTest, LabelIndexCreateVertexAndValidate) { // Create vertices with CreateVertexAndValidate for (int i = 0; i < 5; ++i) { - auto vertex = - acc.CreateVertexAndValidate(primary_label, {label1}, {{primary_property, PropertyValue(primary_key_id++)}}); + auto vertex = acc.CreateVertexAndValidate({label1}, {PropertyValue{primary_key_id++}}, {}); ASSERT_TRUE(vertex.HasValue()); } acc.Commit(GetNextHlc()); @@ -935,8 +933,7 @@ TEST_F(IndexTest, LabelIndexCreateVertexAndValidate) { EXPECT_THAT(GetPrimaryKeyIds(acc.Vertices(label1, View::OLD), View::OLD), UnorderedElementsAre(0, 1, 2, 3, 4)); for (int i = 0; i < 5; ++i) { - auto vertex = - acc.CreateVertexAndValidate(primary_label, {label1}, {{primary_property, PropertyValue(primary_key_id++)}}); + auto vertex = acc.CreateVertexAndValidate({label1}, {PropertyValue{primary_key_id++}}, {}); ASSERT_TRUE(vertex.HasValue()); } @@ -956,9 +953,8 @@ TEST_F(IndexTest, LabelPropertyIndexCreateVertexAndValidate) { // Create vertices with CreateVertexAndValidate for (int i = 0; i < 5; ++i) { - auto vertex = acc.CreateVertexAndValidate( - primary_label, {label1}, - {{primary_property, PropertyValue(primary_key_id++)}, {prop_id, PropertyValue(vertex_id++)}}); + auto vertex = acc.CreateVertexAndValidate({label1}, {PropertyValue{primary_key_id++}}, + {{prop_id, PropertyValue(vertex_id++)}}); ASSERT_TRUE(vertex.HasValue()); } acc.Commit(GetNextHlc()); @@ -986,9 +982,8 @@ TEST_F(IndexTest, LabelPropertyIndexCreateVertexAndValidate) { UnorderedElementsAre(0, 1, 2, 3, 4)); for (int i = 0; i < 5; ++i) { - auto vertex = acc.CreateVertexAndValidate( - primary_label, {label1}, - {{primary_property, PropertyValue(primary_key_id++)}, {prop_id, PropertyValue(vertex_id++)}}); + auto vertex = acc.CreateVertexAndValidate({label1}, {PropertyValue{primary_key_id++}}, + {{prop_id, PropertyValue(vertex_id++)}}); ASSERT_TRUE(vertex.HasValue()); } diff --git a/tests/unit/storage_v3_isolation_level.cpp b/tests/unit/storage_v3_isolation_level.cpp index 47b863823..2fe259c00 100644 --- a/tests/unit/storage_v3_isolation_level.cpp +++ b/tests/unit/storage_v3_isolation_level.cpp @@ -99,8 +99,7 @@ TEST_P(StorageIsolationLevelTest, Visibility) { "(default isolation level = {}, override isolation level = {})", IsolationLevelToString(default_isolation_level), IsolationLevelToString(override_isolation_level))); for (auto i{1}; i <= iteration_count; ++i) { - ASSERT_TRUE( - creator.CreateVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue{i}}}).HasValue()); + ASSERT_TRUE(creator.CreateVertexAndValidate({}, {PropertyValue{i}}, {}).HasValue()); const auto check_vertices_count = [i](auto &accessor, const auto isolation_level) { const auto expected_count = isolation_level == IsolationLevel::READ_UNCOMMITTED ? i : 0; diff --git a/tests/unit/storage_v3_schema.cpp b/tests/unit/storage_v3_schema.cpp index d364f4a54..c36b92bc3 100644 --- a/tests/unit/storage_v3_schema.cpp +++ b/tests/unit/storage_v3_schema.cpp @@ -178,86 +178,65 @@ class SchemaValidatorTest : public testing::Test { TEST_F(SchemaValidatorTest, TestSchemaValidateVertexCreate) { // Validate against secondary label { - const auto schema_violation = - schema_validator.ValidateVertexCreate(NameToLabel("test"), {}, {{prop_string, PropertyValue(1)}}); + const auto schema_violation = schema_validator.ValidateVertexCreate(NameToLabel("test"), {}, {PropertyValue(1)}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::NO_SCHEMA_DEFINED_FOR_LABEL, NameToLabel("test"))); } - // Validate missing property + { - const auto schema_violation = schema_validator.ValidateVertexCreate(label1, {}, {{prop_int, PropertyValue(1)}}); - ASSERT_NE(schema_violation, std::nullopt); - EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_HAS_NO_PRIMARY_PROPERTY, - label1, schema_prop_string)); - } - { - const auto schema_violation = - schema_validator.ValidateVertexCreate(label2, {}, std::vector<std::pair<PropertyId, PropertyValue>>{}); - ASSERT_NE(schema_violation, std::nullopt); - EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_HAS_NO_PRIMARY_PROPERTY, - label2, schema_prop_string)); - } - { - const auto schema_violation = schema_validator.ValidateVertexCreate(label2, {}, std::vector<PropertyValue>{}); + const auto schema_violation = schema_validator.ValidateVertexCreate(label2, {}, {}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_PRIMARY_PROPERTIES_UNDEFINED, label2)); } // Validate wrong secondary label { - const auto schema_violation = - schema_validator.ValidateVertexCreate(label1, {label1}, {{prop_string, PropertyValue("test")}}); + const auto schema_violation = schema_validator.ValidateVertexCreate(label1, {label1}, {PropertyValue("test")}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_SECONDARY_LABEL_IS_PRIMARY, label1)); } { - const auto schema_violation = - schema_validator.ValidateVertexCreate(label1, {label2}, {{prop_string, PropertyValue("test")}}); + const auto schema_violation = schema_validator.ValidateVertexCreate(label1, {label2}, {PropertyValue("test")}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_SECONDARY_LABEL_IS_PRIMARY, label2)); } // Validate wrong property type { - const auto schema_violation = schema_validator.ValidateVertexCreate(label1, {}, {{prop_string, PropertyValue(1)}}); + const auto schema_violation = schema_validator.ValidateVertexCreate(label1, {}, {PropertyValue(1)}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_PROPERTY_WRONG_TYPE, label1, schema_prop_string, PropertyValue(1))); } { - const auto schema_violation = schema_validator.ValidateVertexCreate( - label2, {}, - {{prop_string, PropertyValue("test")}, {prop_int, PropertyValue(12)}, {prop_duration, PropertyValue(1)}}); + const auto schema_violation = + schema_validator.ValidateVertexCreate(label2, {}, {PropertyValue("test"), PropertyValue(12), PropertyValue(1)}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_PROPERTY_WRONG_TYPE, label2, schema_prop_duration, PropertyValue(1))); } { const auto wrong_prop = PropertyValue(TemporalData(TemporalType::Date, 1234)); - const auto schema_violation = schema_validator.ValidateVertexCreate( - label2, {}, {{prop_string, PropertyValue("test")}, {prop_int, PropertyValue(12)}, {prop_duration, wrong_prop}}); + const auto schema_violation = + schema_validator.ValidateVertexCreate(label2, {}, {PropertyValue("test"), PropertyValue(12), wrong_prop}); ASSERT_NE(schema_violation, std::nullopt); EXPECT_EQ(*schema_violation, SchemaViolation(SchemaViolation::ValidationStatus::VERTEX_PROPERTY_WRONG_TYPE, label2, schema_prop_duration, wrong_prop)); } // Passing validations - EXPECT_EQ(schema_validator.ValidateVertexCreate(label1, {}, {{prop_string, PropertyValue("test")}}), std::nullopt); + EXPECT_EQ(schema_validator.ValidateVertexCreate(label1, {}, {PropertyValue("test")}), std::nullopt); EXPECT_EQ(schema_validator.ValidateVertexCreate(label1, {NameToLabel("label3"), NameToLabel("label4")}, - {{prop_string, PropertyValue("test")}}), + {PropertyValue("test")}), std::nullopt); EXPECT_EQ(schema_validator.ValidateVertexCreate( label2, {}, - {{prop_string, PropertyValue("test")}, - {prop_int, PropertyValue(122)}, - {prop_duration, PropertyValue(TemporalData(TemporalType::Duration, 1234))}}), + {PropertyValue("test"), PropertyValue(122), PropertyValue(TemporalData(TemporalType::Duration, 1234))}), std::nullopt); - EXPECT_EQ(schema_validator.ValidateVertexCreate( - label2, {NameToLabel("label5"), NameToLabel("label6")}, - {{prop_string, PropertyValue("test123")}, - {prop_int, PropertyValue(122221)}, - {prop_duration, PropertyValue(TemporalData(TemporalType::Duration, 12344321))}}), + EXPECT_EQ(schema_validator.ValidateVertexCreate(label2, {NameToLabel("label5"), NameToLabel("label6")}, + {PropertyValue("test123"), PropertyValue(122221), + PropertyValue(TemporalData(TemporalType::Duration, 12344321))}), std::nullopt); } diff --git a/tests/unit/storage_v3_vertex_accessors.cpp b/tests/unit/storage_v3_vertex_accessors.cpp index 7c4e26dc7..683f58a56 100644 --- a/tests/unit/storage_v3_vertex_accessors.cpp +++ b/tests/unit/storage_v3_vertex_accessors.cpp @@ -34,10 +34,9 @@ class StorageV3Accessor : public ::testing::Test { protected: void SetUp() override { storage.StoreMapping({{1, "label"}, {2, "property"}}); } - VertexAccessor CreateVertexAndValidate(Shard::Accessor &acc, LabelId primary_label, - const std::vector<LabelId> &labels, - const std::vector<std::pair<PropertyId, PropertyValue>> &properties) { - auto vtx = acc.CreateVertexAndValidate(primary_label, labels, properties); + static VertexAccessor CreateVertexAndValidate(Shard::Accessor &acc, const std::vector<LabelId> &labels, + const PropertyValue &primary_key) { + auto vtx = acc.CreateVertexAndValidate(labels, {primary_key}, {}); EXPECT_TRUE(vtx.HasValue()); return *vtx; } @@ -65,7 +64,7 @@ class StorageV3Accessor : public ::testing::Test { TEST_F(StorageV3Accessor, TestPrimaryLabel) { { auto acc = storage.Access(GetNextHlc()); - const auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(0)}}); + const auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}); ASSERT_TRUE(vertex.PrimaryLabel(View::NEW).HasValue()); const auto vertex_primary_label = vertex.PrimaryLabel(View::NEW).GetValue(); ASSERT_FALSE(vertex.PrimaryLabel(View::OLD).HasValue()); @@ -73,7 +72,7 @@ TEST_F(StorageV3Accessor, TestPrimaryLabel) { } { auto acc = storage.Access(GetNextHlc()); - const auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(1)}}); + const auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{1}); ASSERT_TRUE(vertex.PrimaryLabel(View::OLD).HasError()); const auto error_primary_label = vertex.PrimaryLabel(View::OLD).GetError(); ASSERT_FALSE(vertex.PrimaryLabel(View::NEW).HasError()); @@ -81,7 +80,7 @@ TEST_F(StorageV3Accessor, TestPrimaryLabel) { } { auto acc = storage.Access(GetNextHlc()); - CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(2)}}); + CreateVertexAndValidate(acc, {}, PropertyValue{2}); acc.Commit(GetNextHlc()); } { @@ -103,8 +102,7 @@ TEST_F(StorageV3Accessor, TestAddLabels) { const auto label1 = NameToLabelId("label1"); const auto label2 = NameToLabelId("label2"); const auto label3 = NameToLabelId("label3"); - const auto vertex = - CreateVertexAndValidate(acc, primary_label, {label1, label2, label3}, {{primary_property, PropertyValue(0)}}); + const auto vertex = CreateVertexAndValidate(acc, {label1, label2, label3}, PropertyValue{0}); ASSERT_TRUE(vertex.Labels(View::NEW).HasValue()); ASSERT_FALSE(vertex.Labels(View::OLD).HasValue()); EXPECT_THAT(vertex.Labels(View::NEW).GetValue(), UnorderedElementsAre(label1, label2, label3)); @@ -114,7 +112,7 @@ TEST_F(StorageV3Accessor, TestAddLabels) { const auto label1 = NameToLabelId("label1"); const auto label2 = NameToLabelId("label2"); const auto label3 = NameToLabelId("label3"); - auto vertex = CreateVertexAndValidate(acc, primary_label, {label1}, {{primary_property, PropertyValue(1)}}); + auto vertex = CreateVertexAndValidate(acc, {label1}, PropertyValue{1}); ASSERT_TRUE(vertex.Labels(View::NEW).HasValue()); ASSERT_FALSE(vertex.Labels(View::OLD).HasValue()); EXPECT_THAT(vertex.Labels(View::NEW).GetValue(), UnorderedElementsAre(label1)); @@ -127,7 +125,7 @@ TEST_F(StorageV3Accessor, TestAddLabels) { { auto acc = storage.Access(GetNextHlc()); const auto label1 = NameToLabelId("label"); - auto vertex = acc.CreateVertexAndValidate(primary_label, {label1}, {{primary_property, PropertyValue(2)}}); + auto vertex = acc.CreateVertexAndValidate({label1}, {PropertyValue{2}}, {}); ASSERT_TRUE(vertex.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(vertex.GetError())); EXPECT_EQ(std::get<SchemaViolation>(vertex.GetError()), @@ -136,7 +134,7 @@ TEST_F(StorageV3Accessor, TestAddLabels) { { auto acc = storage.Access(GetNextHlc()); const auto label1 = NameToLabelId("label"); - auto vertex = acc.CreateVertexAndValidate(primary_label, {}, {{primary_property, PropertyValue(3)}}); + auto vertex = acc.CreateVertexAndValidate({}, {PropertyValue{3}}, {}); ASSERT_TRUE(vertex.HasValue()); const auto schema_violation = vertex->AddLabelAndValidate(label1); ASSERT_TRUE(schema_violation.HasError()); @@ -154,8 +152,7 @@ TEST_F(StorageV3Accessor, TestRemoveLabels) { const auto label1 = NameToLabelId("label1"); const auto label2 = NameToLabelId("label2"); const auto label3 = NameToLabelId("label3"); - auto vertex = - CreateVertexAndValidate(acc, primary_label, {label1, label2, label3}, {{primary_property, PropertyValue(0)}}); + auto vertex = CreateVertexAndValidate(acc, {label1, label2, label3}, PropertyValue{0}); ASSERT_TRUE(vertex.Labels(View::NEW).HasValue()); EXPECT_THAT(vertex.Labels(View::NEW).GetValue(), UnorderedElementsAre(label1, label2, label3)); const auto res1 = vertex.RemoveLabelAndValidate(label2); @@ -175,7 +172,7 @@ TEST_F(StorageV3Accessor, TestRemoveLabels) { { auto acc = storage.Access(GetNextHlc()); const auto label1 = NameToLabelId("label1"); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(1)}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{1}); ASSERT_TRUE(vertex.Labels(View::NEW).HasValue()); EXPECT_TRUE(vertex.Labels(View::NEW).GetValue().empty()); const auto res1 = vertex.RemoveLabelAndValidate(label1); @@ -184,7 +181,7 @@ TEST_F(StorageV3Accessor, TestRemoveLabels) { } { auto acc = storage.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(2)}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{2}); const auto res1 = vertex.RemoveLabelAndValidate(primary_label); ASSERT_TRUE(res1.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(res1.GetError())); @@ -199,13 +196,13 @@ TEST_F(StorageV3Accessor, TestSetKeysAndProperties) { { auto acc = storage.Access(GetNextHlc()); const PropertyId prop1{NameToPropertyId("prop1")}; - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(0)}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{0}); const auto res = vertex.SetPropertyAndValidate(prop1, PropertyValue(1)); ASSERT_TRUE(res.HasValue()); } { auto acc = storage.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(1)}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{1}); const auto res = vertex.SetPropertyAndValidate(primary_property, PropertyValue(1)); ASSERT_TRUE(res.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(res.GetError())); @@ -215,7 +212,7 @@ TEST_F(StorageV3Accessor, TestSetKeysAndProperties) { } { auto acc = storage.Access(GetNextHlc()); - auto vertex = CreateVertexAndValidate(acc, primary_label, {}, {{primary_property, PropertyValue(2)}}); + auto vertex = CreateVertexAndValidate(acc, {}, PropertyValue{2}); const auto res = vertex.SetPropertyAndValidate(primary_property, PropertyValue()); ASSERT_TRUE(res.HasError()); ASSERT_TRUE(std::holds_alternative<SchemaViolation>(res.GetError()));