From edaa03a960324ab38fa0208e4e244cf63152dffd Mon Sep 17 00:00:00 2001
From: Teon Banek <teon.banek@memgraph.io>
Date: Tue, 12 Nov 2019 15:41:16 +0100
Subject: [PATCH] Expose explicit Label index creation through Cypher
Reviewers: mferencevic, ipaljak
Reviewed By: mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D2547
---
.../frontend/ast/cypher_main_visitor.cpp | 12 ++++++----
.../frontend/opencypher/grammar/Cypher.g4 | 4 ++--
src/query/interpreter.cpp | 24 ++++++++++++++-----
src/query/plan/rewrite/index_lookup.hpp | 24 +++++++++++++------
src/query/plan/vertex_count_cache.hpp | 4 ++++
tests/manual/interactive_planning.cpp | 2 ++
tests/unit/query_plan.cpp | 22 +++++++++++++----
tests/unit/query_plan_checker.hpp | 4 ++++
8 files changed, 73 insertions(+), 23 deletions(-)
diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp
index c9c8a3864..c5621f471 100644
--- a/src/query/frontend/ast/cypher_main_visitor.cpp
+++ b/src/query/frontend/ast/cypher_main_visitor.cpp
@@ -154,8 +154,10 @@ antlrcpp::Any CypherMainVisitor::visitCreateIndex(
auto *index_query = storage_->Create<IndexQuery>();
index_query->action_ = IndexQuery::Action::CREATE;
index_query->label_ = AddLabel(ctx->labelName()->accept(this));
- PropertyIx name_key = ctx->propertyKeyName()->accept(this);
- index_query->properties_ = {name_key};
+ if (ctx->propertyKeyName()) {
+ PropertyIx name_key = ctx->propertyKeyName()->accept(this);
+ index_query->properties_ = {name_key};
+ }
return index_query;
}
@@ -163,8 +165,10 @@ antlrcpp::Any CypherMainVisitor::visitDropIndex(
MemgraphCypher::DropIndexContext *ctx) {
auto *index_query = storage_->Create<IndexQuery>();
index_query->action_ = IndexQuery::Action::DROP;
- PropertyIx key = ctx->propertyKeyName()->accept(this);
- index_query->properties_ = {key};
+ if (ctx->propertyKeyName()) {
+ PropertyIx key = ctx->propertyKeyName()->accept(this);
+ index_query->properties_ = {key};
+ }
index_query->label_ = AddLabel(ctx->labelName()->accept(this));
return index_query;
}
diff --git a/src/query/frontend/opencypher/grammar/Cypher.g4 b/src/query/frontend/opencypher/grammar/Cypher.g4
index d34cdf5d5..554449584 100644
--- a/src/query/frontend/opencypher/grammar/Cypher.g4
+++ b/src/query/frontend/opencypher/grammar/Cypher.g4
@@ -306,9 +306,9 @@ integerLiteral : DecimalLiteral
| HexadecimalLiteral
;
-createIndex : CREATE INDEX ON ':' labelName '(' propertyKeyName ')' ;
+createIndex : CREATE INDEX ON ':' labelName ( '(' propertyKeyName ')' )? ;
-dropIndex : DROP INDEX ON ':' labelName '(' propertyKeyName ')' ;
+dropIndex : DROP INDEX ON ':' labelName ( '(' propertyKeyName ')' )? ;
doubleLiteral : FloatingLiteral ;
diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp
index 32d527c8d..7b7fab805 100644
--- a/src/query/interpreter.cpp
+++ b/src/query/interpreter.cpp
@@ -850,15 +850,21 @@ PreparedQuery PrepareIndexQuery(
#ifdef MG_SINGLE_NODE_V2
handler = [interpreter_context, label, properties = std::move(properties),
invalidate_plan_cache = std::move(invalidate_plan_cache)] {
- CHECK(properties.size() == 1);
- interpreter_context->db->CreateIndex(label, properties[0]);
+ if (properties.empty()) {
+ interpreter_context->db->CreateIndex(label);
+ } else {
+ CHECK(properties.size() == 1U);
+ interpreter_context->db->CreateIndex(label, properties[0]);
+ }
invalidate_plan_cache();
};
#else
handler = [dba, label, properties = std::move(properties),
invalidate_plan_cache = std::move(invalidate_plan_cache)] {
+ // Old storage creates label index by default.
+ if (properties.empty()) return;
try {
- CHECK(properties.size() == 1);
+ CHECK(properties.size() == 1U);
dba->CreateIndex(label, properties[0]);
invalidate_plan_cache();
} catch (const database::ConstraintViolationException &e) {
@@ -876,15 +882,21 @@ PreparedQuery PrepareIndexQuery(
#ifdef MG_SINGLE_NODE_V2
handler = [interpreter_context, label, properties = std::move(properties),
invalidate_plan_cache = std::move(invalidate_plan_cache)] {
- CHECK(properties.size() == 1);
- interpreter_context->db->DropIndex(label, properties[0]);
+ if (properties.empty()) {
+ interpreter_context->db->DropIndex(label);
+ } else {
+ CHECK(properties.size() == 1U);
+ interpreter_context->db->DropIndex(label, properties[0]);
+ }
invalidate_plan_cache();
};
#else
handler = [dba, label, properties = std::move(properties),
invalidate_plan_cache = std::move(invalidate_plan_cache)] {
+ if (properties.empty())
+ throw QueryRuntimeException("Label index cannot be dropped!");
try {
- CHECK(properties.size() == 1);
+ CHECK(properties.size() == 1U);
dba->DropIndex(label, properties[0]);
invalidate_plan_cache();
} catch (const database::TransactionException &e) {
diff --git a/src/query/plan/rewrite/index_lookup.hpp b/src/query/plan/rewrite/index_lookup.hpp
index 1ad7a07d0..c8bac9475 100644
--- a/src/query/plan/rewrite/index_lookup.hpp
+++ b/src/query/plan/rewrite/index_lookup.hpp
@@ -435,14 +435,22 @@ class IndexLookupRewriter final : public HierarchicalLogicalOperatorVisitor {
return db_->NameToProperty(prop.name);
}
- LabelIx FindBestLabelIndex(const std::unordered_set<LabelIx> &labels) {
+ std::optional<LabelIx> FindBestLabelIndex(
+ const std::unordered_set<LabelIx> &labels) {
CHECK(!labels.empty())
<< "Trying to find the best label without any labels.";
- return *std::min_element(labels.begin(), labels.end(),
- [this](const auto &label1, const auto &label2) {
- return db_->VerticesCount(GetLabel(label1)) <
- db_->VerticesCount(GetLabel(label2));
- });
+ std::optional<LabelIx> best_label;
+ for (const auto &label : labels) {
+ if (!db_->LabelIndexExists(GetLabel(label))) continue;
+ if (!best_label) {
+ best_label = label;
+ continue;
+ }
+ if (db_->VerticesCount(GetLabel(label)) <
+ db_->VerticesCount(GetLabel(*best_label)))
+ best_label = label;
+ }
+ return best_label;
}
// Finds the label-property combination which has indexed the lowest amount of
@@ -560,7 +568,9 @@ class IndexLookupRewriter final : public HierarchicalLogicalOperatorVisitor {
prop_filter.value_, view);
}
}
- auto label = FindBestLabelIndex(labels);
+ auto maybe_label = FindBestLabelIndex(labels);
+ if (!maybe_label) return nullptr;
+ const auto &label = *maybe_label;
if (max_vertex_count &&
db_->VerticesCount(GetLabel(label)) > *max_vertex_count) {
// Don't create an indexed lookup, since we have more labeled vertices
diff --git a/src/query/plan/vertex_count_cache.hpp b/src/query/plan/vertex_count_cache.hpp
index dfaf3b479..50e2ca618 100644
--- a/src/query/plan/vertex_count_cache.hpp
+++ b/src/query/plan/vertex_count_cache.hpp
@@ -69,6 +69,10 @@ class VertexCountCache {
return bounds_vertex_count.at(bounds);
}
+ bool LabelIndexExists(storage::Label label) {
+ return db_->LabelIndexExists(label);
+ }
+
bool LabelPropertyIndexExists(storage::Label label,
storage::Property property) {
return db_->LabelPropertyIndexExists(label, property);
diff --git a/tests/manual/interactive_planning.cpp b/tests/manual/interactive_planning.cpp
index b833400ac..d1fe621f8 100644
--- a/tests/manual/interactive_planning.cpp
+++ b/tests/manual/interactive_planning.cpp
@@ -200,6 +200,8 @@ class InteractiveDbAccessor {
"' in range " + range_string.str());
}
+ bool LabelIndexExists(storage::Label label) { return true; }
+
bool LabelPropertyIndexExists(storage::Label label_id,
storage::Property property_id) {
auto label = dba_->LabelName(label_id);
diff --git a/tests/unit/query_plan.cpp b/tests/unit/query_plan.cpp
index 1b2d35d64..0afede6d3 100644
--- a/tests/unit/query_plan.cpp
+++ b/tests/unit/query_plan.cpp
@@ -167,10 +167,21 @@ TYPED_TEST(TestPlanner, MatchLabeledNodes) {
auto *as_n = NEXPR("n", IDENT("n"));
auto *query =
QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("n", label))), RETURN(as_n)));
- auto symbol_table = query::MakeSymbolTable(query);
- auto planner = MakePlanner<TypeParam>(&dba, storage, symbol_table, query);
- CheckPlan(planner.plan(), symbol_table, ExpectScanAllByLabel(),
- ExpectProduce());
+ {
+ // Without created label index
+ auto symbol_table = query::MakeSymbolTable(query);
+ auto planner = MakePlanner<TypeParam>(&dba, storage, symbol_table, query);
+ CheckPlan(planner.plan(), symbol_table, ExpectScanAll(), ExpectFilter(),
+ ExpectProduce());
+ }
+ {
+ // With created label index
+ dba.SetIndexCount(dba.Label(label), 0);
+ auto symbol_table = query::MakeSymbolTable(query);
+ auto planner = MakePlanner<TypeParam>(&dba, storage, symbol_table, query);
+ CheckPlan(planner.plan(), symbol_table, ExpectScanAllByLabel(),
+ ExpectProduce());
+ }
}
TYPED_TEST(TestPlanner, MatchPathReturn) {
@@ -1130,6 +1141,7 @@ TYPED_TEST(TestPlanner, UnableToUsePropertyIndex) {
FakeDbAccessor dba;
auto label = dba.Label("label");
auto property = dba.Property("property");
+ dba.SetIndexCount(label, 0);
dba.SetIndexCount(label, property, 0);
AstStorage storage;
auto *query = QUERY(SINGLE_QUERY(
@@ -1149,6 +1161,7 @@ TYPED_TEST(TestPlanner, SecondPropertyIndex) {
FakeDbAccessor dba;
auto label = dba.Label("label");
auto property = PROPERTY_PAIR("property");
+ dba.SetIndexCount(label, 0);
dba.SetIndexCount(label, dba.Property("property"), 0);
AstStorage storage;
auto n_prop = PROPERTY_LOOKUP("n", property);
@@ -1296,6 +1309,7 @@ TYPED_TEST(TestPlanner, MatchDoubleScanToExpandExisting) {
// Test MATCH (n) -[r]- (m :label) RETURN r
FakeDbAccessor dba;
auto label = "label";
+ dba.SetIndexCount(dba.Label(label), 0);
AstStorage storage;
auto *query = QUERY(SINGLE_QUERY(
MATCH(PATTERN(NODE("n"), EDGE("r"), NODE("m", label))), RETURN("r")));
diff --git a/tests/unit/query_plan_checker.hpp b/tests/unit/query_plan_checker.hpp
index 897d7a939..f315a81cd 100644
--- a/tests/unit/query_plan_checker.hpp
+++ b/tests/unit/query_plan_checker.hpp
@@ -397,6 +397,10 @@ class FakeDbAccessor {
return 0;
}
+ bool LabelIndexExists(storage::Label label) const {
+ return label_index_.find(label) != label_index_.end();
+ }
+
bool LabelPropertyIndexExists(storage::Label label,
storage::Property property) const {
for (auto &index : label_property_index_) {