From 67538aceeb7e2c71b0026bbb1acb3ee51eb34627 Mon Sep 17 00:00:00 2001 From: Dominik Gleich <dominik.gleich@memgraph.io> Date: Thu, 23 Nov 2017 16:36:54 +0100 Subject: [PATCH] Migrate labels/properties/edgetypes to ids Summary: In preparation for distributed storage we need to have labels/properties/edgetypes uniquely identifiable by their ids, which will be global in near future. The old design has to be abandoned because it's not possible to keep track of global labels/properties/edgetypes while they are local pointers. Reviewers: mislav.bradac, florijan Reviewed By: florijan Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D993 --- src/database/graph_db.hpp | 13 ++- src/database/graph_db_accessor.cpp | 12 +-- src/database/graph_db_datatypes.hpp | 54 ++++++++++- src/query/plan/vertex_count_cache.hpp | 7 +- src/storage/concurrent_id_mapper.hpp | 57 +++++++++++ src/storage/edges.hpp | 7 +- src/utils/datetime/timestamp.hpp | 7 +- tests/manual/query_planner.cpp | 96 ++++++++++++------- tests/property_based/random_graph.cpp | 4 +- tests/unit/bolt_encoder.cpp | 25 ++--- tests/unit/concurrent_id_mapper.cpp | 74 ++++++++++++++ tests/unit/durability.cpp | 37 ++++--- tests/unit/graph_db_accessor.cpp | 4 +- tests/unit/graph_db_accessor_index_api.cpp | 7 +- tests/unit/query_common.hpp | 5 +- tests/unit/query_expression_evaluator.cpp | 21 ++-- .../query_plan_create_set_remove_delete.cpp | 4 +- 17 files changed, 330 insertions(+), 104 deletions(-) create mode 100644 src/storage/concurrent_id_mapper.hpp create mode 100644 tests/unit/concurrent_id_mapper.cpp diff --git a/src/database/graph_db.hpp b/src/database/graph_db.hpp index c0254022a..00061d131 100644 --- a/src/database/graph_db.hpp +++ b/src/database/graph_db.hpp @@ -11,6 +11,7 @@ #include "database/indexes/label_property_index.hpp" #include "durability/wal.hpp" #include "mvcc/version_list.hpp" +#include "storage/concurrent_id_mapper.hpp" #include "storage/deferred_deleter.hpp" #include "storage/edge.hpp" #include "storage/garbage_collector.hpp" @@ -114,9 +115,15 @@ class GraphDb { // unique object stores // TODO this should be also garbage collected - ConcurrentSet<std::string> labels_; - ConcurrentSet<std::string> edge_types_; - ConcurrentSet<std::string> properties_; + ConcurrentIdMapper<GraphDbTypes::Label, GraphDbTypes::Label::StorageT, + std::string> + labels_; + ConcurrentIdMapper<GraphDbTypes::EdgeType, GraphDbTypes::EdgeType::StorageT, + std::string> + edge_types_; + ConcurrentIdMapper<GraphDbTypes::Property, GraphDbTypes::Property::StorageT, + std::string> + properties_; // indexes KeyIndex<GraphDbTypes::Label, Vertex> labels_index_; diff --git a/src/database/graph_db_accessor.cpp b/src/database/graph_db_accessor.cpp index 8f99ae4ae..a98b9cb9f 100644 --- a/src/database/graph_db_accessor.cpp +++ b/src/database/graph_db_accessor.cpp @@ -336,37 +336,37 @@ void GraphDbAccessor::RemoveEdge(EdgeAccessor &edge_accessor, GraphDbTypes::Label GraphDbAccessor::Label(const std::string &label_name) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - return &(*db_.labels_.access().insert(label_name).first); + return db_.labels_.insert_value(label_name); } const std::string &GraphDbAccessor::LabelName( const GraphDbTypes::Label label) const { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - return *label; + return db_.labels_.value_by_id(label); } GraphDbTypes::EdgeType GraphDbAccessor::EdgeType( const std::string &edge_type_name) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - return &(*db_.edge_types_.access().insert(edge_type_name).first); + return db_.edge_types_.insert_value(edge_type_name); } const std::string &GraphDbAccessor::EdgeTypeName( const GraphDbTypes::EdgeType edge_type) const { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - return *edge_type; + return db_.edge_types_.value_by_id(edge_type); } GraphDbTypes::Property GraphDbAccessor::Property( const std::string &property_name) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - return &(*db_.properties_.access().insert(property_name).first); + return db_.properties_.insert_value(property_name); } const std::string &GraphDbAccessor::PropertyName( const GraphDbTypes::Property property) const { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - return *property; + return db_.properties_.value_by_id(property); } int64_t GraphDbAccessor::Counter(const std::string &name) { diff --git a/src/database/graph_db_datatypes.hpp b/src/database/graph_db_datatypes.hpp index 4cd8499a9..9f88a8ee0 100644 --- a/src/database/graph_db_datatypes.hpp +++ b/src/database/graph_db_datatypes.hpp @@ -2,10 +2,54 @@ #include <string> +#include "utils/total_ordering.hpp" + namespace GraphDbTypes { -// definitions for what data types are used for a Label, Property, EdgeType -// TODO: Typedefing pointers is terrible astractions, get rid of it. -using Label = const std::string *; -using EdgeType = const std::string *; -using Property = const std::string *; +template <typename TSpecificType> +class Common : TotalOrdering<TSpecificType> { + public: + using StorageT = uint16_t; + + Common() {} + explicit Common(const StorageT storage) : storage_(storage) {} + friend bool operator==(const TSpecificType &a, const TSpecificType &b) { + return a.storage_ == b.storage_; + } + friend bool operator<(const TSpecificType &a, const TSpecificType &b) { + return a.storage_ < b.storage_; + } + + struct Hash { + std::hash<StorageT> hash{}; + size_t operator()(const TSpecificType &t) const { return hash(t.storage_); } + }; + + private: + StorageT storage_{0}; }; + +class Label : public Common<Label> { + using Common::Common; +}; + +class EdgeType : public Common<EdgeType> { + using Common::Common; +}; + +class Property : public Common<Property> { + using Common::Common; +}; + +}; // namespace GraphDbTypes + +namespace std { +template <> +struct hash<GraphDbTypes::Label> + : public GraphDbTypes::Common<GraphDbTypes::Label>::Hash {}; +template <> +struct hash<GraphDbTypes::EdgeType> + : public GraphDbTypes::Common<GraphDbTypes::EdgeType>::Hash {}; +template <> +struct hash<GraphDbTypes::Property> + : public GraphDbTypes::Common<GraphDbTypes::Property>::Hash {}; +}; // namespace std diff --git a/src/query/plan/vertex_count_cache.hpp b/src/query/plan/vertex_count_cache.hpp index 0b78e573c..96f04202f 100644 --- a/src/query/plan/vertex_count_cache.hpp +++ b/src/query/plan/vertex_count_cache.hpp @@ -1,6 +1,11 @@ /// @file #pragma once +#include <experimental/optional> + +#include "database/graph_db_datatypes.hpp" +#include "storage/property_value.hpp" +#include "utils/bound.hpp" #include "utils/hashing/fnv.hpp" namespace query::plan { @@ -129,4 +134,4 @@ auto MakeVertexCountCache(const TDbAccessor &db) { return VertexCountCache<TDbAccessor>(db); } -} // namespace plan::query +} // namespace query::plan diff --git a/src/storage/concurrent_id_mapper.hpp b/src/storage/concurrent_id_mapper.hpp new file mode 100644 index 000000000..f96761c57 --- /dev/null +++ b/src/storage/concurrent_id_mapper.hpp @@ -0,0 +1,57 @@ +#pragma once + +#include "glog/logging.h" + +#include "data_structures/concurrent/concurrent_map.hpp" + +/** + * @brief Implements injection between values and ids. Safe to use + * concurrently. + * @TParam TIds - Id type which to use + * @TParam TIdPrimitive - Primitive type which defines storage size (uint16_t, + * uint32_t, etc.) + * @TParam TRecord - Value type for which to define bijection + */ +template <typename TId, typename TIdPrimitive, typename TValue> +class ConcurrentIdMapper { + public: + /** + * Thread safe insert value and get a unique id for it. Calling this method + * with the same value from any number of threads will always return the same + * id, i.e. mapping between values and ids will always be consistent. + */ + TId insert_value(const TValue &value) { + auto value_to_id_acc = value_to_id_.access(); + auto found = value_to_id_acc.find(value); + TId inserted_id(0); + if (found == value_to_id_acc.end()) { + TIdPrimitive new_id = id_.fetch_add(1); + DCHECK(new_id < std::numeric_limits<TIdPrimitive>::max()) + << "Number of used ids overflowed our container"; + auto insert_result = value_to_id_acc.insert(value, TId(new_id)); + // After we tried to insert value with our id we either got our id, or the + // id created by the thread which succesfully inserted (value, id) pair + inserted_id = insert_result.first->second; + } else { + inserted_id = found->second; + } + auto id_to_value_acc = id_to_value_.access(); + // We have to try to insert the inserted_id and value even if we are not the + // one who assigned id because we have to make sure that after this method + // returns that both mappings between id->value and value->id exist. + id_to_value_acc.insert(inserted_id, value); + return inserted_id; + } + + const TValue &value_by_id(const TId &id) const { + const auto id_to_value_acc = id_to_value_.access(); + auto result = id_to_value_acc.find(id); + DCHECK(result != id_to_value_acc.end()); + return result->second; + } + + private: + ConcurrentMap<TId, TValue> id_to_value_; + ConcurrentMap<TValue, TId> value_to_id_; + std::atomic<TIdPrimitive> id_{0}; +}; diff --git a/src/storage/edges.hpp b/src/storage/edges.hpp index 3164c2484..e506b83d6 100644 --- a/src/storage/edges.hpp +++ b/src/storage/edges.hpp @@ -90,9 +90,10 @@ class Edges { * present in this iterator. */ void update_position() { if (vertex_.local()) { - position_ = std::find_if( - position_, end_, - [v = this->vertex_](const Element &e) { return e.vertex == v; }); + position_ = std::find_if(position_, + end_, [v = this->vertex_](const Element &e) { + return e.vertex == v; + }); } if (edge_types_) { position_ = std::find_if(position_, end_, [this](const Element &e) { diff --git a/src/utils/datetime/timestamp.hpp b/src/utils/datetime/timestamp.hpp index 34baf47ce..1de678352 100644 --- a/src/utils/datetime/timestamp.hpp +++ b/src/utils/datetime/timestamp.hpp @@ -14,8 +14,7 @@ class Timestamp : public TotalOrdering<Timestamp> { public: Timestamp() : Timestamp(0, 0) {} - Timestamp(std::time_t time, long nsec = 0) - : unix_time(time), nsec(nsec) { + Timestamp(std::time_t time, long nsec = 0) : unix_time(time), nsec(nsec) { auto result = gmtime_r(&time, &this->time); if (result == nullptr) @@ -51,9 +50,9 @@ class Timestamp : public TotalOrdering<Timestamp> { subsec()); } - const std::string to_string(const std::string &format = fiso8601) const { + const std::string to_string(const std::string& format = fiso8601) const { return fmt::format(format, year(), month(), day(), hour(), min(), sec(), - subsec()); + subsec()); } friend std::ostream& operator<<(std::ostream& stream, const Timestamp& ts) { diff --git a/tests/manual/query_planner.cpp b/tests/manual/query_planner.cpp index 9799698e2..8b618dcea 100644 --- a/tests/manual/query_planner.cpp +++ b/tests/manual/query_planner.cpp @@ -132,33 +132,39 @@ class Timer { // Dummy DbAccessor which forwards user input for various vertex counts. class InteractiveDbAccessor { public: - InteractiveDbAccessor(int64_t vertices_count, Timer &timer) - : vertices_count_(vertices_count), timer_(timer) {} + InteractiveDbAccessor(GraphDbAccessor &dba, int64_t vertices_count, + Timer &timer) + : dba_(dba), vertices_count_(vertices_count), timer_(timer) {} int64_t VerticesCount() const { return vertices_count_; } - int64_t VerticesCount(const GraphDbTypes::Label &label) const { - if (label_vertex_count_.find(*label) == label_vertex_count_.end()) { - label_vertex_count_[*label] = ReadVertexCount("label '" + *label + "'"); + int64_t VerticesCount(const GraphDbTypes::Label &label_id) const { + auto label = dba_.LabelName(label_id); + if (label_vertex_count_.find(label) == label_vertex_count_.end()) { + label_vertex_count_[label] = ReadVertexCount("label '" + label + "'"); } - return label_vertex_count_.at(*label); + return label_vertex_count_.at(label); } - int64_t VerticesCount(const GraphDbTypes::Label &label, - const GraphDbTypes::Property &property) const { - auto key = std::make_pair(*label, *property); + int64_t VerticesCount(const GraphDbTypes::Label &label_id, + const GraphDbTypes::Property &property_id) const { + auto label = dba_.LabelName(label_id); + auto property = dba_.PropertyName(property_id); + auto key = std::make_pair(label, property); if (label_property_vertex_count_.find(key) == label_property_vertex_count_.end()) { label_property_vertex_count_[key] = ReadVertexCount( - "label '" + *label + "' and property '" + *property + "'"); + "label '" + label + "' and property '" + property + "'"); } return label_property_vertex_count_.at(key); } - int64_t VerticesCount(const GraphDbTypes::Label &label, - const GraphDbTypes::Property &property, + int64_t VerticesCount(const GraphDbTypes::Label &label_id, + const GraphDbTypes::Property &property_id, const PropertyValue &value) const { - auto label_prop = std::make_pair(*label, *property); + auto label = dba_.LabelName(label_id); + auto property = dba_.PropertyName(property_id); + auto label_prop = std::make_pair(label, property); if (label_property_index_.find(label_prop) == label_property_index_.end()) { return 0; } @@ -166,18 +172,21 @@ class InteractiveDbAccessor { if (value_vertex_count.find(value) == value_vertex_count.end()) { std::stringstream ss; ss << value; - int64_t count = ReadVertexCount("label '" + *label + "' and property '" + - *property + "' value '" + ss.str() + "'"); + int64_t count = ReadVertexCount("label '" + label + "' and property '" + + property + "' value '" + ss.str() + "'"); value_vertex_count[value] = count; } return value_vertex_count.at(value); } int64_t VerticesCount( - const GraphDbTypes::Label &label, const GraphDbTypes::Property &property, + const GraphDbTypes::Label &label_id, + const GraphDbTypes::Property &property_id, const std::experimental::optional<utils::Bound<PropertyValue>> lower, const std::experimental::optional<utils::Bound<PropertyValue>> upper) const { + auto label = dba_.LabelName(label_id); + auto property = dba_.PropertyName(property_id); std::stringstream range_string; if (lower) { range_string << (lower->IsInclusive() ? "[" : "(") << lower->value() @@ -188,17 +197,19 @@ class InteractiveDbAccessor { if (upper) { range_string << upper->value() << (upper->IsInclusive() ? "]" : ")"); } - return ReadVertexCount("label '" + *label + "' and property '" + *property + + return ReadVertexCount("label '" + label + "' and property '" + property + "' in range " + range_string.str()); } - bool LabelPropertyIndexExists(const GraphDbTypes::Label &label, - const GraphDbTypes::Property &property) const { - auto key = std::make_pair(*label, *property); + bool LabelPropertyIndexExists( + const GraphDbTypes::Label &label_id, + const GraphDbTypes::Property &property_id) const { + auto label = dba_.LabelName(label_id); + auto property = dba_.PropertyName(property_id); + auto key = std::make_pair(label, property); if (label_property_index_.find(key) == label_property_index_.end()) { bool resp = timer_.WithPause([&label, &property]() { - return AskYesNo("Index for ':" + *label + "(" + *property + - ")' exists:"); + return AskYesNo("Index for ':" + label + "(" + property + ")' exists:"); }); label_property_index_[key] = resp; } @@ -307,6 +318,7 @@ class InteractiveDbAccessor { private: typedef std::pair<std::string, std::string> LabelPropertyKey; + GraphDbAccessor &dba_; int64_t vertices_count_; Timer &timer_; mutable std::map<std::string, int64_t> label_vertex_count_; @@ -352,9 +364,11 @@ class InteractiveDbAccessor { class PlanPrinter : public query::plan::HierarchicalLogicalOperatorVisitor { public: + using HierarchicalLogicalOperatorVisitor::PostVisit; using HierarchicalLogicalOperatorVisitor::PreVisit; using HierarchicalLogicalOperatorVisitor::Visit; - using HierarchicalLogicalOperatorVisitor::PostVisit; + + explicit PlanPrinter(GraphDbAccessor &dba) : dba_(dba) {} #define PRE_VISIT(TOp) \ bool PreVisit(query::plan::TOp &) override { \ @@ -377,7 +391,8 @@ class PlanPrinter : public query::plan::HierarchicalLogicalOperatorVisitor { bool PreVisit(query::plan::ScanAllByLabel &op) override { WithPrintLn([&](auto &out) { out << "* ScanAllByLabel" - << " (" << op.output_symbol().name() << " :" << *op.label() << ")"; + << " (" << op.output_symbol().name() << " :" + << dba_.LabelName(op.label()) << ")"; }); return true; } @@ -385,8 +400,9 @@ class PlanPrinter : public query::plan::HierarchicalLogicalOperatorVisitor { bool PreVisit(query::plan::ScanAllByLabelPropertyValue &op) override { WithPrintLn([&](auto &out) { out << "* ScanAllByLabelPropertyValue" - << " (" << op.output_symbol().name() << " :" << *op.label() << " {" - << *op.property() << "})"; + << " (" << op.output_symbol().name() << " :" + << dba_.LabelName(op.label()) << " {" + << dba_.PropertyName(op.property()) << "})"; }); return true; } @@ -394,8 +410,9 @@ class PlanPrinter : public query::plan::HierarchicalLogicalOperatorVisitor { bool PreVisit(query::plan::ScanAllByLabelPropertyRange &op) override { WithPrintLn([&](auto &out) { out << "* ScanAllByLabelPropertyRange" - << " (" << op.output_symbol().name() << " :" << *op.label() << " {" - << *op.property() << "})"; + << " (" << op.output_symbol().name() << " :" + << dba_.LabelName(op.label()) << " {" + << dba_.PropertyName(op.property()) << "})"; }); return true; } @@ -500,6 +517,7 @@ class PlanPrinter : public query::plan::HierarchicalLogicalOperatorVisitor { } int depth_ = 0; + GraphDbAccessor &dba_; }; // Shorthand for a vector of pairs (logical_plan, cost). @@ -511,22 +529,24 @@ typedef std::vector< struct Command { typedef std::vector<std::string> Args; // Function of this command - std::function<void(PlansWithCost &, const Args &)> function; + std::function<void(GraphDbAccessor &, PlansWithCost &, const Args &)> + function; // Number of arguments the function works with. int arg_count; // Explanation of the command. std::string documentation; }; -#define DEFCOMMAND(Name) \ - void Name##Command(PlansWithCost &plans, const Command::Args &args) +#define DEFCOMMAND(Name) \ + void Name##Command(GraphDbAccessor &dba, PlansWithCost &plans, \ + const Command::Args &args) DEFCOMMAND(Top) { int64_t n_plans = 0; std::stringstream ss(args[0]); ss >> n_plans; if (ss.fail() || !ss.eof()) return; - PlanPrinter printer; + PlanPrinter printer(dba); n_plans = std::min(static_cast<int64_t>(plans.size()), n_plans); for (int64_t i = 0; i < n_plans; ++i) { auto &plan_pair = plans[i]; @@ -545,7 +565,7 @@ DEFCOMMAND(Show) { const auto &plan = plans[plan_ix].first; auto cost = plans[plan_ix].second; std::cout << "Plan cost: " << cost << std::endl; - PlanPrinter printer; + PlanPrinter printer(dba); plan->Accept(printer); } @@ -571,6 +591,7 @@ DEFCOMMAND(Help) { #undef DEFCOMMAND void ExaminePlans( + GraphDbAccessor &dba, std::vector<std::pair<std::unique_ptr<query::plan::LogicalOperator>, double>> &plans) { while (true) { @@ -592,7 +613,7 @@ void ExaminePlans( << " arguments" << std::endl; continue; } - command.function(plans, args); + command.function(dba, plans, args); } } @@ -646,9 +667,11 @@ int main(int argc, char *argv[]) { std::exit(EXIT_FAILURE); } GraphDb db; + GraphDbAccessor dba(db); Timer planning_timer; InteractiveDbAccessor interactive_db( - in_db_filename.empty() ? ReadInt("Vertices in DB: ") : 0, planning_timer); + dba, in_db_filename.empty() ? ReadInt("Vertices in DB: ") : 0, + planning_timer); if (!in_db_filename.empty()) { std::ifstream db_file(in_db_filename); interactive_db.Load(db_file); @@ -658,7 +681,6 @@ int main(int argc, char *argv[]) { if (!line || *line == "quit") break; if (line->empty()) continue; try { - GraphDbAccessor dba(db); auto ast = MakeAst(*line, dba); auto symbol_table = MakeSymbolTable(ast); planning_timer.Start(); @@ -669,7 +691,7 @@ int main(int argc, char *argv[]) { << std::chrono::duration<double, std::milli>(planning_time).count() << "ms" << std::endl; std::cout << "Generated " << plans.size() << " plans" << std::endl; - ExaminePlans(plans); + ExaminePlans(dba, plans); } catch (const utils::BasicException &e) { std::cout << "Error: " << e.what() << std::endl; } diff --git a/tests/property_based/random_graph.cpp b/tests/property_based/random_graph.cpp index 26460c7d9..48d6293b6 100644 --- a/tests/property_based/random_graph.cpp +++ b/tests/property_based/random_graph.cpp @@ -54,12 +54,12 @@ RC_GTEST_PROP(RandomGraph, RandomGraph, for (const auto &vertex : dba.Vertices(false)) { auto label = vertex_label_map.at(vertex); RC_ASSERT(vertex.labels().size() == 1); - RC_ASSERT(*vertex.labels()[0] == label); + RC_ASSERT(dba.LabelName(vertex.labels()[0]) == label); vertices_num_check++; } for (const auto &edge : dba.Edges(false)) { auto type = edge_type_map.at(edge); - RC_ASSERT(*edge.EdgeType() == type); + RC_ASSERT(dba.EdgeTypeName(edge.EdgeType()) == type); edges_num_check++; } RC_ASSERT(vertices_num_check == vertices_num); diff --git a/tests/unit/bolt_encoder.cpp b/tests/unit/bolt_encoder.cpp index 1846edaad..74b76c6f2 100644 --- a/tests/unit/bolt_encoder.cpp +++ b/tests/unit/bolt_encoder.cpp @@ -168,21 +168,24 @@ TEST(BoltEncoder, VertexAndEdge) { GraphDbAccessor db_accessor(db); auto va1 = db_accessor.InsertVertex(); auto va2 = db_accessor.InsertVertex(); - std::string l1("label1"), l2("label2"); - va1.add_label(&l1); - va1.add_label(&l2); - std::string p1("prop1"), p2("prop2"); + auto l1 = db_accessor.Label("label1"); + auto l2 = db_accessor.Label("label2"); + va1.add_label(l1); + va1.add_label(l2); + auto p1 = db_accessor.Property("prop1"); + auto p2 = db_accessor.Property("prop2"); PropertyValue pv1(12), pv2(200); - va1.PropsSet(&p1, pv1); - va1.PropsSet(&p2, pv2); + va1.PropsSet(p1, pv1); + va1.PropsSet(p2, pv2); // create edge - std::string et("edgetype"); - auto ea = db_accessor.InsertEdge(va1, va2, &et); - std::string p3("prop3"), p4("prop4"); + auto et = db_accessor.EdgeType("edgetype"); + auto ea = db_accessor.InsertEdge(va1, va2, et); + auto p3 = db_accessor.Property("prop3"); + auto p4 = db_accessor.Property("prop4"); PropertyValue pv3(42), pv4(1234); - ea.PropsSet(&p3, pv3); - ea.PropsSet(&p4, pv4); + ea.PropsSet(p3, pv3); + ea.PropsSet(p4, pv4); // check everything std::vector<TypedValue> vals; diff --git a/tests/unit/concurrent_id_mapper.cpp b/tests/unit/concurrent_id_mapper.cpp new file mode 100644 index 000000000..0c447e9a6 --- /dev/null +++ b/tests/unit/concurrent_id_mapper.cpp @@ -0,0 +1,74 @@ +#include <thread> +#include <vector> + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "storage/concurrent_id_mapper.hpp" + +const int THREAD_NUM = 20; +const int VALUE_MAX = 50; + +TEST(ConcurrentIdMapper, SameValueGivesSameId) { + ConcurrentIdMapper<int, int, int> mapper; + EXPECT_EQ(mapper.insert_value(1), mapper.insert_value(1)); +} + +TEST(ConcurrentIdMapper, IdToValue) { + ConcurrentIdMapper<int, int, int> mapper; + auto value = 1; + auto id = mapper.insert_value(value); + EXPECT_EQ(value, mapper.value_by_id(id)); +} + +TEST(ConcurrentIdMapper, TwoValuesTwoIds) { + ConcurrentIdMapper<int, int, int> mapper; + EXPECT_NE(mapper.insert_value(1), mapper.insert_value(2)); +} + +TEST(ConcurrentIdMapper, SameIdReturnedMultipleThreads) { + std::vector<std::thread> threads; + ConcurrentIdMapper<int, int, int> mapper; + std::vector<std::vector<int>> thread_value_ids(THREAD_NUM); + + std::atomic<int> current_value{0}; + std::atomic<int> current_value_insertion_count{0}; + + // Try to insert every value from [0, VALUE_MAX] by multiple threads in the + // same time + for (int i = 0; i < THREAD_NUM; ++i) { + threads.push_back(std::thread([&mapper, &thread_value_ids, ¤t_value, + ¤t_value_insertion_count, i]() { + int last = -1; + while (current_value <= VALUE_MAX) { + while (last == current_value) continue; + auto id = mapper.insert_value(current_value.load()); + thread_value_ids[i].push_back(id); + // Also check that reverse mapping exists after method exits + EXPECT_EQ(mapper.value_by_id(id), current_value.load()); + last = current_value; + current_value_insertion_count.fetch_add(1); + } + })); + } + // Increment current_value when all threads finish inserting it and getting an + // id for it + threads.push_back( + std::thread([¤t_value, ¤t_value_insertion_count]() { + while (current_value.load() <= VALUE_MAX) { + while (current_value_insertion_count.load() != THREAD_NUM) continue; + current_value_insertion_count.store(0); + current_value.fetch_add(1); + } + })); + for (auto &thread : threads) thread.join(); + + // For every value inserted, each thread should have the same id + for (int i = 0; i < THREAD_NUM; ++i) + for (int j = 0; j < THREAD_NUM; ++j) + EXPECT_EQ(thread_value_ids[i], thread_value_ids[j]); + + // Each value should have a unique id + std::set<int> ids(thread_value_ids[0].begin(), thread_value_ids[0].end()); + EXPECT_EQ(ids.size(), thread_value_ids[0].size()); +} diff --git a/tests/unit/durability.cpp b/tests/unit/durability.cpp index 9c7976ab1..8e23a022d 100644 --- a/tests/unit/durability.cpp +++ b/tests/unit/durability.cpp @@ -164,15 +164,25 @@ void CompareDbs(GraphDb &a, GraphDb &b) { << utils::Join(index_b, ", "); } - auto is_permutation_props = [&dba_a, &dba_b](const auto &p1, const auto p2) { + auto is_permutation_props = [&dba_a, &dba_b](const auto &p1_id, + const auto &p2_id) { + + std::vector<std::pair<std::string, query::TypedValue>> p1; + std::vector<std::pair<std::string, query::TypedValue>> p2; + + for (auto x : p1_id) p1.push_back({dba_a.PropertyName(x.first), x.second}); + for (auto x : p2_id) p2.push_back({dba_b.PropertyName(x.first), x.second}); + + // Don't use a binary predicate which depends on different value getters + // semantics for two containers because is_permutation might call the + // predicate with both arguments on the same container return p1.size() == p2.size() && - std::is_permutation( - p1.begin(), p1.end(), p2.begin(), - [&dba_a, &dba_b](const auto &p1, const auto &p2) { - return dba_a.PropertyName(p1.first) == - dba_b.PropertyName(p2.first) && - query::TypedValue::BoolEqual{}(p1.second, p2.second); - }); + std::is_permutation(p1.begin(), p1.end(), p2.begin(), + [](const auto &p1, const auto &p2) { + return p1.first == p2.first && + query::TypedValue::BoolEqual{}( + p1.second, p2.second); + }); }; { @@ -182,11 +192,12 @@ void CompareDbs(GraphDb &a, GraphDb &b) { auto v_b = dba_b.FindVertex(v_a.id(), false); ASSERT_TRUE(v_b) << "Vertex not found, id: " << v_a.id(); ASSERT_EQ(v_a.labels().size(), v_b->labels().size()); - EXPECT_TRUE(std::is_permutation( - v_a.labels().begin(), v_a.labels().end(), v_b->labels().begin(), - [&dba_a, &dba_b](const auto &la, const auto &lb) { - return dba_a.LabelName(la) == dba_b.LabelName(lb); - })); + std::vector<std::string> v_a_labels; + std::vector<std::string> v_b_labels; + for (auto x : v_a.labels()) v_a_labels.push_back(dba_a.LabelName(x)); + for (auto x : v_b->labels()) v_b_labels.push_back(dba_b.LabelName(x)); + EXPECT_TRUE(std::is_permutation(v_a_labels.begin(), v_a_labels.end(), + v_b_labels.begin())); EXPECT_TRUE(is_permutation_props(v_a.Properties(), v_b->Properties())); } auto vertices_b = dba_b.Vertices(false); diff --git a/tests/unit/graph_db_accessor.cpp b/tests/unit/graph_db_accessor.cpp index 75a627c92..6105821f2 100644 --- a/tests/unit/graph_db_accessor.cpp +++ b/tests/unit/graph_db_accessor.cpp @@ -38,7 +38,7 @@ TEST(GraphDbAccessorTest, UniqueVertexId) { std::vector<std::thread> threads; for (int i = 0; i < 50; i++) { threads.emplace_back([&db, &ids]() { - GraphDbAccessor dba(db); + GraphDbAccessor dba(db); auto access = ids.access(); for (int i = 0; i < 200; i++) access.insert(dba.InsertVertex().id()); }); @@ -350,7 +350,7 @@ TEST(GraphDbAccessorTest, Properties) { GraphDb db; GraphDbAccessor dba(db); - GraphDbTypes::EdgeType prop = dba.Property("name"); + GraphDbTypes::Property prop = dba.Property("name"); EXPECT_EQ(prop, dba.Property("name")); EXPECT_NE(prop, dba.Property("surname")); EXPECT_EQ(dba.PropertyName(prop), "name"); diff --git a/tests/unit/graph_db_accessor_index_api.cpp b/tests/unit/graph_db_accessor_index_api.cpp index 6583df692..4ad67adf2 100644 --- a/tests/unit/graph_db_accessor_index_api.cpp +++ b/tests/unit/graph_db_accessor_index_api.cpp @@ -125,10 +125,10 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuildTwice) { TEST_F(GraphDbAccessorIndex, LabelPropertyIndexCount) { dba->BuildIndex(label, property); EXPECT_EQ(dba->VerticesCount(label, property), 0); - EXPECT_EQ(Count(dba->Vertices(label, property)), 0); + EXPECT_EQ(Count(dba->Vertices(label, property, true)), 0); for (int i = 0; i < 14; ++i) AddVertex(0); EXPECT_EQ(dba->VerticesCount(label, property), 14); - EXPECT_EQ(Count(dba->Vertices(label, property)), 14); + EXPECT_EQ(Count(dba->Vertices(label, property, true)), 14); } TEST(GraphDbAccessorIndexApi, LabelPropertyBuildIndexConcurrent) { @@ -147,7 +147,8 @@ TEST(GraphDbAccessorIndexApi, LabelPropertyBuildIndexConcurrent) { dba.Abort(); success = 0; } - }).detach(); + }) + .detach(); }; int build_1_success = -1; diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index db96f69f9..df21afb4b 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -180,9 +180,10 @@ auto GetEdgeVariable(AstTreeStorage &storage, const std::string &name, /// Name is used to create the Identifier which is assigned to the node. /// auto GetNode(AstTreeStorage &storage, const std::string &name, - GraphDbTypes::Label label = nullptr) { + std::experimental::optional<GraphDbTypes::Label> label = + std::experimental::nullopt) { auto node = storage.Create<NodeAtom>(storage.Create<Identifier>(name)); - if (label) node->labels_.emplace_back(label); + if (label) node->labels_.emplace_back(*label); return node; } diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp index 4737bffbb..b206cd2ae 100644 --- a/tests/unit/query_expression_evaluator.cpp +++ b/tests/unit/query_expression_evaluator.cpp @@ -801,13 +801,14 @@ TEST(ExpressionEvaluator, FunctionProperties) { } return properties; }; - ASSERT_THAT(prop_values_to_int(EvaluateFunction("PROPERTIES", {v1})), + + ASSERT_THAT(prop_values_to_int(EvaluateFunction("PROPERTIES", {v1}, db)), UnorderedElementsAre(testing::Pair("height", 5), testing::Pair("age", 10))); - ASSERT_THAT(prop_values_to_int(EvaluateFunction("PROPERTIES", {e})), + ASSERT_THAT(prop_values_to_int(EvaluateFunction("PROPERTIES", {e}, db)), UnorderedElementsAre(testing::Pair("height", 3), testing::Pair("age", 15))); - ASSERT_THROW(EvaluateFunction("PROPERTIES", {2}), QueryRuntimeException); + ASSERT_THROW(EvaluateFunction("PROPERTIES", {2}, db), QueryRuntimeException); } TEST(ExpressionEvaluator, FunctionLast) { @@ -935,8 +936,8 @@ TEST(ExpressionEvaluator, FunctionType) { auto v2 = dba.InsertVertex(); v2.add_label(dba.Label("label2")); auto e = dba.InsertEdge(v1, v2, dba.EdgeType("type1")); - ASSERT_EQ(EvaluateFunction("TYPE", {e}).Value<std::string>(), "type1"); - ASSERT_THROW(EvaluateFunction("TYPE", {2}), QueryRuntimeException); + ASSERT_EQ(EvaluateFunction("TYPE", {e}, db).Value<std::string>(), "type1"); + ASSERT_THROW(EvaluateFunction("TYPE", {2}, db), QueryRuntimeException); } TEST(ExpressionEvaluator, FunctionLabels) { @@ -950,12 +951,12 @@ TEST(ExpressionEvaluator, FunctionLabels) { v.add_label(dba.Label("label2")); std::vector<std::string> labels; auto _labels = - EvaluateFunction("LABELS", {v}).Value<std::vector<TypedValue>>(); + EvaluateFunction("LABELS", {v}, db).Value<std::vector<TypedValue>>(); for (auto label : _labels) { labels.push_back(label.Value<std::string>()); } ASSERT_THAT(labels, UnorderedElementsAre("label1", "label2")); - ASSERT_THROW(EvaluateFunction("LABELS", {2}), QueryRuntimeException); + ASSERT_THROW(EvaluateFunction("LABELS", {2}, db), QueryRuntimeException); } TEST(ExpressionEvaluator, FunctionNodesRelationships) { @@ -1040,11 +1041,11 @@ TEST(ExpressionEvaluator, FunctionKeys) { } return keys; }; - ASSERT_THAT(prop_keys_to_string(EvaluateFunction("KEYS", {v1})), + ASSERT_THAT(prop_keys_to_string(EvaluateFunction("KEYS", {v1}, db)), UnorderedElementsAre("height", "age")); - ASSERT_THAT(prop_keys_to_string(EvaluateFunction("KEYS", {e})), + ASSERT_THAT(prop_keys_to_string(EvaluateFunction("KEYS", {e}, db)), UnorderedElementsAre("width", "age")); - ASSERT_THROW(EvaluateFunction("KEYS", {2}), QueryRuntimeException); + ASSERT_THROW(EvaluateFunction("KEYS", {2}, db), QueryRuntimeException); } TEST(ExpressionEvaluator, FunctionTail) { diff --git a/tests/unit/query_plan_create_set_remove_delete.cpp b/tests/unit/query_plan_create_set_remove_delete.cpp index 554beac04..bb3902942 100644 --- a/tests/unit/query_plan_create_set_remove_delete.cpp +++ b/tests/unit/query_plan_create_set_remove_delete.cpp @@ -103,7 +103,7 @@ TEST(QueryPlan, CreateExpand) { GraphDbTypes::Label label_node_1 = dba.Label("Node1"); GraphDbTypes::Label label_node_2 = dba.Label("Node2"); auto property = PROPERTY_PAIR("property"); - GraphDbTypes::EdgeType edge_type = dba.Label("edge_type"); + GraphDbTypes::EdgeType edge_type = dba.EdgeType("edge_type"); SymbolTable symbol_table; AstTreeStorage storage; @@ -210,7 +210,7 @@ TEST(QueryPlan, MatchCreateExpand) { // GraphDbTypes::Label label_node_1 = dba.Label("Node1"); // GraphDbTypes::Label label_node_2 = dba.Label("Node2"); // GraphDbTypes::Property property = dba.Label("prop"); - GraphDbTypes::EdgeType edge_type = dba.Label("edge_type"); + GraphDbTypes::EdgeType edge_type = dba.EdgeType("edge_type"); SymbolTable symbol_table; AstTreeStorage storage;