diff --git a/src/data_structures/concurrent/skiplist.hpp b/src/data_structures/concurrent/skiplist.hpp index 464397fa6..b12ff92ea 100644 --- a/src/data_structures/concurrent/skiplist.hpp +++ b/src/data_structures/concurrent/skiplist.hpp @@ -517,14 +517,29 @@ class SkipList : private Lockable { return skiplist->reverse(item); } - template - ConstIterator find_or_larger(const K &item) const { - return static_cast(*skiplist).find_or_larger(item); + /** + * Returns an iterator pointing to the element equal to + * item, or the first larger element. + * + * @param item An item that is comparable to skiplist element type. + * @tparam TItem item type + */ + template + Iterator find_or_larger(const TItem &item) { + return skiplist->find_or_larger(item); } - template - It find_or_larger(const K &item) { - return skiplist->find_or_larger(item); + /** + * Returns an iterator pointing to the element equal to + * item, or the first larger element. + * + * @param item An item that is comparable to skiplist element type. + * @tparam TItem item type + */ + template + ConstIterator find_or_larger(const TItem &item) const { + return static_cast(*skiplist) + .find_or_larger(item); } /** diff --git a/src/database/graph_db_accessor.cpp b/src/database/graph_db_accessor.cpp index 4f52ad7f3..4bec1b16b 100644 --- a/src/database/graph_db_accessor.cpp +++ b/src/database/graph_db_accessor.cpp @@ -100,6 +100,12 @@ int64_t GraphDbAccessor::vertices_count( debug_assert(db_.label_property_index_.IndexExists(key), "Index doesn't exist."); debug_assert(lower || upper, "At least one bound must be provided"); + debug_assert( + !lower || lower.value().value().type() != PropertyValue::Type::Null, + "Null value is not a valid index bound"); + debug_assert( + !upper || upper.value().value().type() != PropertyValue::Type::Null, + "Null value is not a valid index bound"); if (!upper) { auto lower_pac = diff --git a/src/database/graph_db_accessor.hpp b/src/database/graph_db_accessor.hpp index d7c4a9b68..db0df58c0 100644 --- a/src/database/graph_db_accessor.hpp +++ b/src/database/graph_db_accessor.hpp @@ -185,6 +185,48 @@ class GraphDbAccessor { *transaction_, current_state)); } + /** + * Return an iterable over VertexAccessors which contain the + * given label and whose property value (for the given property) + * falls within the given (lower, upper) @c Bound. + * + * The returned iterator will only contain + * vertices/edges whose property value is comparable with the + * given bounds (w.r.t. type). This has implications on Cypher + * query execuction semantics which have not been resovled yet. + * + * At least one of the bounds must be specified. Bonds can't be + * @c PropertyValue::Null. If both bounds are + * specified, their PropertyValue elments must be of comparable + * types. + * + * @param label - label for which to return VertexAccessors + * @param property - property for which to return VertexAccessors + * @param lower - Lower bound of the interval. + * @param upper - Upper bound of the interval. + * @param value - property value for which to return VertexAccessors + * @param current_state If true then the graph state for the + * current transaction+command is returned (insertions, updates and + * deletions performed in the current transaction+command are not + * ignored). + * @return iterable collection of record accessors + * satisfy the bounds and are visible to the current transaction. + */ + auto vertices( + const GraphDbTypes::Label &label, const GraphDbTypes::Property &property, + const std::experimental::optional> lower, + const std::experimental::optional> upper, + bool current_state) { + debug_assert(db_.label_property_index_.IndexExists( + LabelPropertyIndex::Key(label, property)), + "Label+property index doesn't exist."); + return iter::imap([this, current_state]( + auto vlist) { return VertexAccessor(*vlist, *this); }, + db_.label_property_index_.GetVlists( + LabelPropertyIndex::Key(label, property), lower, + upper, *transaction_, current_state)); + } + /** * Creates a new Edge and returns an accessor to it. * @@ -366,8 +408,9 @@ class GraphDbAccessor { /** * Returns approximate number of vertices that have the given label * and whose vaue is in the range defined by upper and lower @c Bound. - * At least one bound must be specified. If lower bound is not specified, - * the whole upper bound prefix is returned. + * + * At least one bound must be specified. Neither can be + * PropertyValue::Null. * * Assumes that an index for that (label, property) exists. */ diff --git a/src/database/indexes/label_property_index.hpp b/src/database/indexes/label_property_index.hpp index 3ece4d070..139c3ade5 100644 --- a/src/database/indexes/label_property_index.hpp +++ b/src/database/indexes/label_property_index.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include "data_structures/concurrent/concurrent_map.hpp" #include "database/graph_db.hpp" #include "database/graph_db_datatypes.hpp" @@ -8,6 +10,7 @@ #include "storage/edge.hpp" #include "storage/vertex.hpp" #include "transactions/transaction.hpp" +#include "utils/bound.hpp" #include "utils/total_ordering.hpp" /** @@ -195,9 +198,10 @@ class LabelPropertyIndex { const tx::Transaction &t, bool current_state) { debug_assert(ready_for_use_.access().contains(key), "Index not yet ready."); auto access = GetKeyStorage(key)->access(); - auto start_iter = - access.find_or_larger::Iterator, - IndexEntry>(IndexEntry(value, nullptr, nullptr)); + auto min_ptr = std::numeric_limits::min(); + auto start_iter = access.find_or_larger(IndexEntry( + value, reinterpret_cast *>(min_ptr), + reinterpret_cast(min_ptr))); return IndexUtils::GetVlists::Iterator, IndexEntry, Vertex>( std::move(access), start_iter, @@ -212,6 +216,97 @@ class LabelPropertyIndex { current_state); } + /** + * @brief - Get an iterable over all mvcc::VersionLists that + * are contained in this index and satisfy the given bounds. + * + * The returned iterator will only contain + * vertices/edges whose property value is comparable with the + * given bounds (w.r.t. type). This has implications on Cypher + * query execuction semantics which have not been resolved yet. + * + * At least one of the bounds must be specified. Bounds can't be + * @c PropertyValue::Null. If both bounds are + * specified, their PropertyValue elments must be of comparable + * types. + * + * @param key - Label+Property to query. + * @param lower - Lower bound of the interval. + * @param upper - Upper bound of the interval. + * @param t - current transaction, which determines visibility. + * @param current_state If true then the graph state for the + * current transaction+command is returned (insertions, updates and + * deletions performed in the current transaction+command are not + * ignored). + * @return iterable collection of mvcc:VersionLists pointers that + * satisfy the bounds and are visible to the given transaction. + */ + auto GetVlists( + const Key &key, + const std::experimental::optional> lower, + const std::experimental::optional> upper, + const tx::Transaction &transaction, bool current_state) { + debug_assert(ready_for_use_.access().contains(key), "Index not yet ready."); + + auto type = [](const auto &bound) { return bound.value().value().type(); }; + debug_assert(lower || upper, "At least one bound must be provided"); + debug_assert(!lower || type(lower) != PropertyValue::Type::Null, + "Null value is not a valid index bound"); + debug_assert(!upper || type(upper) != PropertyValue::Type::Null, + "Null value is not a valid index bound"); + + // helper function for creating a bound with an IndexElement + auto make_index_bound = [](const auto &optional_bound, bool bottom) { + std::uintptr_t ptr_bound = + bottom ? std::numeric_limits::min() + : std::numeric_limits::max(); + return IndexEntry( + optional_bound.value().value(), + reinterpret_cast *>(ptr_bound), + reinterpret_cast(ptr_bound)); + }; + + auto access = GetKeyStorage(key)->access(); + + // create the iterator startpoint based on the lower bound + auto start_iter = lower + ? access.find_or_larger(make_index_bound( + lower, lower.value().IsInclusive())) + : access.begin(); + + // a function that defines if an entry staisfies the filtering predicate. + // since we already handled the lower bound, we only need to deal with the + // upper bound and value type + std::function predicate; + if (lower && upper && + !PropertyValue::AreComparableTypes(type(lower), type(upper))) + predicate = [](const IndexEntry &) { return false; }; + else if (upper) { + auto upper_index_entry = + make_index_bound(upper, upper.value().IsExclusive()); + predicate = [upper_index_entry](const IndexEntry &entry) { + return PropertyValue::AreComparableTypes( + entry.value_.type(), upper_index_entry.value_.type()) && + entry < upper_index_entry; + }; + } else { + auto lower_type = type(lower); + make_index_bound(lower, lower.value().IsExclusive()); + predicate = [lower_type](const IndexEntry &entry) { + return PropertyValue::AreComparableTypes(entry.value_.type(), + lower_type); + }; + } + + return IndexUtils::GetVlists::Iterator, + IndexEntry, Vertex>( + std::move(access), start_iter, predicate, transaction, + [key](const IndexEntry &entry, const Vertex *const vertex) { + return LabelPropertyIndex::Exists(key, entry.value_, vertex); + }, + current_state); + } + /** * @brief - Check for existance of index. * @param key - Index key @@ -328,8 +423,7 @@ class LabelPropertyIndex { * than the second one */ static bool Less(const PropertyValue &a, const PropertyValue &b) { - if (a.type() != b.type() && - !(IsCastableToDouble(a) && IsCastableToDouble(b))) + if (!PropertyValue::AreComparableTypes(a.type(), b.type())) return a.type() < b.type(); if (a.type() == b.type()) { @@ -356,30 +450,18 @@ class LabelPropertyIndex { } } + // helper for getting a double from PropertyValue, if possible + auto get_double = [](const PropertyValue &value) { + debug_assert(value.type() == PropertyValue::Type::Int || + value.type() == PropertyValue::Type::Double, + "Invalid data type."); + if (value.type() == PropertyValue::Type::Int) + return static_cast(value.Value()); + return value.Value(); + }; + // Types are int and double - convert int to double - return GetDouble(a) < GetDouble(b); - } - - /** - * @brief - Return value casted to double. This is only possible for - * integers and doubles. - */ - static double GetDouble(const PropertyValue &value) { - debug_assert(value.type() == PropertyValue::Type::Int || - value.type() == PropertyValue::Type::Double, - "Invalid data type."); - if (value.type() == PropertyValue::Type::Int) - return static_cast(value.Value()); - return value.Value(); - } - - /** - * @brief - Return if this value is castable to double (returns true for - * integers and doubles). - */ - static bool IsCastableToDouble(const PropertyValue &value) { - return value.type() == PropertyValue::Type::Int || - value.type() == PropertyValue::Type::Double; + return get_double(a) < get_double(b); } /** diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index ed7676a4b..6e8c8afcc 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -297,21 +297,14 @@ std::unique_ptr ScanAllByLabelPropertyRange::MakeCursor( auto vertices = [this, &db, is_less](Frame &frame, const SymbolTable &symbol_table) { ExpressionEvaluator evaluator(frame, symbol_table, db, graph_view_); - auto lower_val = lower_bound_ ? lower_bound_->value()->Accept(evaluator) - : TypedValue::Null; - auto upper_val = upper_bound_ ? upper_bound_->value()->Accept(evaluator) - : TypedValue::Null; - return iter::filter( - [this, lower_val, upper_val, is_less](const VertexAccessor &vertex) { - TypedValue value = vertex.PropsAt(property_); - debug_assert(!value.IsNull(), "Unexpected property with Null value"); - if (lower_bound_ && is_less(value, lower_val, lower_bound_->type())) - return false; - if (upper_bound_ && is_less(upper_val, value, upper_bound_->type())) - return false; - return true; - }, - db.vertices(label_, property_, graph_view_ == GraphView::NEW)); + auto convert = [&evaluator](const auto &bound) + -> std::experimental::optional> { + if (!bound) return std::experimental::nullopt; + return std::experimental::make_optional(utils::Bound( + bound.value().value()->Accept(evaluator), bound.value().type())); + }; + return db.vertices(label_, property_, convert(lower_bound()), + convert(upper_bound()), graph_view_ == GraphView::NEW); }; return std::make_unique>( output_symbol_, input_->MakeCursor(db), std::move(vertices), db); diff --git a/src/storage/property_value.hpp b/src/storage/property_value.hpp index 8e470dc0f..7258c6ed8 100644 --- a/src/storage/property_value.hpp +++ b/src/storage/property_value.hpp @@ -28,6 +28,15 @@ class PropertyValue { // single static reference to Null, used whenever Null should be returned static const PropertyValue Null; + /** Checks if the given PropertyValue::Types are comparable */ + static bool AreComparableTypes(Type a, Type b) { + auto is_numeric = [](const Type t) { + return t == Type::Int || t == Type::Double; + }; + + return a == b || (is_numeric(a) && is_numeric(b)); + } + // constructors for primitive types PropertyValue(bool value) : type_(Type::Bool) { bool_v = value; } PropertyValue(int value) : type_(Type::Int) { int_v = value; } diff --git a/tests/concurrent/sl_hang.cpp b/tests/concurrent/sl_hang.cpp index 402d00f84..bce9ad583 100644 --- a/tests/concurrent/sl_hang.cpp +++ b/tests/concurrent/sl_hang.cpp @@ -25,7 +25,7 @@ TEST(SkipList, HangDuringFindOrLarger) { threads.emplace_back([&iter, &skiplist]() { auto accessor = skiplist.access(); for (int i = 0; i < iter; ++i) - accessor.find_or_larger::ConstIterator>(rand() % 3); + accessor.find_or_larger(rand() % 3); }); } for (auto &thread : threads) thread.join(); diff --git a/tests/unit/graph_db_accessor_index_api.cpp b/tests/unit/graph_db_accessor_index_api.cpp index 37261df95..4c3cedb82 100644 --- a/tests/unit/graph_db_accessor_index_api.cpp +++ b/tests/unit/graph_db_accessor_index_api.cpp @@ -15,100 +15,143 @@ auto Count(TIterable iterable) { return std::distance(iterable.begin(), iterable.end()); } -TEST(GraphDbAccessor, VertexByLabelCount) { +/** + * A test fixture that contains a database, accessor, + * label, property and an edge_type. + */ +class GraphDbAccessorIndex : public testing::Test { + protected: Dbms dbms; - auto dba = dbms.active(); - auto lab1 = dba->label("lab1"); - auto lab2 = dba->label("lab2"); + std::unique_ptr dba = dbms.active(); + GraphDbTypes::Property property = dba->property("property"); + GraphDbTypes::Label label = dba->label("label"); + GraphDbTypes::EdgeType edge_type = dba->edge_type("edge_type"); - EXPECT_EQ(dba->vertices_count(lab1), 0); - EXPECT_EQ(dba->vertices_count(lab2), 0); + auto AddVertex() { + auto vertex = dba->insert_vertex(); + vertex.add_label(label); + return vertex; + } + + auto AddVertex(int property_value) { + auto vertex = dba->insert_vertex(); + vertex.add_label(label); + vertex.PropsSet(property, property_value); + return vertex; + } + + // commits the current dba, and replaces it with a new one + void Commit() { + dba->commit(); + auto dba2 = dbms.active(); + dba.swap(dba2); + } +}; + +TEST_F(GraphDbAccessorIndex, LabelIndexCount) { + auto label2 = dba->label("label2"); + EXPECT_EQ(dba->vertices_count(label), 0); + EXPECT_EQ(dba->vertices_count(label2), 0); EXPECT_EQ(dba->vertices_count(), 0); - for (int i = 0; i < 11; ++i) dba->insert_vertex().add_label(lab1); - for (int i = 0; i < 17; ++i) dba->insert_vertex().add_label(lab2); + for (int i = 0; i < 11; ++i) dba->insert_vertex().add_label(label); + for (int i = 0; i < 17; ++i) dba->insert_vertex().add_label(label2); // even though xxx_count functions in GraphDbAccessor can over-estaimate // in this situation they should be exact (nothing was ever deleted) - EXPECT_EQ(dba->vertices_count(lab1), 11); - EXPECT_EQ(dba->vertices_count(lab2), 17); + EXPECT_EQ(dba->vertices_count(label), 11); + EXPECT_EQ(dba->vertices_count(label2), 17); EXPECT_EQ(dba->vertices_count(), 28); } -TEST(GraphDbAccessor, VertexByLabelPropertyCount) { - Dbms dbms; - auto dba = dbms.active(); +TEST_F(GraphDbAccessorIndex, LabelIndexIteration) { + // add 10 vertices, check visibility + for (int i = 0; i < 10; i++) AddVertex(); + EXPECT_EQ(Count(dba->vertices(label, false)), 0); + EXPECT_EQ(Count(dba->vertices(label, true)), 10); + Commit(); + EXPECT_EQ(Count(dba->vertices(label, false)), 10); + EXPECT_EQ(Count(dba->vertices(label, true)), 10); - auto lab1 = dba->label("lab1"); - auto lab2 = dba->label("lab2"); - - auto prop1 = dba->property("prop1"); - auto prop2 = dba->property("prop2"); - - dba->BuildIndex(lab1, prop1); - dba->BuildIndex(lab1, prop2); - dba->BuildIndex(lab2, prop1); - dba->BuildIndex(lab2, prop2); - - EXPECT_EQ(dba->vertices_count(lab1, prop1), 0); - EXPECT_EQ(dba->vertices_count(lab1, prop2), 0); - EXPECT_EQ(dba->vertices_count(lab2, prop1), 0); - EXPECT_EQ(dba->vertices_count(lab2, prop2), 0); - EXPECT_EQ(dba->vertices_count(), 0); - - for (int i = 0; i < 14; ++i) { - VertexAccessor vertex = dba->insert_vertex(); - vertex.add_label(lab1); - vertex.PropsSet(prop1, 1); + // remove 3 vertices, check visibility + int deleted = 0; + for (auto vertex : dba->vertices(false)) { + dba->remove_vertex(vertex); + if (++deleted >= 3) break; } - for (int i = 0; i < 15; ++i) { - auto vertex = dba->insert_vertex(); - vertex.add_label(lab1); - vertex.PropsSet(prop2, 2); - } - for (int i = 0; i < 16; ++i) { - auto vertex = dba->insert_vertex(); - vertex.add_label(lab2); - vertex.PropsSet(prop1, 3); - } - for (int i = 0; i < 17; ++i) { - auto vertex = dba->insert_vertex(); - vertex.add_label(lab2); - vertex.PropsSet(prop2, 4); - } - // even though xxx_count functions in GraphDbAccessor can over-estimate + EXPECT_EQ(Count(dba->vertices(label, false)), 10); + EXPECT_EQ(Count(dba->vertices(label, true)), 7); + Commit(); + EXPECT_EQ(Count(dba->vertices(label, false)), 7); + EXPECT_EQ(Count(dba->vertices(label, true)), 7); +} + +TEST_F(GraphDbAccessorIndex, EdgeTypeCount) { + auto edge_type2 = dba->edge_type("edge_type2"); + EXPECT_EQ(dba->edges_count(edge_type), 0); + EXPECT_EQ(dba->edges_count(edge_type2), 0); + EXPECT_EQ(dba->edges_count(), 0); + + auto v1 = AddVertex(); + auto v2 = AddVertex(); + for (int i = 0; i < 11; ++i) dba->insert_edge(v1, v2, edge_type); + for (int i = 0; i < 17; ++i) dba->insert_edge(v1, v2, edge_type2); + // even though xxx_count functions in GraphDbAccessor can over-estaimate // in this situation they should be exact (nothing was ever deleted) - EXPECT_EQ(dba->vertices_count(lab1, prop1), 14); - EXPECT_EQ(dba->vertices_count(lab1, prop2), 15); - EXPECT_EQ(dba->vertices_count(lab2, prop1), 16); - EXPECT_EQ(dba->vertices_count(lab2, prop2), 17); - EXPECT_EQ(dba->vertices_count(), 14 + 15 + 16 + 17); + EXPECT_EQ(dba->edges_count(edge_type), 11); + EXPECT_EQ(dba->edges_count(edge_type2), 17); + EXPECT_EQ(dba->edges_count(), 28); +} + +TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuild) { + AddVertex(0); + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + EXPECT_DEATH(dba->vertices_count(label, property), "Index doesn't exist."); + + Commit(); + dba->BuildIndex(label, property); + Commit(); + + EXPECT_EQ(dba->vertices_count(label, property), 1); + + // confirm there is a differentiation of indexes based on (label, property) + auto label2 = dba->label("label2"); + auto property2 = dba->property("property2"); + dba->BuildIndex(label2, property); + dba->BuildIndex(label, property2); + Commit(); + + EXPECT_EQ(dba->vertices_count(label, property), 1); + EXPECT_EQ(dba->vertices_count(label2, property), 0); + EXPECT_EQ(dba->vertices_count(label, property2), 0); +} + +TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuildTwice) { + dba->BuildIndex(label, property); + EXPECT_THROW(dba->BuildIndex(label, property), utils::BasicException); +} + +TEST_F(GraphDbAccessorIndex, LabelPropertyIndexCount) { + dba->BuildIndex(label, property); + EXPECT_EQ(dba->vertices_count(label, property), 0); + EXPECT_EQ(Count(dba->vertices(label, property)), 0); + for (int i = 0; i < 14; ++i) AddVertex(0); + EXPECT_EQ(dba->vertices_count(label, property), 14); + EXPECT_EQ(Count(dba->vertices(label, property)), 14); } #define EXPECT_WITH_MARGIN(x, center) \ EXPECT_THAT( \ x, testing::AllOf(testing::Ge(center - 2), testing::Le(center + 2))); -TEST(GraphDbAccessor, VertexByLabelPropertyValueCount) { - Dbms dbms; - auto dba = dbms.active(); - auto label = dba->label("label"); - auto property = dba->property("property"); +TEST_F(GraphDbAccessorIndex, LabelPropertyValueCount) { dba->BuildIndex(label, property); // add some vertices without the property - for (int i = 0; i < 20; i++) dba->insert_vertex(); + for (int i = 0; i < 20; i++) AddVertex(); // add vertices with prop values [0, 29), ten vertices for each value - for (int i = 0; i < 300; i++) { - auto vertex = dba->insert_vertex(); - vertex.add_label(label); - vertex.PropsSet(property, i / 10); - } + for (int i = 0; i < 300; i++) AddVertex(i / 10); // add verties in t he [30, 40) range, 100 vertices for each value - for (int i = 0; i < 1000; i++) { - auto vertex = dba->insert_vertex(); - vertex.add_label(label); - vertex.PropsSet(property, 30 + i / 100); - } + for (int i = 0; i < 1000; i++) AddVertex(30 + i / 100); // test estimates for exact value count EXPECT_WITH_MARGIN(dba->vertices_count(label, property, 10), 10); @@ -126,127 +169,47 @@ TEST(GraphDbAccessor, VertexByLabelPropertyValueCount) { return std::experimental::make_optional( utils::MakeBoundExclusive(PropertyValue(value))); }; - auto Count = [&dba, label, property](auto lower, auto upper) { + auto vertices_count = [this](auto lower, auto upper) { return dba->vertices_count(label, property, lower, upper); }; using std::experimental::nullopt; - EXPECT_DEATH(Count(nullopt, nullopt), "bound must be provided"); - EXPECT_WITH_MARGIN(Count(nullopt, Exclusive(4)), 40); - EXPECT_WITH_MARGIN(Count(nullopt, Inclusive(4)), 50); - EXPECT_WITH_MARGIN(Count(Exclusive(13), nullopt), 160 + 1000); - EXPECT_WITH_MARGIN(Count(Inclusive(13), nullopt), 170 + 1000); - EXPECT_WITH_MARGIN(Count(Inclusive(13), Exclusive(14)), 10); - EXPECT_WITH_MARGIN(Count(Exclusive(13), Inclusive(14)), 10); - EXPECT_WITH_MARGIN(Count(Exclusive(13), Exclusive(13)), 0); - EXPECT_WITH_MARGIN(Count(Inclusive(20), Exclusive(13)), 0); + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + EXPECT_DEATH(vertices_count(nullopt, nullopt), "bound must be provided"); + EXPECT_WITH_MARGIN(vertices_count(nullopt, Exclusive(4)), 40); + EXPECT_WITH_MARGIN(vertices_count(nullopt, Inclusive(4)), 50); + EXPECT_WITH_MARGIN(vertices_count(Exclusive(13), nullopt), 160 + 1000); + EXPECT_WITH_MARGIN(vertices_count(Inclusive(13), nullopt), 170 + 1000); + EXPECT_WITH_MARGIN(vertices_count(Inclusive(13), Exclusive(14)), 10); + EXPECT_WITH_MARGIN(vertices_count(Exclusive(13), Inclusive(14)), 10); + EXPECT_WITH_MARGIN(vertices_count(Exclusive(13), Exclusive(13)), 0); + EXPECT_WITH_MARGIN(vertices_count(Inclusive(20), Exclusive(13)), 0); } #undef EXPECT_WITH_MARGIN -TEST(GraphDbAccessor, EdgeByEdgeTypeCount) { - Dbms dbms; - auto dba = dbms.active(); - auto t1 = dba->edge_type("t1"); - auto t2 = dba->edge_type("t2"); - - EXPECT_EQ(dba->edges_count(t1), 0); - EXPECT_EQ(dba->edges_count(t2), 0); - EXPECT_EQ(dba->edges_count(), 0); - auto v1 = dba->insert_vertex(); - auto v2 = dba->insert_vertex(); - for (int i = 0; i < 11; ++i) dba->insert_edge(v1, v2, t1); - for (int i = 0; i < 17; ++i) dba->insert_edge(v1, v2, t2); - // even though xxx_count functions in GraphDbAccessor can over-estaimate - // in this situation they should be exact (nothing was ever deleted) - EXPECT_EQ(dba->edges_count(t1), 11); - EXPECT_EQ(dba->edges_count(t2), 17); - EXPECT_EQ(dba->edges_count(), 28); -} - -// Check if build index adds old vertex entries (ones before the index was -// created) -TEST(GraphDbAccessor, BuildIndexOnOld) { - Dbms dbms; - auto dba = dbms.active(); - - auto label = dba->label("lab1"); - auto property = dba->property("prop1"); - - auto vertex_accessor = dba->insert_vertex(); - vertex_accessor.add_label(label); - vertex_accessor.PropsSet(property, 0); - - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - EXPECT_DEATH(dba->vertices_count(label, property), "Index doesn't exist."); - dba->commit(); - - auto dba2 = dbms.active(); - dba2->BuildIndex(label, property); - dba2->commit(); - - auto dba3 = dbms.active(); - // Index is built and vertex is automatically added inside - EXPECT_EQ(dba3->vertices_count(label, property), 1); - EXPECT_EQ(Count(dba3->vertices(label, property)), 1); - dba3->commit(); -} - -// Try to build index two times -TEST(GraphDbAccessor, BuildIndexDouble) { - Dbms dbms; - auto dba = dbms.active(); - - auto label = dba->label("lab1"); - auto property = dba->property("prop1"); +TEST_F(GraphDbAccessorIndex, LabelPropertyValueIteration) { dba->BuildIndex(label, property); - EXPECT_THROW(dba->BuildIndex(label, property), utils::BasicException); + Commit(); + + // insert 10 verties and and check visibility + for (int i = 0; i < 10; i++) AddVertex(12); + EXPECT_EQ(Count(dba->vertices(label, property, 12, false)), 0); + EXPECT_EQ(Count(dba->vertices(label, property, 12, true)), 10); + Commit(); + EXPECT_EQ(Count(dba->vertices(label, property, 12, false)), 10); + EXPECT_EQ(Count(dba->vertices(label, property, 12, true)), 10); } -// Inserts vertices with properties with integers and filters to get exact -// vertices with an exact integer. -TEST(GraphDbAccessor, FilterLabelPropertySpecificValue) { - Dbms dbms; - auto dba = dbms.active(); - auto label = dba->label("lab1"); - auto property = dba->property("prop1"); +TEST_F(GraphDbAccessorIndex, LabelPropertyValueSorting) { dba->BuildIndex(label, property); - dba->commit(); + Commit(); - auto dba2 = dbms.active(); - for (int i = 1; i <= 5; ++i) { - for (int j = 1; j <= i; ++j) { - auto vertex = dba2->insert_vertex(); - vertex.add_label(label); - vertex.PropsSet(property, i); - } - } - dba2->commit(); - auto dba3 = dbms.active(); - for (int i = 1; i <= 5; ++i) - EXPECT_EQ(Count(dba3->vertices(label, property, PropertyValue(i), false)), - i); -} - -// Inserts integers, double, lists, booleans into index and check if they -// are -// sorted as they should be sorted. -TEST(GraphDbAccessor, SortedLabelPropertyEntries) { - Dbms dbms; - auto dba = dbms.active(); - - auto label = dba->label("lab1"); - auto property = dba->property("prop1"); - - dba->BuildIndex(label, property); - dba->commit(); - - auto dba2 = dbms.active(); std::vector expected_property_value(50, 0); // strings for (int i = 0; i < 10; ++i) { - auto vertex_accessor = dba2->insert_vertex(); + auto vertex_accessor = dba->insert_vertex(); vertex_accessor.add_label(label); vertex_accessor.PropsSet(property, static_cast(std::to_string(i))); @@ -254,7 +217,7 @@ TEST(GraphDbAccessor, SortedLabelPropertyEntries) { } // bools - insert in reverse to check for comparison between values. for (int i = 9; i >= 0; --i) { - auto vertex_accessor = dba2->insert_vertex(); + auto vertex_accessor = dba->insert_vertex(); vertex_accessor.add_label(label); vertex_accessor.PropsSet(property, static_cast(i / 5)); expected_property_value[10 + i] = vertex_accessor.PropsAt(property); @@ -262,14 +225,14 @@ TEST(GraphDbAccessor, SortedLabelPropertyEntries) { // integers for (int i = 0; i < 10; ++i) { - auto vertex_accessor = dba2->insert_vertex(); + auto vertex_accessor = dba->insert_vertex(); vertex_accessor.add_label(label); vertex_accessor.PropsSet(property, i); expected_property_value[20 + 2 * i] = vertex_accessor.PropsAt(property); } // doubles for (int i = 0; i < 10; ++i) { - auto vertex_accessor = dba2->insert_vertex(); + auto vertex_accessor = dba->insert_vertex(); vertex_accessor.add_label(label); vertex_accessor.PropsSet(property, static_cast(i + 0.5)); expected_property_value[20 + 2 * i + 1] = vertex_accessor.PropsAt(property); @@ -278,7 +241,7 @@ TEST(GraphDbAccessor, SortedLabelPropertyEntries) { // lists of ints - insert in reverse to check for comparision between // lists. for (int i = 9; i >= 0; --i) { - auto vertex_accessor = dba2->insert_vertex(); + auto vertex_accessor = dba->insert_vertex(); vertex_accessor.add_label(label); std::vector value; value.push_back(PropertyValue(i)); @@ -286,11 +249,11 @@ TEST(GraphDbAccessor, SortedLabelPropertyEntries) { expected_property_value[40 + i] = vertex_accessor.PropsAt(property); } - EXPECT_EQ(Count(dba2->vertices(label, property, false)), 0); - EXPECT_EQ(Count(dba2->vertices(label, property, true)), 50); + EXPECT_EQ(Count(dba->vertices(label, property, false)), 0); + EXPECT_EQ(Count(dba->vertices(label, property, true)), 50); int cnt = 0; - for (auto vertex : dba2->vertices(label, property, true)) { + for (auto vertex : dba->vertices(label, property, true)) { const PropertyValue &property_value = vertex.PropsAt(property); EXPECT_EQ(property_value.type(), expected_property_value[cnt].type()); switch (property_value.type()) { @@ -328,72 +291,92 @@ TEST(GraphDbAccessor, SortedLabelPropertyEntries) { } } -TEST(GraphDbAccessor, VisibilityAfterInsertion) { - Dbms dbms; - auto dba = dbms.active(); - auto v1 = dba->insert_vertex(); - auto v2 = dba->insert_vertex(); - auto lab1 = dba->label("lab1"); - auto lab2 = dba->label("lab2"); - v1.add_label(lab1); - auto type1 = dba->edge_type("type1"); - auto type2 = dba->edge_type("type2"); - dba->insert_edge(v1, v2, type1); +/** + * A test fixture that contains a database, accessor, + * (label, property) index and 100 vertices, 10 for + * each of [0, 10) property values. + */ +class GraphDbAccesssorIndexRange : public GraphDbAccessorIndex { + protected: + void SetUp() override { + dba->BuildIndex(label, property); + for (int i = 0; i < 100; i++) AddVertex(i / 10); - EXPECT_EQ(Count(dba->vertices(lab1, false)), 0); - EXPECT_EQ(Count(dba->vertices(lab1, true)), 1); - EXPECT_EQ(Count(dba->vertices(lab2, false)), 0); - EXPECT_EQ(Count(dba->vertices(lab2, true)), 0); - EXPECT_EQ(Count(dba->edges(type1, false)), 0); - EXPECT_EQ(Count(dba->edges(type1, true)), 1); - EXPECT_EQ(Count(dba->edges(type2, false)), 0); - EXPECT_EQ(Count(dba->edges(type2, true)), 0); - - dba->advance_command(); - - EXPECT_EQ(Count(dba->vertices(lab1, false)), 1); - EXPECT_EQ(Count(dba->vertices(lab1, true)), 1); - EXPECT_EQ(Count(dba->vertices(lab2, false)), 0); - EXPECT_EQ(Count(dba->vertices(lab2, true)), 0); - EXPECT_EQ(Count(dba->edges(type1, false)), 1); - EXPECT_EQ(Count(dba->edges(type1, true)), 1); - EXPECT_EQ(Count(dba->edges(type2, false)), 0); - EXPECT_EQ(Count(dba->edges(type2, true)), 0); -} - -TEST(GraphDbAccessor, VisibilityAfterDeletion) { - Dbms dbms; - auto dba = dbms.active(); - auto lab = dba->label("lab"); - for (int i = 0; i < 5; ++i) dba->insert_vertex().add_label(lab); - dba->advance_command(); - auto type = dba->edge_type("type"); - for (int j = 0; j < 3; ++j) { - auto vertices_it = dba->vertices(false).begin(); - dba->insert_edge(*vertices_it++, *vertices_it, type); + ASSERT_EQ(Count(dba->vertices(false)), 0); + ASSERT_EQ(Count(dba->vertices(true)), 100); + Commit(); + ASSERT_EQ(Count(dba->vertices(false)), 100); } - dba->advance_command(); - EXPECT_EQ(Count(dba->vertices(lab, false)), 5); - EXPECT_EQ(Count(dba->vertices(lab, true)), 5); - EXPECT_EQ(Count(dba->edges(type, false)), 3); - EXPECT_EQ(Count(dba->edges(type, true)), 3); + auto Vertices(std::experimental::optional> lower, + std::experimental::optional> upper, + bool current_state = false) { + return dba->vertices(label, property, lower, upper, current_state); + } - // delete two edges - auto edges_it = dba->edges(false).begin(); - for (int k = 0; k < 2; ++k) dba->remove_edge(*edges_it++); - EXPECT_EQ(Count(dba->edges(type, false)), 3); - EXPECT_EQ(Count(dba->edges(type, true)), 1); - dba->advance_command(); - EXPECT_EQ(Count(dba->edges(type, false)), 1); - EXPECT_EQ(Count(dba->edges(type, true)), 1); + auto Inclusive(PropertyValue value) { + return std::experimental::make_optional( + utils::MakeBoundInclusive(PropertyValue(value))); + } - // detach-delete 2 vertices - auto vertices_it = dba->vertices(false).begin(); - for (int k = 0; k < 2; ++k) dba->detach_remove_vertex(*vertices_it++); - EXPECT_EQ(Count(dba->vertices(lab, false)), 5); - EXPECT_EQ(Count(dba->vertices(lab, true)), 3); - dba->advance_command(); - EXPECT_EQ(Count(dba->vertices(lab, false)), 3); - EXPECT_EQ(Count(dba->vertices(lab, true)), 3); + auto Exclusive(int value) { + return std::experimental::make_optional( + utils::MakeBoundExclusive(PropertyValue(value))); + } +}; + +TEST_F(GraphDbAccesssorIndexRange, RangeIteration) { + using std::experimental::nullopt; + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(7))), 80); + EXPECT_EQ(Count(Vertices(nullopt, Exclusive(7))), 70); + EXPECT_EQ(Count(Vertices(Inclusive(7), nullopt)), 30); + EXPECT_EQ(Count(Vertices(Exclusive(7), nullopt)), 20); + EXPECT_EQ(Count(Vertices(Exclusive(3), Exclusive(6))), 20); + EXPECT_EQ(Count(Vertices(Inclusive(3), Inclusive(6))), 40); + EXPECT_EQ(Count(Vertices(Inclusive(6), Inclusive(3))), 0); + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + EXPECT_DEATH(Vertices(nullopt, nullopt), "bound must be provided"); +} + +TEST_F(GraphDbAccesssorIndexRange, RangeIterationCurrentState) { + using std::experimental::nullopt; + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(7))), 80); + for (int i = 0; i < 20; i++) AddVertex(2); + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(7))), 80); + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(7), true)), 100); + Commit(); + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(7))), 100); +} + +TEST_F(GraphDbAccesssorIndexRange, RangeInterationIncompatibleTypes) { + using std::experimental::nullopt; + + // using PropertyValue::Null as a bound fails with an assertion + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + EXPECT_DEATH(Vertices(nullopt, Inclusive(PropertyValue::Null)), + "not a valid index bound"); + EXPECT_DEATH(Vertices(Inclusive(PropertyValue::Null), nullopt), + "not a valid index bound"); + std::vector incompatible_with_int{ + "string", true, std::vector{1}}; + + // using incompatible upper and lower bounds yields no results + EXPECT_EQ(Count(Vertices(Inclusive(2), Inclusive("string"))), 0); + + // for incomparable bound and stored data, + // expect that no results are returned + ASSERT_EQ(Count(Vertices(Inclusive(0), nullopt)), 100); + for (PropertyValue value : incompatible_with_int) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(value))), 0) + << "Found vertices of type int for predicate value type: " + << value.type(); + EXPECT_EQ(Count(Vertices(Inclusive(value), nullopt)), 0) + << "Found vertices of type int for predicate value type: " + << value.type(); + } + + // we can compare int to double + EXPECT_EQ(Count(Vertices(nullopt, Inclusive(1000.0))), 100); + EXPECT_EQ(Count(Vertices(Inclusive(0.0), nullopt)), 100); } diff --git a/tests/unit/query_plan_match_filter_return.cpp b/tests/unit/query_plan_match_filter_return.cpp index 33e32af3e..443e47046 100644 --- a/tests/unit/query_plan_match_filter_return.cpp +++ b/tests/unit/query_plan_match_filter_return.cpp @@ -840,104 +840,79 @@ TEST(QueryPlan, ScanAllByLabelProperty) { // Add 5 vertices with same label, but with different property values. auto label = dba->label("label"); auto prop = dba->property("prop"); - for (int i = 1; i <= 5; ++i) { + + // vertex property values that will be stored into the DB + // clang-format off + std::vector values{ + true, false, "a", "b", "c", 0, 1, 2, 0.5, 1.5, 2.5, + std::vector{0}, std::vector{1}, + std::vector{2}}; + // clang-format on + + for (const auto &value : values) { auto vertex = dba->insert_vertex(); vertex.add_label(label); - vertex.PropsSet(prop, i); + vertex.PropsSet(prop, value); } dba->commit(); dba = dbms.active(); dba->BuildIndex(label, prop); - dba = dbms.active(); - EXPECT_EQ(5, CountIterable(dba->vertices(false))); - // MATCH (n :label) WHERE 2 <= n.prop < 4 - AstTreeStorage storage; - SymbolTable symbol_table; - auto scan_all = MakeScanAllByLabelPropertyRange( - storage, symbol_table, "n", label, prop, - Bound{LITERAL(2), Bound::Type::INCLUSIVE}, - Bound{LITERAL(4), Bound::Type::EXCLUSIVE}); - // RETURN n - auto output = NEXPR("n", IDENT("n")); - auto produce = MakeProduce(scan_all.op_, output); - symbol_table[*output->expression_] = scan_all.sym_; - symbol_table[*output] = symbol_table.CreateSymbol("n", true); - auto results = CollectProduce(produce.get(), symbol_table, *dba); - EXPECT_EQ(results.size(), 2); - for (const auto &row : results) { - ASSERT_EQ(row.size(), 1); - auto vertex = row[0].Value(); - auto value = vertex.PropsAt(prop); - TypedValue::BoolEqual eq; - EXPECT_TRUE(eq(value, 2) || eq(value, 3)); - } -} - -TEST(QueryPlan, ScanAllByLabelPropertyCompareError) { - Dbms dbms; - auto dba = dbms.active(); - // Add 2 vertices with same label, but with property values that cannot be - // compared. - auto label = dba->label("label"); - auto prop = dba->property("prop"); - auto number_vertex = dba->insert_vertex(); - number_vertex.add_label(label); - number_vertex.PropsSet(prop, 42); - auto string_vertex = dba->insert_vertex(); - string_vertex.add_label(label); - string_vertex.PropsSet(prop, "string"); dba->commit(); dba = dbms.active(); - dba->BuildIndex(label, prop); - dba = dbms.active(); - EXPECT_EQ(2, CountIterable(dba->vertices(false))); - // MATCH (n :label) WHERE 1 < n.prop - AstTreeStorage storage; - SymbolTable symbol_table; - auto scan_all = MakeScanAllByLabelPropertyRange( - storage, symbol_table, "n", label, prop, - Bound{LITERAL(1), Bound::Type::EXCLUSIVE}, std::experimental::nullopt); - // RETURN n - auto output = NEXPR("n", IDENT("n")); - auto produce = MakeProduce(scan_all.op_, output); - symbol_table[*output->expression_] = scan_all.sym_; - symbol_table[*output] = symbol_table.CreateSymbol("n", true); - EXPECT_THROW(CollectProduce(produce.get(), symbol_table, *dba), - QueryRuntimeException); -} + ASSERT_EQ(14, CountIterable(dba->vertices(false))); -TEST(QueryPlan, ScanAllByLabelPropertyRangeNull) { - Dbms dbms; - auto dba = dbms.active(); - // Add 2 vertices with the same label, but with property values that cannot be - // compared. We are comparing to null, so no results should be produced. - auto label = dba->label("label"); - auto prop = dba->property("prop"); - auto number_vertex = dba->insert_vertex(); - number_vertex.add_label(label); - number_vertex.PropsSet(prop, 42); - auto string_vertex = dba->insert_vertex(); - string_vertex.add_label(label); - string_vertex.PropsSet(prop, "string"); - dba->commit(); - dba = dbms.active(); - dba->BuildIndex(label, prop); - dba = dbms.active(); - EXPECT_EQ(2, CountIterable(dba->vertices(false))); - // MATCH (n :label) WHERE null < n.prop - AstTreeStorage storage; - SymbolTable symbol_table; - auto scan_all = MakeScanAllByLabelPropertyRange( - storage, symbol_table, "n", label, prop, - Bound{LITERAL(TypedValue::Null), Bound::Type::EXCLUSIVE}, - std::experimental::nullopt); - // RETURN n - auto output = NEXPR("n", IDENT("n")); - auto produce = MakeProduce(scan_all.op_, output); - symbol_table[*output->expression_] = scan_all.sym_; - symbol_table[*output] = symbol_table.CreateSymbol("n", true); - auto results = CollectProduce(produce.get(), symbol_table, *dba); - EXPECT_EQ(results.size(), 0); + auto check = [&dba, label, prop](TypedValue lower, Bound::Type lower_type, + TypedValue upper, Bound::Type upper_type, + const std::vector &expected) { + AstTreeStorage storage; + SymbolTable symbol_table; + auto scan_all = MakeScanAllByLabelPropertyRange( + storage, symbol_table, "n", label, prop, + Bound{LITERAL(lower), lower_type}, Bound{LITERAL(upper), upper_type}); + // RETURN n + auto output = NEXPR("n", IDENT("n")); + auto produce = MakeProduce(scan_all.op_, output); + symbol_table[*output->expression_] = scan_all.sym_; + symbol_table[*output] = symbol_table.CreateSymbol("n", true); + auto results = CollectProduce(produce.get(), symbol_table, *dba); + ASSERT_EQ(results.size(), expected.size()); + for (int i = 0; i < expected.size(); i++) { + TypedValue equal = + results[i][0].Value().PropsAt(prop) == expected[i]; + ASSERT_EQ(equal.type(), TypedValue::Type::Bool); + EXPECT_TRUE(equal.Value()); + } + }; + + // normal ranges that return something + check(false, Bound::Type::INCLUSIVE, true, Bound::Type::EXCLUSIVE, {false}); + check(false, Bound::Type::EXCLUSIVE, true, Bound::Type::INCLUSIVE, {true}); + check("a", Bound::Type::EXCLUSIVE, "c", Bound::Type::EXCLUSIVE, {"b"}); + check(0, Bound::Type::EXCLUSIVE, 2, Bound::Type::INCLUSIVE, {0.5, 1, 1.5, 2}); + check(1.5, Bound::Type::EXCLUSIVE, 2.5, Bound::Type::INCLUSIVE, {2, 2.5}); + check(std::vector{0.5}, Bound::Type::EXCLUSIVE, + std::vector{1.5}, Bound::Type::INCLUSIVE, + {TypedValue(std::vector{1})}); + + // when a range contains different types, nothing should get returned + for (const auto &value_a : values) + for (const auto &value_b : values) { + if (PropertyValue::AreComparableTypes( + static_cast(value_a).type(), + static_cast(value_b).type())) + continue; + check(value_a, Bound::Type::INCLUSIVE, value_b, Bound::Type::INCLUSIVE, + {}); + } + + // it's not allowed to have Null as a bound, we assert against that + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + EXPECT_DEATH(check(TypedValue::Null, Bound::Type::INCLUSIVE, 0, + Bound::Type::INCLUSIVE, {}), + "Null value is not a valid index bound"); + EXPECT_DEATH(check(0, Bound::Type::INCLUSIVE, TypedValue::Null, + Bound::Type::INCLUSIVE, {}), + "Null value is not a valid index bound"); } TEST(QueryPlan, ScanAllByLabelPropertyEqualityNoError) {