diff --git a/src/database/distributed/distributed_graph_db.cpp b/src/database/distributed/distributed_graph_db.cpp index 5299c3fb9..0135b8002 100644 --- a/src/database/distributed/distributed_graph_db.cpp +++ b/src/database/distributed/distributed_graph_db.cpp @@ -483,7 +483,7 @@ class WorkerAccessor final : public DistributedAccessor { DistributedEdgeAccessor *edge_accessor) : DistributedAccessor(db, tx_id, vertex_accessor, edge_accessor) {} - void BuildIndex(storage::Label, storage::Property) override { + void BuildIndex(storage::Label, storage::Property, bool) override { // TODO: Rethink BuildIndex API or inheritance. It's rather strange that a // derived type blocks this functionality. LOG(FATAL) << "BuildIndex invoked on worker."; diff --git a/src/database/distributed/graph_db_accessor.cpp b/src/database/distributed/graph_db_accessor.cpp index fe6a1095a..7f905d736 100644 --- a/src/database/distributed/graph_db_accessor.cpp +++ b/src/database/distributed/graph_db_accessor.cpp @@ -111,9 +111,10 @@ EdgeAccessor GraphDbAccessor::FindEdge(gid::Gid gid, bool current_state) { } void GraphDbAccessor::BuildIndex(storage::Label label, - storage::Property property) { + storage::Property property, + bool unique) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - + if (unique) throw utils::NotYetImplemented("Distributed unique index"); db_.storage().index_build_tx_in_progress_.access().insert(transaction_.id_); // on function exit remove the create index transaction from diff --git a/src/database/distributed/graph_db_accessor.hpp b/src/database/distributed/graph_db_accessor.hpp index 658d27ca1..a336810dd 100644 --- a/src/database/distributed/graph_db_accessor.hpp +++ b/src/database/distributed/graph_db_accessor.hpp @@ -22,6 +22,11 @@ namespace database { +/** Thrown when inserting in an index with constraint. */ +class IndexConstraintViolationException : public utils::BasicException { + using utils::BasicException::BasicException; +}; + /** Thrown when creating an index which already exists. */ class IndexExistsException : public utils::BasicException { using utils::BasicException::BasicException; @@ -444,7 +449,8 @@ class GraphDbAccessor { * @param label - label to build for * @param property - property to build for */ - virtual void BuildIndex(storage::Label label, storage::Property property); + virtual void BuildIndex(storage::Label label, storage::Property property, + bool); /// Populates index with vertices containing the key void PopulateIndex(const LabelPropertyIndex::Key &key); diff --git a/src/database/single_node/graph_db_accessor.cpp b/src/database/single_node/graph_db_accessor.cpp index bbaedabad..575589907 100644 --- a/src/database/single_node/graph_db_accessor.cpp +++ b/src/database/single_node/graph_db_accessor.cpp @@ -106,7 +106,7 @@ EdgeAccessor GraphDbAccessor::FindEdge(gid::Gid gid, bool current_state) { } void GraphDbAccessor::BuildIndex(storage::Label label, - storage::Property property) { + storage::Property property, bool unique) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; db_.storage().index_build_tx_in_progress_.access().insert(transaction_.id_); @@ -120,13 +120,15 @@ void GraphDbAccessor::BuildIndex(storage::Label label, }); // Create the index - const LabelPropertyIndex::Key key(label, property); + const LabelPropertyIndex::Key key(label, property, unique); if (db_.storage().label_property_index_.CreateIndex(key) == false) { throw IndexExistsException( "Index is either being created by another transaction or already " "exists."); } + // TODO (msantl): If unique constraint, lock the tx engine + // Everything that happens after the line above ended will be added to the // index automatically, but we still have to add to index everything that // happened earlier. We have to first wait for every transaction that @@ -164,7 +166,12 @@ void GraphDbAccessor::BuildIndex(storage::Label label, DCHECK(removed) << "Index building (read) transaction should be inside set"; }); - dba->PopulateIndex(key); + try { + dba->PopulateIndex(key); + } catch (const IndexConstraintViolationException &) { + db_.storage().label_property_index_.DeleteIndex(key); + throw; + } dba->EnableIndex(key); dba->Commit(); @@ -178,7 +185,7 @@ void GraphDbAccessor::EnableIndex(const LabelPropertyIndex::Key &key) { auto wal_build_index_tx_id = transaction_id(); wal().Emplace(database::StateDelta::BuildIndex( wal_build_index_tx_id, key.label_, LabelName(key.label_), key.property_, - PropertyName(key.property_))); + PropertyName(key.property_), key.unique_)); // After these two operations we are certain that everything is contained in // the index under the assumption that the original index creation transaction @@ -190,8 +197,11 @@ void GraphDbAccessor::PopulateIndex(const LabelPropertyIndex::Key &key) { for (auto vertex : Vertices(key.label_, false)) { if (vertex.PropsAt(key.property_).type() == PropertyValue::Type::Null) continue; - db_.storage().label_property_index_.UpdateOnLabelProperty(vertex.address(), - vertex.current_); + if (!db_.storage().label_property_index_.UpdateOnLabelProperty( + vertex.address(), vertex.current_)) { + throw IndexConstraintViolationException( + "Index couldn't be populated due to constraint violation!"); + } } } @@ -200,16 +210,24 @@ void GraphDbAccessor::UpdateLabelIndices(storage::Label label, const Vertex *const vertex) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; auto *vlist_ptr = vertex_accessor.address(); + + if (!db_.storage().label_property_index_.UpdateOnLabel(label, vlist_ptr, + vertex)) { + throw IndexConstraintViolationException( + "Index couldn't be updated due to constraint violation!"); + } db_.storage().labels_index_.Update(label, vlist_ptr, vertex); - db_.storage().label_property_index_.UpdateOnLabel(label, vlist_ptr, vertex); } void GraphDbAccessor::UpdatePropertyIndex( storage::Property property, const RecordAccessor &vertex_accessor, const Vertex *const vertex) { DCHECK(!commited_ && !aborted_) << "Accessor committed or aborted"; - db_.storage().label_property_index_.UpdateOnProperty( - property, vertex_accessor.address(), vertex); + if (!db_.storage().label_property_index_.UpdateOnProperty( + property, vertex_accessor.address(), vertex)) { + throw IndexConstraintViolationException( + "Index couldn't be updated due to constraint violation!"); + } } int64_t GraphDbAccessor::VerticesCount() const { @@ -419,8 +437,9 @@ std::vector GraphDbAccessor::IndexInfo() const { } for (LabelPropertyIndex::Key key : db_.storage().label_property_index_.Keys()) { - info.emplace_back(fmt::format(":{}({})", LabelName(key.label_), - PropertyName(key.property_))); + info.emplace_back(fmt::format(":{}({}){}", LabelName(key.label_), + PropertyName(key.property_), + key.unique_ ? " unique" : "")); } return info; } diff --git a/src/database/single_node/graph_db_accessor.hpp b/src/database/single_node/graph_db_accessor.hpp index 4a6ffaa6d..9ae68bce8 100644 --- a/src/database/single_node/graph_db_accessor.hpp +++ b/src/database/single_node/graph_db_accessor.hpp @@ -21,6 +21,11 @@ namespace database { +/** Thrown when inserting in an index with constraint. */ +class IndexConstraintViolationException : public utils::BasicException { + using utils::BasicException::BasicException; +}; + /** Thrown when creating an index which already exists. */ class IndexExistsException : public utils::BasicException { using utils::BasicException::BasicException; @@ -420,7 +425,7 @@ class GraphDbAccessor { * @param label - label to build for * @param property - property to build for */ - void BuildIndex(storage::Label label, storage::Property property); + void BuildIndex(storage::Label label, storage::Property property, bool unique); /// Populates index with vertices containing the key void PopulateIndex(const LabelPropertyIndex::Key &key); diff --git a/src/durability/single_node/recovery.cpp b/src/durability/single_node/recovery.cpp index 06b5fc000..233a68f35 100644 --- a/src/durability/single_node/recovery.cpp +++ b/src/durability/single_node/recovery.cpp @@ -141,9 +141,11 @@ bool RecoverSnapshot(const fs::path &snapshot_file, database::GraphDb *db, auto label = *it++; RETURN_IF_NOT(it != index_value.end()); auto property = *it++; - RETURN_IF_NOT(label.IsString() && property.IsString()); - recovery_data->indexes.emplace_back(label.ValueString(), - property.ValueString()); + RETURN_IF_NOT(it != index_value.end()); + auto unique = *it++; + RETURN_IF_NOT(label.IsString() && property.IsString() && unique.IsBool()); + recovery_data->indexes.emplace_back(IndexRecoveryData{ + label.ValueString(), property.ValueString(), unique.ValueBool()}); } auto dba = db->Access(); @@ -400,8 +402,8 @@ void RecoverWal(const fs::path &durability_dir, database::GraphDb *db, break; case database::StateDelta::Type::BUILD_INDEX: // TODO index building might still be problematic in HA - recovery_data->indexes.emplace_back(delta.label_name, - delta.property_name); + recovery_data->indexes.emplace_back(IndexRecoveryData{ + delta.label_name, delta.property_name, delta.unique}); break; default: transactions->Apply(delta); @@ -416,14 +418,13 @@ void RecoverWal(const fs::path &durability_dir, database::GraphDb *db, db->tx_engine().EnsureNextIdGreater(max_observed_tx_id); } -void RecoverIndexes( - database::GraphDb *db, - const std::vector> &indexes) { +void RecoverIndexes(database::GraphDb *db, + const std::vector &indexes) { auto db_accessor_indices = db->Access(); - for (const auto &label_prop : indexes) { + for (const auto &index : indexes) { const database::LabelPropertyIndex::Key key{ - db_accessor_indices->Label(label_prop.first), - db_accessor_indices->Property(label_prop.second)}; + db_accessor_indices->Label(index.label), + db_accessor_indices->Property(index.property), index.unique}; db_accessor_indices->db().storage().label_property_index().CreateIndex(key); db_accessor_indices->PopulateIndex(key); db_accessor_indices->EnableIndex(key); diff --git a/src/durability/single_node/recovery.hpp b/src/durability/single_node/recovery.hpp index 6b40a6c62..5124b3272 100644 --- a/src/durability/single_node/recovery.hpp +++ b/src/durability/single_node/recovery.hpp @@ -36,6 +36,12 @@ struct RecoveryInfo { bool operator!=(const RecoveryInfo &other) const { return !(*this == other); } }; +struct IndexRecoveryData { + std::string label; + std::string property; + bool unique; +}; + // A data structure for exchanging info between main recovery function and // snapshot and WAL recovery functions. struct RecoveryData { @@ -44,7 +50,7 @@ struct RecoveryData { std::vector snapshooter_tx_snapshot; // A collection into which the indexes should be added so they // can be rebuilt at the end of the recovery transaction. - std::vector> indexes; + std::vector indexes; void Clear() { snapshooter_tx_id = 0; @@ -135,11 +141,10 @@ class RecoveryTransactions { }; void RecoverWal(const std::experimental::filesystem::path &durability_dir, - database::GraphDb *db, RecoveryData *recovery_data, - RecoveryTransactions *transactions); + database::GraphDb *db, RecoveryData *recovery_data, + RecoveryTransactions *transactions); -void RecoverIndexes( - database::GraphDb *db, - const std::vector> &indexes); +void RecoverIndexes(database::GraphDb *db, + const std::vector &indexes); } // namespace durability diff --git a/src/durability/single_node/snapshooter.cpp b/src/durability/single_node/snapshooter.cpp index c878eb661..bfa50301d 100644 --- a/src/durability/single_node/snapshooter.cpp +++ b/src/durability/single_node/snapshooter.cpp @@ -50,6 +50,7 @@ bool Encode(const fs::path &snapshot_file, database::GraphDb &db, for (const auto &key : dba.GetIndicesKeys()) { index_vec.emplace_back(dba.LabelName(key.label_)); index_vec.emplace_back(dba.PropertyName(key.property_)); + index_vec.emplace_back(key.unique_); } encoder.WriteList(index_vec); } diff --git a/src/durability/single_node/state_delta.cpp b/src/durability/single_node/state_delta.cpp index 18c653813..ec48a1391 100644 --- a/src/durability/single_node/state_delta.cpp +++ b/src/durability/single_node/state_delta.cpp @@ -103,12 +103,14 @@ StateDelta StateDelta::RemoveEdge(tx::TransactionId tx_id, gid::Gid edge_id) { StateDelta StateDelta::BuildIndex(tx::TransactionId tx_id, storage::Label label, const std::string &label_name, storage::Property property, - const std::string &property_name) { + const std::string &property_name, + bool unique) { StateDelta op(StateDelta::Type::BUILD_INDEX, tx_id); op.label = label; op.label_name = label_name; op.property = property; op.property_name = property_name; + op.unique = unique; return op; } @@ -162,6 +164,7 @@ void StateDelta::Encode( encoder.WriteString(label_name); encoder.WriteInt(property.Id()); encoder.WriteString(property_name); + encoder.WriteBool(unique); break; } @@ -236,6 +239,7 @@ std::experimental::optional StateDelta::Decode( DECODE_MEMBER(label_name, ValueString) DECODE_MEMBER_CAST(property, ValueInt, storage::Property) DECODE_MEMBER(property_name, ValueString) + DECODE_MEMBER(unique, ValueBool) break; } diff --git a/src/durability/single_node/state_delta.lcp b/src/durability/single_node/state_delta.lcp index 5285d1ac8..0a3c6cfc9 100644 --- a/src/durability/single_node/state_delta.lcp +++ b/src/durability/single_node/state_delta.lcp @@ -42,7 +42,8 @@ cpp<# (value "PropertyValue" :initval "PropertyValue::Null") (label "storage::Label") (label-name "std::string") - (check-empty :bool)) + (check-empty :bool) + (unique :bool)) (:documentation "Describes single change to the database state. Used for durability (WAL) and state communication over network in HA and for distributed remote storage @@ -69,7 +70,7 @@ in StateDeltas.") remove-label ;; vertex_id, label, label_name remove-vertex ;; vertex_id, check_empty remove-edge ;; edge_id - build-index ;; label, label_name, property, property_name + build-index ;; label, label_name, property, property_name, unique ) (:documentation "Defines StateDelta type. For each type the comment indicates which values @@ -123,7 +124,7 @@ omitted in the comment.")) static StateDelta BuildIndex(tx::TransactionId tx_id, storage::Label label, const std::string &label_name, storage::Property property, - const std::string &property_name); + const std::string &property_name, bool unique); /// Applies CRUD delta to database accessor. Fails on other types of deltas void Apply(GraphDbAccessor &dba) const; diff --git a/src/query/common.hpp b/src/query/common.hpp index 5356cc706..87956ba80 100644 --- a/src/query/common.hpp +++ b/src/query/common.hpp @@ -4,6 +4,7 @@ #include #include +#include "database/graph_db_accessor.hpp" #include "query/exceptions.hpp" #include "query/frontend/ast/ast.hpp" #include "query/frontend/semantic/symbol.hpp" @@ -74,6 +75,8 @@ void PropsSetChecked(TRecordAccessor *record, const storage::Property &key, } catch (const RecordDeletedError &) { throw QueryRuntimeException( "Trying to set properties on a deleted graph element."); + } catch (const database::IndexConstraintViolationException &e) { + throw QueryRuntimeException(e.what()); } } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 2461364e8..c6bcd98a7 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -3037,15 +3037,19 @@ class CreateIndexCursor : public Cursor { if (ctx.in_explicit_transaction_) { throw IndexInMulticommandTxException(); } - if (self_.is_unique_) { - throw utils::NotYetImplemented("CREATE UNIQUE INDEX"); - } else { - try { - CHECK(self_.properties_.size() == 1U); - db_.BuildIndex(self_.label_, self_.properties_[0]); - } catch (const database::IndexExistsException &) { - // Ignore creating an existing index. + if (self_.properties_.size() > 1) + throw utils::NotYetImplemented("index on multiple properties"); + + try { + CHECK(self_.properties_.size() == 1U); + db_.BuildIndex(self_.label_, self_.properties_[0], self_.is_unique_); + } catch (const database::IndexConstraintViolationException &e) { + throw QueryRuntimeException(e.what()); + } catch (const database::IndexExistsException &e) { + if (self_.is_unique_) { + throw QueryRuntimeException(e.what()); } + // Otherwise ignore creating an existing index. } ctx.is_index_created_ = did_create_ = true; return true; diff --git a/src/storage/single_node/indexes/label_property_index.hpp b/src/storage/single_node/indexes/label_property_index.hpp index ce1d3e4f0..dd0bfb0f9 100644 --- a/src/storage/single_node/indexes/label_property_index.hpp +++ b/src/storage/single_node/indexes/label_property_index.hpp @@ -38,10 +38,14 @@ class LabelPropertyIndex { public: const storage::Label label_; const storage::Property property_; + bool unique_{false}; Key(storage::Label label, storage::Property property) : label_(label), property_(property) {} + Key(storage::Label label, storage::Property property, bool unique) + : label_(label), property_(property), unique_(unique) {} + // Comparison operators - we need them to keep this sorted inside skiplist. bool operator<(const Key &other) const { if (this->label_ != other.label_) return this->label_ < other.label_; @@ -83,25 +87,47 @@ class LabelPropertyIndex { ready_for_use_.access().insert(key); } + /** NOTE: All update methods aren't supporting the case where two threads + * try to update the index with the same value. If both of them conclude that + * the insert is valid, one will insert first and that makes the second insert + * invalid if the unique constraint set. + */ + /** * @brief - Updates all indexes which should contain this vertex. * @param vlist - pointer to vlist entry to add * @param vertex - pointer to vertex record entry to add (contained in vlist) */ - void UpdateOnLabelProperty(mvcc::VersionList *const vlist, + bool UpdateOnLabelProperty(mvcc::VersionList *const vlist, const Vertex *const vertex) { const auto &labels = vertex->labels_; - for (auto &index : indices_.access()) { + // We need to check if the given vertex can be inserted in all indexes + auto access = indices_.access(); + for (auto &index : access) { + if (!index.first.unique_) continue; + // Vertex has the given label + if (std::find(labels.begin(), labels.end(), index.first.label_) == + labels.end()) + continue; + auto prop = vertex->properties_.at(index.first.property_); + if (prop.type() != PropertyValue::Type::Null) { + if (!CheckUniqueConstraint(*index.second, prop, vlist, vertex)) { + return false; + } + } + } + + for (auto &index : access) { // Vertex has the given label if (std::find(labels.begin(), labels.end(), index.first.label_) == labels.end()) continue; auto prop = vertex->properties_.at(index.first.property_); if (prop.type() != PropertyValue::Type::Null) { - // Property exists and vertex should be added to skiplist. Insert(*index.second, prop, vlist, vertex); } } + return true; } /** @@ -112,10 +138,23 @@ class LabelPropertyIndex { * @param vlist - pointer to vlist entry to add * @param vertex - pointer to vertex record entry to add (contained in vlist) */ - void UpdateOnLabel(storage::Label label, + bool UpdateOnLabel(storage::Label label, mvcc::VersionList *const vlist, const Vertex *const vertex) { - for (auto &index : indices_.access()) { + // We need to check if the given vertex can be inserted in all indexes + auto access = indices_.access(); + for (auto &index : access) { + if (!index.first.unique_) continue; + if (index.first.label_ != label) continue; + auto prop = vertex->properties_.at(index.first.property_); + if (prop.type() != PropertyValue::Type::Null) { + if (!CheckUniqueConstraint(*index.second, prop, vlist, vertex)) { + return false; + } + } + } + + for (auto &index : access) { if (index.first.label_ != label) continue; auto prop = vertex->properties_.at(index.first.property_); if (prop.type() != PropertyValue::Type::Null) { @@ -123,6 +162,7 @@ class LabelPropertyIndex { Insert(*index.second, prop, vlist, vertex); } } + return true; } /** @@ -133,11 +173,28 @@ class LabelPropertyIndex { * @param vlist - pointer to vlist entry to add * @param vertex - pointer to vertex record entry to add (contained in vlist) */ - void UpdateOnProperty(storage::Property property, + bool UpdateOnProperty(storage::Property property, mvcc::VersionList *const vlist, const Vertex *const vertex) { const auto &labels = vertex->labels_; - for (auto &index : indices_.access()) { + + // We need to check if the given vertex can be inserted in all indexes + auto access = indices_.access(); + for (auto &index : access) { + if (!index.first.unique_) continue; + if (index.first.property_ != property) continue; + if (std::find(labels.begin(), labels.end(), index.first.label_) != + labels.end()) { + // Label exists and vertex should be added to skiplist. + if (!CheckUniqueConstraint(*index.second, + vertex->properties_.at(property), vlist, + vertex)) { + return false; + } + } + } + + for (auto &index : access) { if (index.first.property_ != property) continue; if (std::find(labels.begin(), labels.end(), index.first.label_) != labels.end()) { @@ -145,6 +202,7 @@ class LabelPropertyIndex { Insert(*index.second, vertex->properties_.at(property), vlist, vertex); } } + return true; } /** @@ -479,6 +537,28 @@ class LabelPropertyIndex { const Vertex *const record_{nullptr}; }; + /** + * @brief - Check if an insert is valid due to the unique constraint + * @param index - into which index to add + * @param value - value which to add + * @param vlist - pointer to vlist entry to add + * @param vertex - pointer to vertex record entry to add (contained in + * vlist) + * @param unique - unique constraint on index + * @return bool - true if valid, false otherwise + */ + bool CheckUniqueConstraint(SkipList &index, + const PropertyValue &value, + mvcc::VersionList *const vlist, + const Vertex *const vertex) { + auto access = index.access(); + auto it = access.find_or_larger(IndexEntry{value, nullptr, nullptr}); + if (it == access.end() || (IndexEntry::Less(it->value_, value) && + IndexEntry::Less(value, it->value_))) + return true; + return false; + } + /** * @brief - Insert value, vlist, vertex into corresponding index (key) if * the index exists. @@ -487,11 +567,13 @@ class LabelPropertyIndex { * @param vlist - pointer to vlist entry to add * @param vertex - pointer to vertex record entry to add (contained in * vlist) + * @param unique - unique constraint on index */ void Insert(SkipList &index, const PropertyValue &value, mvcc::VersionList *const vlist, const Vertex *const vertex) { - index.access().insert(IndexEntry(value, vlist, vertex)); + // Property exists and vertex should be added to skiplist. + index.access().insert(IndexEntry{value, vlist, vertex}); } /** diff --git a/tests/benchmark/query/planner.cpp b/tests/benchmark/query/planner.cpp index 9b3c193cf..0d7b74828 100644 --- a/tests/benchmark/query/planner.cpp +++ b/tests/benchmark/query/planner.cpp @@ -94,7 +94,7 @@ static auto CreateIndexedVertices(int index_count, int vertex_count, database::GraphDb &db) { auto label = db.Access()->Label("label"); auto prop = db.Access()->Property("prop"); - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); auto dba = db.Access(); for (int vi = 0; vi < vertex_count; ++vi) { for (int index = 0; index < index_count; ++index) { diff --git a/tests/unit/distributed_dynamic_worker.cpp b/tests/unit/distributed_dynamic_worker.cpp index 308feaedf..06ade932e 100644 --- a/tests/unit/distributed_dynamic_worker.cpp +++ b/tests/unit/distributed_dynamic_worker.cpp @@ -87,7 +87,7 @@ TEST_F(DistributedDynamicWorker, IndexExistsOnNewWorker) { label = dba->Label("label"); property = dba->Property("property"); - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); EXPECT_TRUE(dba->LabelPropertyIndexExists(label, property)); EXPECT_EQ(CountIterable(dba->Vertices(label, property, false)), 100); } @@ -149,7 +149,7 @@ TEST_F(DistributedDynamicWorker, IndexExistsOnNewWorkerAfterRecovery) { label = dba->Label("label"); property = dba->Property("property"); - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); EXPECT_TRUE(dba->LabelPropertyIndexExists(label, property)); } diff --git a/tests/unit/distributed_graph_db.cpp b/tests/unit/distributed_graph_db.cpp index eb7908e89..eb45973ec 100644 --- a/tests/unit/distributed_graph_db.cpp +++ b/tests/unit/distributed_graph_db.cpp @@ -154,7 +154,7 @@ TEST_F(DistributedGraphDb, BuildIndexDistributed) { { auto dba = master().Access(); - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); EXPECT_TRUE(dba->LabelPropertyIndexExists(label, property)); EXPECT_EQ(CountIterable(dba->Vertices(label, property, false)), 100); } @@ -201,7 +201,7 @@ TEST_F(DistributedGraphDb, BuildIndexConcurrentInsert) { std::this_thread::sleep_for(0.5s); { auto dba = master().Access(); - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); EXPECT_TRUE(dba->LabelPropertyIndexExists(label, property)); } diff --git a/tests/unit/durability.cpp b/tests/unit/durability.cpp index 83cba1c90..365d40d52 100644 --- a/tests/unit/durability.cpp +++ b/tests/unit/durability.cpp @@ -52,7 +52,7 @@ class DbGenerator { void BuildIndex(int seq_number) { dba_.BuildIndex(Label(seq_number % kLabelCount), - Property(seq_number % kPropertyCount)); + Property(seq_number % kPropertyCount), false); } EdgeAccessor RandomEdge(bool remove_from_ids = false) { @@ -355,7 +355,7 @@ TEST_F(Durability, WalEncoding) { auto e0 = dba->InsertEdge(v0, v1, dba->EdgeType("et0")); ASSERT_EQ(e0.gid(), gid0); e0.PropsSet(dba->Property("p0"), std::vector{1, 2, 3}); - dba->BuildIndex(dba->Label("l1"), dba->Property("p1")); + dba->BuildIndex(dba->Label("l1"), dba->Property("p1"), false); dba->Commit(); db.wal().Flush(); @@ -438,7 +438,7 @@ TEST_F(Durability, SnapshotEncoding) { e0.PropsSet(dba->Property("p0"), std::vector{1, 2, 3}); auto e1 = dba->InsertEdge(v2, v1, dba->EdgeType("et1")); ASSERT_EQ(e1.gid(), gid1); - dba->BuildIndex(dba->Label("l1"), dba->Property("p1")); + dba->BuildIndex(dba->Label("l1"), dba->Property("p1"), false); dba->Commit(); MakeSnapshot(db); } @@ -470,9 +470,10 @@ TEST_F(Durability, SnapshotEncoding) { ASSERT_TRUE(dv.IsList()); // Label property indices. decoder.ReadValue(&dv); - ASSERT_EQ(dv.ValueList().size(), 2); + ASSERT_EQ(dv.ValueList().size(), 3); EXPECT_EQ(dv.ValueList()[0].ValueString(), "l1"); EXPECT_EQ(dv.ValueList()[1].ValueString(), "p1"); + EXPECT_EQ(dv.ValueList()[2].ValueBool(), false); std::map decoded_vertices; @@ -895,3 +896,9 @@ TEST_F(Durability, MoveToBackupWal) { database::GraphDb db{DbConfig(true, false)}; ASSERT_TRUE(durability::ContainsDurabilityFiles(backup_dir_)); } + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + google::InitGoogleLogging(argv[0]); + return RUN_ALL_TESTS(); +} diff --git a/tests/unit/graph_db.cpp b/tests/unit/graph_db.cpp index 8238e1134..b7f040396 100644 --- a/tests/unit/graph_db.cpp +++ b/tests/unit/graph_db.cpp @@ -19,7 +19,7 @@ TEST(GraphDbTest, GarbageCollectIndices) { }; auto label = dba->Label("label"); auto property = dba->Property("property"); - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); commit(); auto vertex = dba->InsertVertex(); diff --git a/tests/unit/graph_db_accessor_index_api.cpp b/tests/unit/graph_db_accessor_index_api.cpp index a51d16e8b..e07e43ff4 100644 --- a/tests/unit/graph_db_accessor_index_api.cpp +++ b/tests/unit/graph_db_accessor_index_api.cpp @@ -103,7 +103,7 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuild) { AddVertex(0); Commit(); - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); Commit(); EXPECT_EQ(dba->VerticesCount(label, property), 1); @@ -111,8 +111,8 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuild) { // 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); + dba->BuildIndex(label2, property, false); + dba->BuildIndex(label, property2, false); Commit(); EXPECT_EQ(dba->VerticesCount(label, property), 1); @@ -121,12 +121,12 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuild) { } TEST_F(GraphDbAccessorIndex, LabelPropertyIndexBuildTwice) { - dba->BuildIndex(label, property); - EXPECT_THROW(dba->BuildIndex(label, property), utils::BasicException); + dba->BuildIndex(label, property, false); + EXPECT_THROW(dba->BuildIndex(label, property, false), utils::BasicException); } TEST_F(GraphDbAccessorIndex, LabelPropertyIndexCount) { - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); EXPECT_EQ(dba->VerticesCount(label, property), 0); EXPECT_EQ(Count(dba->Vertices(label, property, true)), 0); for (int i = 0; i < 14; ++i) AddVertex(0); @@ -144,7 +144,7 @@ TEST(GraphDbAccessorIndexApi, LabelPropertyBuildIndexConcurrent) { threads.emplace_back([&db, index]() { auto dba = db.Access(); dba->BuildIndex(dba->Label("l" + std::to_string(index)), - dba->Property("p" + std::to_string(index))); + dba->Property("p" + std::to_string(index)), false); }); } @@ -158,7 +158,7 @@ TEST(GraphDbAccessorIndexApi, LabelPropertyBuildIndexConcurrent) { x, testing::AllOf(testing::Ge(center - 2), testing::Le(center + 2))); TEST_F(GraphDbAccessorIndex, LabelPropertyValueCount) { - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); // add some vertices without the property for (int i = 0; i < 20; i++) AddVertex(); @@ -204,7 +204,7 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyValueCount) { #undef EXPECT_WITH_MARGIN TEST_F(GraphDbAccessorIndex, LabelPropertyValueIteration) { - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); Commit(); // insert 10 verties and and check visibility @@ -217,7 +217,7 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyValueIteration) { } TEST_F(GraphDbAccessorIndex, LabelPropertyValueSorting) { - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); Commit(); std::vector expected_property_value(50, 0); @@ -345,7 +345,7 @@ TEST_F(GraphDbAccessorIndex, LabelPropertyValueSorting) { class GraphDbAccessorIndexRange : public GraphDbAccessorIndex { protected: void SetUp() override { - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); for (int i = 0; i < 100; i++) AddVertex(i / 10); ASSERT_EQ(Count(dba->Vertices(false)), 0); @@ -426,3 +426,18 @@ TEST_F(GraphDbAccessorIndexRange, RangeInterationIncompatibleTypes) { EXPECT_EQ(Count(Vertices(nullopt, Inclusive(1000.0))), 100); EXPECT_EQ(Count(Vertices(Inclusive(0.0), nullopt)), 100); } + +TEST_F(GraphDbAccessorIndex, UniqueConstraintViolationOnInsert) { + dba->BuildIndex(label, property, true); + Commit(); + AddVertex(0); + EXPECT_THROW(AddVertex(0), database::IndexConstraintViolationException); +} + +TEST_F(GraphDbAccessorIndex, UniqueConstraintViolationOnBuild) { + AddVertex(0); + AddVertex(0); + Commit(); + EXPECT_THROW(dba->BuildIndex(label, property, true), + database::IndexConstraintViolationException); +} diff --git a/tests/unit/query_cost_estimator.cpp b/tests/unit/query_cost_estimator.cpp index 577c48f36..b69e740a9 100644 --- a/tests/unit/query_cost_estimator.cpp +++ b/tests/unit/query_cost_estimator.cpp @@ -39,7 +39,7 @@ class QueryCostEstimator : public ::testing::Test { void SetUp() { // create the index in the current db accessor and then swap it to a new one - dba->BuildIndex(label, property); + dba->BuildIndex(label, property, false); dba = db.Access(); } diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp index d05d86246..259bbb2ef 100644 --- a/tests/unit/query_expression_evaluator.cpp +++ b/tests/unit/query_expression_evaluator.cpp @@ -1397,11 +1397,18 @@ TEST_F(FunctionTest, IndexInfo) { EXPECT_EQ(info[0], ":l1"); } { - dba->BuildIndex(dba->Label("l1"), dba->Property("prop")); + dba->BuildIndex(dba->Label("l1"), dba->Property("prop"), false); auto info = ToList(EvaluateFunction("INDEXINFO", {})); EXPECT_EQ(info.size(), 2); EXPECT_THAT(info, testing::UnorderedElementsAre(":l1", ":l1(prop)")); } + { + dba->BuildIndex(dba->Label("l1"), dba->Property("prop1"), true); + auto info = ToList(EvaluateFunction("INDEXINFO", {})); + EXPECT_EQ(info.size(), 3); + EXPECT_THAT(info, testing::UnorderedElementsAre(":l1", ":l1(prop)", + ":l1(prop1) unique")); + } } TEST_F(FunctionTest, Id) { diff --git a/tests/unit/query_plan_create_set_remove_delete.cpp b/tests/unit/query_plan_create_set_remove_delete.cpp index 1ceadf348..a293084f6 100644 --- a/tests/unit/query_plan_create_set_remove_delete.cpp +++ b/tests/unit/query_plan_create_set_remove_delete.cpp @@ -966,20 +966,17 @@ TEST(QueryPlan, CreateIndex) { } TEST(QueryPlan, CreateUniqueIndex) { - // CREATE UNIQUE INDEX ON :Label(prop1, prop2) + // CREATE UNIQUE INDEX ON :Label(prop1) database::GraphDb db; auto dba = db.Access(); auto label = dba->Label("label"); auto prop1 = dba->Property("prop1"); - auto prop2 = dba->Property("prop2"); - std::vector properties{prop1, prop2}; + std::vector properties{prop1}; auto create_index = std::make_shared(label, properties, true); SymbolTable symbol_table; - EXPECT_THROW(PullAll(create_index, *dba, symbol_table), - utils::NotYetImplemented); - // TODO: Check unique index created - // EXPECT_EQ(PullAll(create_index, *dba, symbol_table), 1); + EXPECT_EQ(PullAll(create_index, *dba, symbol_table), 1); + EXPECT_TRUE(dba->LabelPropertyIndexExists(label, prop1)); } TEST(QueryPlan, DeleteSetProperty) { diff --git a/tests/unit/query_plan_match_filter_return.cpp b/tests/unit/query_plan_match_filter_return.cpp index 2c8f091cc..d4c0e0fd8 100644 --- a/tests/unit/query_plan_match_filter_return.cpp +++ b/tests/unit/query_plan_match_filter_return.cpp @@ -1684,7 +1684,7 @@ TEST(QueryPlan, ScanAllByLabelProperty) { vertex.PropsSet(prop, value); } dba->Commit(); - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); } auto dba = db.Access(); ASSERT_EQ(14, CountIterable(dba->Vertices(false))); @@ -1749,7 +1749,7 @@ TEST(QueryPlan, ScanAllByLabelPropertyEqualityNoError) { string_vertex.add_label(label); string_vertex.PropsSet(prop, "string"); dba->Commit(); - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); } auto dba = db.Access(); EXPECT_EQ(2, CountIterable(dba->Vertices(false))); @@ -1786,7 +1786,7 @@ TEST(QueryPlan, ScanAllByLabelPropertyValueError) { } dba->Commit(); } - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); auto dba = db.Access(); EXPECT_EQ(2, CountIterable(dba->Vertices(false))); // MATCH (m), (n :label {prop: m}) @@ -1814,7 +1814,7 @@ TEST(QueryPlan, ScanAllByLabelPropertyRangeError) { } dba->Commit(); } - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); auto dba = db.Access(); EXPECT_EQ(2, CountIterable(dba->Vertices(false))); // MATCH (m), (n :label {prop: m}) @@ -1866,7 +1866,7 @@ TEST(QueryPlan, ScanAllByLabelPropertyEqualNull) { vertex_with_prop.add_label(label); vertex_with_prop.PropsSet(prop, 42); dba->Commit(); - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); } auto dba = db.Access(); EXPECT_EQ(2, CountIterable(dba->Vertices(false))); @@ -1899,7 +1899,7 @@ TEST(QueryPlan, ScanAllByLabelPropertyRangeNull) { vertex_with_prop.add_label(label); vertex_with_prop.PropsSet(prop, 42); dba->Commit(); - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); } auto dba = db.Access(); EXPECT_EQ(2, CountIterable(dba->Vertices(false))); @@ -1929,7 +1929,7 @@ TEST(QueryPlan, ScanAllByLabelPropertyNoValueInIndexContinuation) { v.add_label(label); v.PropsSet(prop, 2); dba->Commit(); - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); } auto dba = db.Access(); EXPECT_EQ(1, CountIterable(dba->Vertices(false))); @@ -1968,7 +1968,7 @@ TEST(QueryPlan, ScanAllEqualsScanAllByLabelProperty) { dba->Commit(); } - db.Access()->BuildIndex(label, prop); + db.Access()->BuildIndex(label, prop, false); // Make sure there are `vertex_count` vertices {