From 1cee7ecb8ae0698bfdab13fe3c8c1850a96e81c8 Mon Sep 17 00:00:00 2001 From: gvolfing Date: Fri, 4 Nov 2022 08:12:37 +0100 Subject: [PATCH 1/5] Make ShardRsm aware of trying to write the same vertex into the skip-list --- src/storage/v3/schema_validator.hpp | 4 +++- src/storage/v3/shard.cpp | 6 +++++- src/storage/v3/shard_rsm.cpp | 8 ++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/storage/v3/schema_validator.hpp b/src/storage/v3/schema_validator.hpp index d57dfae2e..ad6f4e680 100644 --- a/src/storage/v3/schema_validator.hpp +++ b/src/storage/v3/schema_validator.hpp @@ -79,7 +79,9 @@ struct VertexValidator { LabelId primary_label_; }; +struct AlreadyInsertedElement {}; + template -using ResultSchema = utils::BasicResult, TValue>; +using ResultSchema = utils::BasicResult, TValue>; } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index d6ba56ac3..09dd91661 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -361,7 +361,11 @@ ResultSchema Shard::Accessor::CreateVertexAndValidate( 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!"); + // Vertex is allready inserted, return an error. + if (!inserted) { + return {AlreadyInsertedElement{}}; + } + // MG_ASSERT(inserted, "The vertex must be inserted here!"); MG_ASSERT(it != acc.end(), "Invalid Vertex accessor!"); // TODO(jbajic) Improve, maybe delay index update diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index 9a124b250..bdd6e7b81 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -498,6 +498,8 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { spdlog::debug("Creating vertex failed with error: SchemaViolation"); } else if constexpr (std::is_same_v) { spdlog::debug("Creating vertex failed with error: Error"); + } else if constexpr (std::is_same_v) { + spdlog::debug("Updating vertex failed with error: AlreadyInsertedElement"); } else { static_assert(kAlwaysFalse, "Missing type from variant visitor"); } @@ -540,17 +542,19 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::UpdateVerticesRequest &&req) { [&action_successful](T &&) { using ErrorType = std::remove_cvref_t; if constexpr (std::is_same_v) { - action_successful = false; spdlog::debug("Updating vertex failed with error: SchemaViolation"); } else if constexpr (std::is_same_v) { - action_successful = false; spdlog::debug("Updating vertex failed with error: Error"); + } else if constexpr (std::is_same_v) { + spdlog::debug("Updating vertex failed with error: AlreadyInsertedElement"); } else { static_assert(kAlwaysFalse, "Missing type from variant visitor"); } }, error); + action_successful = false; + break; } } From 3d954e7abcc998750039c1a44df7f313d3d873f8 Mon Sep 17 00:00:00 2001 From: gvolfing Date: Fri, 4 Nov 2022 15:04:25 +0100 Subject: [PATCH 2/5] Restructure SchemaResult type and uts usage Rename SchemaResult to ShardOperationResult move it into a separate header and add a new type to the underlying variant that indicates that the vertex, the user would like to insert into the skip-list already exist. --- src/query/v2/common.hpp | 4 ++-- src/query/v2/db_accessor.hpp | 21 +++++++++-------- src/storage/v3/schema_validator.hpp | 5 ---- src/storage/v3/shard.cpp | 5 ++-- src/storage/v3/shard.hpp | 3 ++- src/storage/v3/shard_operation_result.hpp | 28 +++++++++++++++++++++++ src/storage/v3/shard_rsm.cpp | 2 +- src/storage/v3/vertex_accessor.cpp | 9 ++++---- src/storage/v3/vertex_accessor.hpp | 8 +++---- tests/unit/storage_v3_edge.cpp | 3 ++- 10 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 src/storage/v3/shard_operation_result.hpp diff --git a/src/query/v2/common.hpp b/src/query/v2/common.hpp index 5637a27bc..b76ce0af0 100644 --- a/src/query/v2/common.hpp +++ b/src/query/v2/common.hpp @@ -28,7 +28,7 @@ #include "storage/v3/id_types.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/result.hpp" -#include "storage/v3/schema_validator.hpp" +#include "storage/v3/shard_operation_result.hpp" #include "storage/v3/view.hpp" #include "utils/exceptions.hpp" #include "utils/logging.hpp" @@ -93,7 +93,7 @@ concept AccessorWithSetPropertyAndValidate = requires(T accessor, const storage: const storage::v3::PropertyValue new_value) { { accessor.SetPropertyAndValidate(key, new_value) - } -> std::same_as>; + } -> std::same_as>; }; template diff --git a/src/query/v2/db_accessor.hpp b/src/query/v2/db_accessor.hpp index a975a70ce..b76da6131 100644 --- a/src/query/v2/db_accessor.hpp +++ b/src/query/v2/db_accessor.hpp @@ -23,6 +23,7 @@ #include "storage/v3/key_store.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/result.hpp" +#include "storage/v3/shard_operation_result.hpp" /////////////////////////////////////////////////////////// // Our communication layer and query engine don't mix @@ -113,17 +114,19 @@ class VertexAccessor final { auto PrimaryKey(storage::v3::View view) const { return impl_.PrimaryKey(view); } - storage::v3::ResultSchema AddLabel(storage::v3::LabelId label) { return impl_.AddLabelAndValidate(label); } - - storage::v3::ResultSchema AddLabelAndValidate(storage::v3::LabelId label) { + storage::v3::ShardOperationResult AddLabel(storage::v3::LabelId label) { return impl_.AddLabelAndValidate(label); } - storage::v3::ResultSchema RemoveLabel(storage::v3::LabelId label) { + storage::v3::ShardOperationResult AddLabelAndValidate(storage::v3::LabelId label) { + return impl_.AddLabelAndValidate(label); + } + + storage::v3::ShardOperationResult RemoveLabel(storage::v3::LabelId label) { return impl_.RemoveLabelAndValidate(label); } - storage::v3::ResultSchema RemoveLabelAndValidate(storage::v3::LabelId label) { + storage::v3::ShardOperationResult RemoveLabelAndValidate(storage::v3::LabelId label) { return impl_.RemoveLabelAndValidate(label); } @@ -138,17 +141,17 @@ class VertexAccessor final { return impl_.GetProperty(key, view); } - storage::v3::ResultSchema SetProperty(storage::v3::PropertyId key, - const storage::v3::PropertyValue &value) { + storage::v3::ShardOperationResult SetProperty(storage::v3::PropertyId key, + const storage::v3::PropertyValue &value) { return impl_.SetPropertyAndValidate(key, value); } - storage::v3::ResultSchema SetPropertyAndValidate( + storage::v3::ShardOperationResult SetPropertyAndValidate( storage::v3::PropertyId key, const storage::v3::PropertyValue &value) { return impl_.SetPropertyAndValidate(key, value); } - storage::v3::ResultSchema RemovePropertyAndValidate(storage::v3::PropertyId key) { + storage::v3::ShardOperationResult RemovePropertyAndValidate(storage::v3::PropertyId key) { return SetPropertyAndValidate(key, storage::v3::PropertyValue{}); } diff --git a/src/storage/v3/schema_validator.hpp b/src/storage/v3/schema_validator.hpp index ad6f4e680..ce813fde2 100644 --- a/src/storage/v3/schema_validator.hpp +++ b/src/storage/v3/schema_validator.hpp @@ -79,9 +79,4 @@ struct VertexValidator { LabelId primary_label_; }; -struct AlreadyInsertedElement {}; - -template -using ResultSchema = utils::BasicResult, TValue>; - } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index 09dd91661..942ce65b2 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -33,6 +33,7 @@ #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/schema_validator.hpp" +#include "storage/v3/shard_operation_result.hpp" #include "storage/v3/transaction.hpp" #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" @@ -343,7 +344,7 @@ Shard::~Shard() {} Shard::Accessor::Accessor(Shard &shard, Transaction &transaction) : shard_(&shard), transaction_(&transaction), config_(shard_->config_.items) {} -ResultSchema Shard::Accessor::CreateVertexAndValidate( +ShardOperationResult Shard::Accessor::CreateVertexAndValidate( const std::vector &labels, const std::vector &primary_properties, const std::vector> &properties) { OOMExceptionEnabler oom_exception; @@ -361,11 +362,9 @@ ResultSchema Shard::Accessor::CreateVertexAndValidate( delta->prev.Set(&it->vertex); VertexAccessor vertex_acc{&it->vertex, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; - // Vertex is allready inserted, return an error. if (!inserted) { return {AlreadyInsertedElement{}}; } - // MG_ASSERT(inserted, "The vertex must be inserted here!"); MG_ASSERT(it != acc.end(), "Invalid Vertex accessor!"); // TODO(jbajic) Improve, maybe delay index update diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index a3f1fc90d..778c92abd 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -38,6 +38,7 @@ #include "storage/v3/result.hpp" #include "storage/v3/schema_validator.hpp" #include "storage/v3/schemas.hpp" +#include "storage/v3/shard_operation_result.hpp" #include "storage/v3/transaction.hpp" #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" @@ -206,7 +207,7 @@ class Shard final { public: /// @throw std::bad_alloc - ResultSchema CreateVertexAndValidate( + ShardOperationResult CreateVertexAndValidate( const std::vector &labels, const std::vector &primary_properties, const std::vector> &properties); diff --git a/src/storage/v3/shard_operation_result.hpp b/src/storage/v3/shard_operation_result.hpp new file mode 100644 index 000000000..e055a0022 --- /dev/null +++ b/src/storage/v3/shard_operation_result.hpp @@ -0,0 +1,28 @@ +// Copyright 2022 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#pragma once + +#include + +#include "storage/v3/result.hpp" +#include "storage/v3/schema_validator.hpp" + +namespace memgraph::storage::v3 { + +struct AlreadyInsertedElement {}; + +using ResultErrorType = std::variant; + +template +using ShardOperationResult = utils::BasicResult; + +} // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index bdd6e7b81..faae52f95 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -539,7 +539,7 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::UpdateVerticesRequest &&req) { auto &error = result_schema.GetError(); std::visit( - [&action_successful](T &&) { + [](T &&) { using ErrorType = std::remove_cvref_t; if constexpr (std::is_same_v) { spdlog::debug("Updating vertex failed with error: SchemaViolation"); diff --git a/src/storage/v3/vertex_accessor.cpp b/src/storage/v3/vertex_accessor.cpp index 696c4657a..543caa88a 100644 --- a/src/storage/v3/vertex_accessor.cpp +++ b/src/storage/v3/vertex_accessor.cpp @@ -21,8 +21,8 @@ #include "storage/v3/key_store.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" -#include "storage/v3/schema_validator.hpp" #include "storage/v3/shard.hpp" +#include "storage/v3/shard_operation_result.hpp" #include "storage/v3/vertex.hpp" #include "utils/logging.hpp" #include "utils/memory_tracker.hpp" @@ -98,7 +98,7 @@ Result VertexAccessor::AddLabel(LabelId label) { return true; } -ResultSchema VertexAccessor::AddLabelAndValidate(LabelId label) { +ShardOperationResult VertexAccessor::AddLabelAndValidate(LabelId label) { if (const auto maybe_violation_error = vertex_validator_->ValidateAddLabel(label); maybe_violation_error) { return {*maybe_violation_error}; } @@ -134,7 +134,7 @@ Result VertexAccessor::RemoveLabel(LabelId label) { return true; } -ResultSchema VertexAccessor::RemoveLabelAndValidate(LabelId label) { +ShardOperationResult VertexAccessor::RemoveLabelAndValidate(LabelId label) { if (const auto maybe_violation_error = vertex_validator_->ValidateRemoveLabel(label); maybe_violation_error) { return {*maybe_violation_error}; } @@ -331,7 +331,8 @@ Result VertexAccessor::CheckVertexExistence(View view) const { return {}; } -ResultSchema VertexAccessor::SetPropertyAndValidate(PropertyId property, const PropertyValue &value) { +ShardOperationResult VertexAccessor::SetPropertyAndValidate(PropertyId property, + const PropertyValue &value) { if (auto maybe_violation_error = vertex_validator_->ValidatePropertyUpdate(property); maybe_violation_error) { return {*maybe_violation_error}; } diff --git a/src/storage/v3/vertex_accessor.hpp b/src/storage/v3/vertex_accessor.hpp index a2bf8352f..682a04e39 100644 --- a/src/storage/v3/vertex_accessor.hpp +++ b/src/storage/v3/vertex_accessor.hpp @@ -17,7 +17,7 @@ #include "storage/v3/id_types.hpp" #include "storage/v3/key_store.hpp" #include "storage/v3/result.hpp" -#include "storage/v3/schema_validator.hpp" +#include "storage/v3/shard_operation_result.hpp" #include "storage/v3/transaction.hpp" #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_id.hpp" @@ -55,13 +55,13 @@ class VertexAccessor final { /// `false` is returned if the label already existed, or SchemaViolation /// if adding the label has violated one of the schema constraints. /// @throw std::bad_alloc - ResultSchema AddLabelAndValidate(LabelId label); + ShardOperationResult AddLabelAndValidate(LabelId label); /// Remove a label and return `true` if deletion took place. /// `false` is returned if the vertex did not have a label already. or SchemaViolation /// if adding the label has violated one of the schema constraints. /// @throw std::bad_alloc - ResultSchema RemoveLabelAndValidate(LabelId label); + ShardOperationResult RemoveLabelAndValidate(LabelId label); Result HasLabel(View view, LabelId label) const; @@ -80,7 +80,7 @@ class VertexAccessor final { /// Set a property value and return the old value or error. /// @throw std::bad_alloc - ResultSchema SetPropertyAndValidate(PropertyId property, const PropertyValue &value); + ShardOperationResult SetPropertyAndValidate(PropertyId property, const PropertyValue &value); /// Remove all properties and return the values of the removed properties. /// @throw std::bad_alloc diff --git a/tests/unit/storage_v3_edge.cpp b/tests/unit/storage_v3_edge.cpp index 293f561df..21c4214ce 100644 --- a/tests/unit/storage_v3_edge.cpp +++ b/tests/unit/storage_v3_edge.cpp @@ -18,6 +18,7 @@ #include "storage/v3/name_id_mapper.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/shard.hpp" +#include "storage/v3/shard_operation_result.hpp" namespace memgraph::storage::v3::tests { using testing::UnorderedElementsAre; @@ -38,7 +39,7 @@ class StorageEdgeTest : public ::testing::TestWithParam { return store.NameToEdgeType(edge_type_name); } - static ResultSchema CreateVertex(Shard::Accessor &acc, const PropertyValue &key) { + static ShardOperationResult CreateVertex(Shard::Accessor &acc, const PropertyValue &key) { return acc.CreateVertexAndValidate({}, {key}, {}); } From 39b40ecf00eb31befdbe14c44ca8af57361b57c4 Mon Sep 17 00:00:00 2001 From: gvolfing Date: Mon, 7 Nov 2022 10:00:34 +0100 Subject: [PATCH 3/5] Extend the Error enum instead of a separate type The error representing that a vertex is already inserted into the skip-list was represented by the struct AlreadyInseertedElement. Instead of using that struct, extend the memgraph::storage::v3::Error scoped enum and use that to represent the double-insertion error. --- src/expr/interpret/eval.hpp | 3 ++ src/memgraph.cpp | 2 + src/query/v2/common.hpp | 2 + src/storage/v3/result.hpp | 1 + src/storage/v3/shard.cpp | 2 +- src/storage/v3/shard_operation_result.hpp | 4 +- src/storage/v3/shard_rsm.cpp | 50 +++++++++++++++++++---- 7 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/expr/interpret/eval.hpp b/src/expr/interpret/eval.hpp index 1538028a0..70127c023 100644 --- a/src/expr/interpret/eval.hpp +++ b/src/expr/interpret/eval.hpp @@ -404,6 +404,7 @@ class ExpressionEvaluator : public ExpressionVisitor { case Error::SERIALIZATION_ERROR: case Error::VERTEX_HAS_EDGES: case Error::PROPERTIES_DISABLED: + case Error::VERTEX_ALREADY_INSERTED: throw ExpressionRuntimeException("Unexpected error when accessing labels."); } } @@ -751,6 +752,7 @@ class ExpressionEvaluator : public ExpressionVisitor { case Error::SERIALIZATION_ERROR: case Error::VERTEX_HAS_EDGES: case Error::PROPERTIES_DISABLED: + case Error::VERTEX_ALREADY_INSERTED: throw ExpressionRuntimeException("Unexpected error when getting a property."); } } @@ -779,6 +781,7 @@ class ExpressionEvaluator : public ExpressionVisitor { case Error::SERIALIZATION_ERROR: case Error::VERTEX_HAS_EDGES: case Error::PROPERTIES_DISABLED: + case Error::VERTEX_ALREADY_INSERTED: throw ExpressionRuntimeException("Unexpected error when getting a property."); } } diff --git a/src/memgraph.cpp b/src/memgraph.cpp index 0f80266e5..deaf9e568 100644 --- a/src/memgraph.cpp +++ b/src/memgraph.cpp @@ -489,6 +489,7 @@ class BoltSession final : public memgraph::communication::bolt::Session diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index 942ce65b2..0e596172d 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -363,7 +363,7 @@ ShardOperationResult Shard::Accessor::CreateVertexAndValidate( VertexAccessor vertex_acc{&it->vertex, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; if (!inserted) { - return {AlreadyInsertedElement{}}; + return {Error::VERTEX_ALREADY_INSERTED}; } MG_ASSERT(it != acc.end(), "Invalid Vertex accessor!"); diff --git a/src/storage/v3/shard_operation_result.hpp b/src/storage/v3/shard_operation_result.hpp index e055a0022..8de800b83 100644 --- a/src/storage/v3/shard_operation_result.hpp +++ b/src/storage/v3/shard_operation_result.hpp @@ -18,9 +18,7 @@ namespace memgraph::storage::v3 { -struct AlreadyInsertedElement {}; - -using ResultErrorType = std::variant; +using ResultErrorType = std::variant; template using ShardOperationResult = utils::BasicResult; diff --git a/src/storage/v3/shard_rsm.cpp b/src/storage/v3/shard_rsm.cpp index faae52f95..2911e26d6 100644 --- a/src/storage/v3/shard_rsm.cpp +++ b/src/storage/v3/shard_rsm.cpp @@ -492,14 +492,31 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CreateVerticesRequest &&req) { auto &error = result_schema.GetError(); std::visit( - [](T &&) { + [](T &&error) { using ErrorType = std::remove_cvref_t; if constexpr (std::is_same_v) { spdlog::debug("Creating vertex failed with error: SchemaViolation"); } else if constexpr (std::is_same_v) { - spdlog::debug("Creating vertex failed with error: Error"); - } else if constexpr (std::is_same_v) { - spdlog::debug("Updating vertex failed with error: AlreadyInsertedElement"); + switch (error) { + case Error::DELETED_OBJECT: + spdlog::debug("Creating vertex failed with error: DELETED_OBJECT"); + break; + case Error::NONEXISTENT_OBJECT: + spdlog::debug("Creating vertex failed with error: NONEXISTENT_OBJECT"); + break; + case Error::SERIALIZATION_ERROR: + spdlog::debug("Creating vertex failed with error: SERIALIZATION_ERROR"); + break; + case Error::PROPERTIES_DISABLED: + spdlog::debug("Creating vertex failed with error: PROPERTIES_DISABLED"); + break; + case Error::VERTEX_HAS_EDGES: + spdlog::debug("Creating vertex failed with error: VERTEX_HAS_EDGES"); + break; + case Error::VERTEX_ALREADY_INSERTED: + spdlog::debug("Creating vertex failed with error: VERTEX_ALREADY_INSERTED"); + break; + } } else { static_assert(kAlwaysFalse, "Missing type from variant visitor"); } @@ -539,14 +556,31 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::UpdateVerticesRequest &&req) { auto &error = result_schema.GetError(); std::visit( - [](T &&) { + [](T &&error) { using ErrorType = std::remove_cvref_t; if constexpr (std::is_same_v) { spdlog::debug("Updating vertex failed with error: SchemaViolation"); } else if constexpr (std::is_same_v) { - spdlog::debug("Updating vertex failed with error: Error"); - } else if constexpr (std::is_same_v) { - spdlog::debug("Updating vertex failed with error: AlreadyInsertedElement"); + switch (error) { + case Error::DELETED_OBJECT: + spdlog::debug("Updating vertex failed with error: DELETED_OBJECT"); + break; + case Error::NONEXISTENT_OBJECT: + spdlog::debug("Updating vertex failed with error: NONEXISTENT_OBJECT"); + break; + case Error::SERIALIZATION_ERROR: + spdlog::debug("Updating vertex failed with error: SERIALIZATION_ERROR"); + break; + case Error::PROPERTIES_DISABLED: + spdlog::debug("Updating vertex failed with error: PROPERTIES_DISABLED"); + break; + case Error::VERTEX_HAS_EDGES: + spdlog::debug("Updating vertex failed with error: VERTEX_HAS_EDGES"); + break; + case Error::VERTEX_ALREADY_INSERTED: + spdlog::debug("Updating vertex failed with error: VERTEX_ALREADY_INSERTED"); + break; + } } else { static_assert(kAlwaysFalse, "Missing type from variant visitor"); } From 91550128a59d336a8bc1a01d4ec53e28bcdcc425 Mon Sep 17 00:00:00 2001 From: gvolfing Date: Mon, 7 Nov 2022 11:46:24 +0100 Subject: [PATCH 4/5] Conform unit test with the new error-handling --- tests/unit/storage_v3.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/unit/storage_v3.cpp b/tests/unit/storage_v3.cpp index b62606a8e..3098acbdb 100644 --- a/tests/unit/storage_v3.cpp +++ b/tests/unit/storage_v3.cpp @@ -33,6 +33,12 @@ using testing::UnorderedElementsAre; +namespace { + +class AlreadyInsertedException : public std::exception {}; + +} // namespace + namespace memgraph::storage::v3::tests { class StorageV3 : public ::testing::TestWithParam { @@ -2650,14 +2656,21 @@ TEST_P(StorageV3, TestCreateVertexAndValidate) { (std::map{{prop1, PropertyValue(111)}})); } { - ASSERT_DEATH( + EXPECT_THROW( { Shard store(primary_label, min_pk, std::nullopt /*max_primary_key*/, schema_property_vector); auto acc = store.Access(GetNextHlc()); auto vertex1 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); auto vertex2 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); + + if (vertex2.HasError()) { + auto error = vertex2.GetError(); + if (auto error_ptr = std::get_if(&error)) { + if (*error_ptr == storage::v3::Error::VERTEX_ALREADY_INSERTED) throw AlreadyInsertedException(); + } + } }, - ""); + AlreadyInsertedException); } { auto acc = store.Access(GetNextHlc()); From 79756ae6fbf58453733f5ce8a6ec4ee54b276362 Mon Sep 17 00:00:00 2001 From: gvolfing Date: Tue, 8 Nov 2022 07:31:01 +0100 Subject: [PATCH 5/5] Modify unit test Instead of Creating an exception that is would be only used in this file, just assert the type of the error the double vertex insertion operation should yield. --- tests/unit/storage_v3.cpp | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/tests/unit/storage_v3.cpp b/tests/unit/storage_v3.cpp index 3098acbdb..b107786e8 100644 --- a/tests/unit/storage_v3.cpp +++ b/tests/unit/storage_v3.cpp @@ -33,12 +33,6 @@ using testing::UnorderedElementsAre; -namespace { - -class AlreadyInsertedException : public std::exception {}; - -} // namespace - namespace memgraph::storage::v3::tests { class StorageV3 : public ::testing::TestWithParam { @@ -2656,21 +2650,16 @@ TEST_P(StorageV3, TestCreateVertexAndValidate) { (std::map{{prop1, PropertyValue(111)}})); } { - EXPECT_THROW( - { - Shard store(primary_label, min_pk, std::nullopt /*max_primary_key*/, schema_property_vector); - auto acc = store.Access(GetNextHlc()); - auto vertex1 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); - auto vertex2 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); + Shard store(primary_label, min_pk, std::nullopt /*max_primary_key*/, schema_property_vector); + auto acc = store.Access(GetNextHlc()); + auto vertex1 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); + auto vertex2 = acc.CreateVertexAndValidate({}, {PropertyValue{0}}, {}); - if (vertex2.HasError()) { - auto error = vertex2.GetError(); - if (auto error_ptr = std::get_if(&error)) { - if (*error_ptr == storage::v3::Error::VERTEX_ALREADY_INSERTED) throw AlreadyInsertedException(); - } - } - }, - AlreadyInsertedException); + ASSERT_TRUE(vertex2.HasError()); + auto error = vertex2.GetError(); + auto error_ptr = std::get_if(&error); + ASSERT_TRUE(error_ptr); + ASSERT_TRUE(*error_ptr == storage::v3::Error::VERTEX_ALREADY_INSERTED); } { auto acc = store.Access(GetNextHlc());