From ed814f3188b07c4ee80066ab3c6382c2241531bb Mon Sep 17 00:00:00 2001
From: jbajic <jure.bajic@memgraph.com>
Date: Fri, 24 Jun 2022 13:39:22 +0200
Subject: [PATCH] Add verification on creation and deletion
---
.../frontend/ast/cypher_main_visitor.cpp | 1 -
src/query/interpreter.cpp | 23 ++++++++-----------
src/storage/v2/schemas.cpp | 12 +++++-----
src/storage/v2/schemas.hpp | 5 ++++
src/storage/v2/storage.cpp | 2 +-
src/storage/v2/storage.hpp | 2 +-
6 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp
index 7ec128ae7..1f7781557 100644
--- a/src/query/frontend/ast/cypher_main_visitor.cpp
+++ b/src/query/frontend/ast/cypher_main_visitor.cpp
@@ -2346,7 +2346,6 @@ antlrcpp::Any CypherMainVisitor::visitSchemaQuery(MemgraphCypher::SchemaQueryCon
}
antlrcpp::Any CypherMainVisitor::visitShowSchema(MemgraphCypher::ShowSchemaContext *ctx) {
- MG_ASSERT(ctx->children.size() == 1, "CreateSchemaQuery should have exactly one child!");
auto *schema_query = storage_->Create<SchemaQuery>();
schema_query->action_ = SchemaQuery::Action::SHOW_SCHEMA;
schema_query->label_ = AddLabel(ctx->labelName()->accept(this));
diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp
index aa40236d9..6ee178fa2 100644
--- a/src/query/interpreter.cpp
+++ b/src/query/interpreter.cpp
@@ -821,16 +821,8 @@ Callback HandleSettingQuery(SettingQuery *setting_query, const Parameters ¶m
}
}
-Callback HandleSchemaQuery(SchemaQuery *schema_query, const Parameters ¶meters,
- InterpreterContext *interpreter_context, DbAccessor *db_accessor,
+Callback HandleSchemaQuery(SchemaQuery *schema_query, InterpreterContext *interpreter_context,
std::vector<Notification> *notifications) {
- Frame frame(0);
- SymbolTable symbol_table;
- EvaluationContext evaluation_context;
- evaluation_context.timestamp = QueryTimestamp();
- evaluation_context.parameters = parameters;
- ExpressionEvaluator evaluator(&frame, symbol_table, evaluation_context, db_accessor, storage::View::OLD);
-
Callback callback;
switch (schema_query->action_) {
case SchemaQuery::Action::SHOW_SCHEMAS: {
@@ -855,7 +847,7 @@ Callback HandleSchemaQuery(SchemaQuery *schema_query, const Parameters ¶mete
});
schema_info_row.emplace_back(utils::Join(primary_key_properties, ", "));
- schema_info_row.emplace_back(schema_types.size() > 1 ? "Single" : "Composite");
+ schema_info_row.emplace_back(schema_types.size() == 1 ? "Single" : "Composite");
results.push_back(std::move(schema_info_row));
}
@@ -897,10 +889,11 @@ Callback HandleSchemaQuery(SchemaQuery *schema_query, const Parameters ¶mete
schemas_types.reserve(schema_type_map.size());
for (const auto &schema_type : schema_type_map) {
auto property_id = db->NameToProperty(schema_type.first.name);
- spdlog::info("sasa {}", db->PropertyToName(db->NameToProperty(schema_type.first.name)));
schemas_types.push_back({schema_type.second, property_id});
}
- const auto res = db->CreateSchema(label, schemas_types);
+ if (!db->CreateSchema(label, schemas_types)) {
+ throw QueryException(fmt::format("Schema on label :{} already exists!", primary_label.name));
+ }
return std::vector<std::vector<TypedValue>>{};
};
return callback;
@@ -910,7 +903,9 @@ Callback HandleSchemaQuery(SchemaQuery *schema_query, const Parameters ¶mete
auto *db = interpreter_context->db;
const auto label = db->NameToLabel(primary_label.name);
- const auto res = db->DeleteSchema(label);
+ if (!db->DeleteSchema(label)) {
+ throw QueryException(fmt::format("Schema on label :{} does not exist!", primary_label.name));
+ }
return std::vector<std::vector<TypedValue>>{};
};
return callback;
@@ -2122,7 +2117,7 @@ PreparedQuery PrepareSchemaQuery(ParsedQuery parsed_query, bool in_explicit_tran
}
auto *schema_query = utils::Downcast<SchemaQuery>(parsed_query.query);
MG_ASSERT(schema_query);
- auto callback = HandleSchemaQuery(schema_query, parsed_query.parameters, interpreter_context, dba, notifications);
+ auto callback = HandleSchemaQuery(schema_query, interpreter_context, notifications);
return PreparedQuery{std::move(callback.header), std::move(parsed_query.required_privileges),
[handler = std::move(callback.fn), action = QueryHandlerResult::NOTHING,
diff --git a/src/storage/v2/schemas.cpp b/src/storage/v2/schemas.cpp
index ae13e056a..123d90433 100644
--- a/src/storage/v2/schemas.cpp
+++ b/src/storage/v2/schemas.cpp
@@ -44,14 +44,14 @@ Schemas::SchemasList Schemas::GetSchema(const LabelId primary_label) const {
}
bool Schemas::CreateSchema(const LabelId primary_label, const std::vector<SchemaPropertyType> &schemas_types) {
- const auto res = schemas_.insert({primary_label, schemas_types}).second;
- return res;
+ if (schemas_.contains(primary_label)) {
+ return false;
+ }
+ schemas_.insert({primary_label, schemas_types});
+ return true;
}
-bool Schemas::DeleteSchema(const LabelId primary_label) {
- const auto res = schemas_.erase(primary_label);
- return res;
-}
+bool Schemas::DeleteSchema(const LabelId primary_label) { return schemas_.erase(primary_label); }
std::optional<SchemaViolation> Schemas::ValidateVertex(const LabelId primary_label, const Vertex &vertex) {
// TODO Check for multiple defined primary labels
diff --git a/src/storage/v2/schemas.hpp b/src/storage/v2/schemas.hpp
index ec821d240..1fe01baf3 100644
--- a/src/storage/v2/schemas.hpp
+++ b/src/storage/v2/schemas.hpp
@@ -11,6 +11,7 @@
#pragma once
+#include <cstdint>
#include <memory>
#include <optional>
#include <unordered_map>
@@ -77,8 +78,12 @@ class Schemas {
[[nodiscard]] SchemasList GetSchema(LabelId primary_label) const;
+ // Returns true if it was successfully created or false if the schema
+ // already exists
[[nodiscard]] bool CreateSchema(LabelId label, const std::vector<SchemaPropertyType> &schemas_types);
+ // Returns true if it was successfully dropped or false if the schema
+ // does not exist
[[nodiscard]] bool DeleteSchema(LabelId label);
[[nodiscard]] std::optional<SchemaViolation> ValidateVertex(LabelId primary_label, const Vertex &vertex);
diff --git a/src/storage/v2/storage.cpp b/src/storage/v2/storage.cpp
index 272052aef..f75c7155f 100644
--- a/src/storage/v2/storage.cpp
+++ b/src/storage/v2/storage.cpp
@@ -1239,7 +1239,7 @@ SchemasInfo Storage::GetSchema(const LabelId primary_label) const {
return {schemas_.GetSchema(primary_label)};
}
-bool Storage::CreateSchema(const LabelId primary_label, std::vector<SchemaPropertyType> &schemas_types) {
+bool Storage::CreateSchema(const LabelId primary_label, const std::vector<SchemaPropertyType> &schemas_types) {
return schemas_.CreateSchema(primary_label, schemas_types);
}
diff --git a/src/storage/v2/storage.hpp b/src/storage/v2/storage.hpp
index b031a0bf1..4245a2e87 100644
--- a/src/storage/v2/storage.hpp
+++ b/src/storage/v2/storage.hpp
@@ -420,7 +420,7 @@ class Storage final {
SchemasInfo GetSchema(LabelId primary_label) const;
- bool CreateSchema(LabelId primary_label, std::vector<SchemaPropertyType> &schemas_types);
+ bool CreateSchema(LabelId primary_label, const std::vector<SchemaPropertyType> &schemas_types);
bool DeleteSchema(LabelId primary_label);