From 3d954e7abcc998750039c1a44df7f313d3d873f8 Mon Sep 17 00:00:00 2001
From: gvolfing <gabor.volfinger@memgraph.io>
Date: Fri, 4 Nov 2022 15:04:25 +0100
Subject: [PATCH] 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<storage::v3::ResultSchema<storage::v3::PropertyValue>>;
+ } -> std::same_as<storage::v3::ShardOperationResult<storage::v3::PropertyValue>>;
};
template <typename TRecordAccessor>
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<bool> AddLabel(storage::v3::LabelId label) { return impl_.AddLabelAndValidate(label); }
-
- storage::v3::ResultSchema<bool> AddLabelAndValidate(storage::v3::LabelId label) {
+ storage::v3::ShardOperationResult<bool> AddLabel(storage::v3::LabelId label) {
return impl_.AddLabelAndValidate(label);
}
- storage::v3::ResultSchema<bool> RemoveLabel(storage::v3::LabelId label) {
+ storage::v3::ShardOperationResult<bool> AddLabelAndValidate(storage::v3::LabelId label) {
+ return impl_.AddLabelAndValidate(label);
+ }
+
+ storage::v3::ShardOperationResult<bool> RemoveLabel(storage::v3::LabelId label) {
return impl_.RemoveLabelAndValidate(label);
}
- storage::v3::ResultSchema<bool> RemoveLabelAndValidate(storage::v3::LabelId label) {
+ storage::v3::ShardOperationResult<bool> RemoveLabelAndValidate(storage::v3::LabelId label) {
return impl_.RemoveLabelAndValidate(label);
}
@@ -138,17 +141,17 @@ class VertexAccessor final {
return impl_.GetProperty(key, view);
}
- storage::v3::ResultSchema<storage::v3::PropertyValue> SetProperty(storage::v3::PropertyId key,
- const storage::v3::PropertyValue &value) {
+ storage::v3::ShardOperationResult<storage::v3::PropertyValue> SetProperty(storage::v3::PropertyId key,
+ const storage::v3::PropertyValue &value) {
return impl_.SetPropertyAndValidate(key, value);
}
- storage::v3::ResultSchema<storage::v3::PropertyValue> SetPropertyAndValidate(
+ storage::v3::ShardOperationResult<storage::v3::PropertyValue> SetPropertyAndValidate(
storage::v3::PropertyId key, const storage::v3::PropertyValue &value) {
return impl_.SetPropertyAndValidate(key, value);
}
- storage::v3::ResultSchema<storage::v3::PropertyValue> RemovePropertyAndValidate(storage::v3::PropertyId key) {
+ storage::v3::ShardOperationResult<storage::v3::PropertyValue> 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 <typename TValue>
-using ResultSchema = utils::BasicResult<std::variant<SchemaViolation, AlreadyInsertedElement, Error>, 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<VertexAccessor> Shard::Accessor::CreateVertexAndValidate(
+ShardOperationResult<VertexAccessor> Shard::Accessor::CreateVertexAndValidate(
const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties,
const std::vector<std::pair<PropertyId, PropertyValue>> &properties) {
OOMExceptionEnabler oom_exception;
@@ -361,11 +362,9 @@ ResultSchema<VertexAccessor> 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<VertexAccessor> CreateVertexAndValidate(
+ ShardOperationResult<VertexAccessor> CreateVertexAndValidate(
const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties,
const std::vector<std::pair<PropertyId, PropertyValue>> &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 <variant>
+
+#include "storage/v3/result.hpp"
+#include "storage/v3/schema_validator.hpp"
+
+namespace memgraph::storage::v3 {
+
+struct AlreadyInsertedElement {};
+
+using ResultErrorType = std::variant<SchemaViolation, AlreadyInsertedElement, Error>;
+
+template <typename TValue>
+using ShardOperationResult = utils::BasicResult<ResultErrorType, TValue>;
+
+} // 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]<typename T>(T &&) {
+ []<typename T>(T &&) {
using ErrorType = std::remove_cvref_t<T>;
if constexpr (std::is_same_v<ErrorType, SchemaViolation>) {
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<bool> VertexAccessor::AddLabel(LabelId label) {
return true;
}
-ResultSchema<bool> VertexAccessor::AddLabelAndValidate(LabelId label) {
+ShardOperationResult<bool> 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<bool> VertexAccessor::RemoveLabel(LabelId label) {
return true;
}
-ResultSchema<bool> VertexAccessor::RemoveLabelAndValidate(LabelId label) {
+ShardOperationResult<bool> 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<void> VertexAccessor::CheckVertexExistence(View view) const {
return {};
}
-ResultSchema<PropertyValue> VertexAccessor::SetPropertyAndValidate(PropertyId property, const PropertyValue &value) {
+ShardOperationResult<PropertyValue> 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<bool> AddLabelAndValidate(LabelId label);
+ ShardOperationResult<bool> 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<bool> RemoveLabelAndValidate(LabelId label);
+ ShardOperationResult<bool> RemoveLabelAndValidate(LabelId label);
Result<bool> 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<PropertyValue> SetPropertyAndValidate(PropertyId property, const PropertyValue &value);
+ ShardOperationResult<PropertyValue> 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<bool> {
return store.NameToEdgeType(edge_type_name);
}
- static ResultSchema<VertexAccessor> CreateVertex(Shard::Accessor &acc, const PropertyValue &key) {
+ static ShardOperationResult<VertexAccessor> CreateVertex(Shard::Accessor &acc, const PropertyValue &key) {
return acc.CreateVertexAndValidate({}, {key}, {});
}