From fcbacdc80de068c72c09197f00d54259a7449de6 Mon Sep 17 00:00:00 2001
From: gvolfing <gabor.volfinger@memgraph.io>
Date: Mon, 23 Jan 2023 11:56:58 +0100
Subject: [PATCH] Rename ScanAllByPrimaryKey operator, fix e2e fail
Rename ScanAllByPrimaryKey operator to ScanByPrimaryKey. Make the
LabelIndexExist function use the same functionality as PrimaryKeyExists
again, for now. Previously it was just returning false and before that
it used the same implementation as PrimaryKeyExist. The change to false
broke some existing e2e tests that relied on some label based indexing
operator being instantiated.
---
src/query/v2/plan/operator.cpp | 34 +++++++++----------
src/query/v2/plan/operator.lcp | 8 ++---
src/query/v2/plan/pretty_print.cpp | 10 +++---
src/query/v2/plan/pretty_print.hpp | 4 +--
src/query/v2/plan/read_write_type_checker.cpp | 2 +-
src/query/v2/plan/read_write_type_checker.hpp | 2 +-
src/query/v2/plan/rewrite/index_lookup.hpp | 6 ++--
src/query/v2/plan/vertex_count_cache.hpp | 2 +-
src/utils/event_counter.cpp | 4 +--
tests/unit/query_plan_checker.hpp | 6 ++--
tests/unit/query_plan_checker_v2.hpp | 8 ++---
tests/unit/query_v2_plan.cpp | 4 +--
12 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp
index 952f2bd94..fa20b4e48 100644
--- a/src/query/v2/plan/operator.cpp
+++ b/src/query/v2/plan/operator.cpp
@@ -93,7 +93,7 @@ extern const Event ScanAllByLabelOperator;
extern const Event ScanAllByLabelPropertyRangeOperator;
extern const Event ScanAllByLabelPropertyValueOperator;
extern const Event ScanAllByLabelPropertyOperator;
-extern const Event ScanAllByPrimaryKeyOperator;
+extern const Event ScanByPrimaryKeyOperator;
extern const Event ExpandOperator;
extern const Event ExpandVariableOperator;
extern const Event ConstructNamedPathOperator;
@@ -519,12 +519,12 @@ class DistributedScanAllAndFilterCursor : public Cursor {
std::optional<std::vector<Expression *>> filter_expressions_;
};
-class DistributedScanAllByPrimaryKeyCursor : public Cursor {
+class DistributedScanByPrimaryKeyCursor : public Cursor {
public:
- explicit DistributedScanAllByPrimaryKeyCursor(Symbol output_symbol, UniqueCursorPtr input_cursor, const char *op_name,
- storage::v3::LabelId label,
- std::optional<std::vector<Expression *>> filter_expressions,
- std::vector<Expression *> primary_key)
+ explicit DistributedScanByPrimaryKeyCursor(Symbol output_symbol, UniqueCursorPtr input_cursor, const char *op_name,
+ storage::v3::LabelId label,
+ std::optional<std::vector<Expression *>> filter_expressions,
+ std::vector<Expression *> primary_key)
: output_symbol_(output_symbol),
input_cursor_(std::move(input_cursor)),
op_name_(op_name),
@@ -586,8 +586,8 @@ class DistributedScanAllByPrimaryKeyCursor : public Cursor {
return false;
}
- void PullMultiple(MultiFrame &input_multi_frame, ExecutionContext &context) override {
- throw utils::NotYetImplemented("Multiframe version of ScanAllByPrimaryKey is yet to be implemented.");
+ void PullMultiple(MultiFrame & /*input_multi_frame*/, ExecutionContext & /*context*/) override {
+ throw utils::NotYetImplemented("Multiframe version of ScanByPrimaryKey is yet to be implemented.");
};
void Reset() override { input_cursor_->Reset(); }
@@ -703,21 +703,21 @@ UniqueCursorPtr ScanAllByLabelProperty::MakeCursor(utils::MemoryResource *mem) c
throw QueryRuntimeException("ScanAllByLabelProperty is not supported");
}
-ScanAllByPrimaryKey::ScanAllByPrimaryKey(const std::shared_ptr<LogicalOperator> &input, Symbol output_symbol,
- storage::v3::LabelId label, std::vector<query::v2::Expression *> primary_key,
- storage::v3::View view)
+ScanByPrimaryKey::ScanByPrimaryKey(const std::shared_ptr<LogicalOperator> &input, Symbol output_symbol,
+ storage::v3::LabelId label, std::vector<query::v2::Expression *> primary_key,
+ storage::v3::View view)
: ScanAll(input, output_symbol, view), label_(label), primary_key_(primary_key) {
MG_ASSERT(primary_key.front());
}
-ACCEPT_WITH_INPUT(ScanAllByPrimaryKey)
+ACCEPT_WITH_INPUT(ScanByPrimaryKey)
-UniqueCursorPtr ScanAllByPrimaryKey::MakeCursor(utils::MemoryResource *mem) const {
- EventCounter::IncrementCounter(EventCounter::ScanAllByPrimaryKeyOperator);
+UniqueCursorPtr ScanByPrimaryKey::MakeCursor(utils::MemoryResource *mem) const {
+ EventCounter::IncrementCounter(EventCounter::ScanByPrimaryKeyOperator);
- return MakeUniqueCursorPtr<DistributedScanAllByPrimaryKeyCursor>(mem, output_symbol_, input_->MakeCursor(mem),
- "ScanAllByPrimaryKey", label_,
- std::nullopt /*filter_expressions*/, primary_key_);
+ return MakeUniqueCursorPtr<DistributedScanByPrimaryKeyCursor>(mem, output_symbol_, input_->MakeCursor(mem),
+ "ScanByPrimaryKey", label_,
+ std::nullopt /*filter_expressions*/, primary_key_);
}
Expand::Expand(const std::shared_ptr<LogicalOperator> &input, Symbol input_symbol, Symbol node_symbol,
diff --git a/src/query/v2/plan/operator.lcp b/src/query/v2/plan/operator.lcp
index bc8b64e5d..d5a707c54 100644
--- a/src/query/v2/plan/operator.lcp
+++ b/src/query/v2/plan/operator.lcp
@@ -113,7 +113,7 @@ class ScanAllByLabel;
class ScanAllByLabelPropertyRange;
class ScanAllByLabelPropertyValue;
class ScanAllByLabelProperty;
-class ScanAllByPrimaryKey;
+class ScanByPrimaryKey;
class Expand;
class ExpandVariable;
class ConstructNamedPath;
@@ -144,7 +144,7 @@ class Foreach;
using LogicalOperatorCompositeVisitor = utils::CompositeVisitor<
Once, CreateNode, CreateExpand, ScanAll, ScanAllByLabel,
ScanAllByLabelPropertyRange, ScanAllByLabelPropertyValue,
- ScanAllByLabelProperty, ScanAllByPrimaryKey,
+ ScanAllByLabelProperty, ScanByPrimaryKey,
Expand, ExpandVariable, ConstructNamedPath, Filter, Produce, Delete,
SetProperty, SetProperties, SetLabels, RemoveProperty, RemoveLabels,
EdgeUniquenessFilter, Accumulate, Aggregate, Skip, Limit, OrderBy, Merge,
@@ -855,8 +855,8 @@ given label and property.
"ScanAll producing a single node with specified by the label and primary key")
(:public
#>cpp
- ScanAllByPrimaryKey() {}
- ScanAllByPrimaryKey(const std::shared_ptr<LogicalOperator> &input,
+ ScanByPrimaryKey() {}
+ ScanByPrimaryKey(const std::shared_ptr<LogicalOperator> &input,
Symbol output_symbol,
storage::v3::LabelId label,
std::vector<query::v2::Expression*> primary_key,
diff --git a/src/query/v2/plan/pretty_print.cpp b/src/query/v2/plan/pretty_print.cpp
index 762a1b19a..7eb1dd5a9 100644
--- a/src/query/v2/plan/pretty_print.cpp
+++ b/src/query/v2/plan/pretty_print.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -86,9 +86,9 @@ bool PlanPrinter::PreVisit(query::v2::plan::ScanAllByLabelProperty &op) {
return true;
}
-bool PlanPrinter::PreVisit(query::v2::plan::ScanAllByPrimaryKey &op) {
+bool PlanPrinter::PreVisit(query::v2::plan::ScanByPrimaryKey &op) {
WithPrintLn([&](auto &out) {
- out << "* ScanAllByPrimaryKey"
+ out << "* ScanByPrimaryKey"
<< " (" << op.output_symbol_.name() << " :" << request_router_->LabelToName(op.label_) << ")";
});
return true;
@@ -487,9 +487,9 @@ bool PlanToJsonVisitor::PreVisit(ScanAllByLabelProperty &op) {
return false;
}
-bool PlanToJsonVisitor::PreVisit(ScanAllByPrimaryKey &op) {
+bool PlanToJsonVisitor::PreVisit(ScanByPrimaryKey &op) {
json self;
- self["name"] = "ScanAllByPrimaryKey";
+ self["name"] = "ScanByPrimaryKey";
self["label"] = ToJson(op.label_, *request_router_);
self["output_symbol"] = ToJson(op.output_symbol_);
diff --git a/src/query/v2/plan/pretty_print.hpp b/src/query/v2/plan/pretty_print.hpp
index d1dad22b7..44686f8dc 100644
--- a/src/query/v2/plan/pretty_print.hpp
+++ b/src/query/v2/plan/pretty_print.hpp
@@ -67,7 +67,7 @@ class PlanPrinter : public virtual HierarchicalLogicalOperatorVisitor {
bool PreVisit(ScanAllByLabelPropertyValue &) override;
bool PreVisit(ScanAllByLabelPropertyRange &) override;
bool PreVisit(ScanAllByLabelProperty &) override;
- bool PreVisit(ScanAllByPrimaryKey &) override;
+ bool PreVisit(ScanByPrimaryKey &) override;
bool PreVisit(Expand &) override;
bool PreVisit(ExpandVariable &) override;
@@ -194,7 +194,7 @@ class PlanToJsonVisitor : public virtual HierarchicalLogicalOperatorVisitor {
bool PreVisit(ScanAllByLabelPropertyRange &) override;
bool PreVisit(ScanAllByLabelPropertyValue &) override;
bool PreVisit(ScanAllByLabelProperty &) override;
- bool PreVisit(ScanAllByPrimaryKey &) override;
+ bool PreVisit(ScanByPrimaryKey &) override;
bool PreVisit(Produce &) override;
bool PreVisit(Accumulate &) override;
diff --git a/src/query/v2/plan/read_write_type_checker.cpp b/src/query/v2/plan/read_write_type_checker.cpp
index d1e037d1f..3143cb1e0 100644
--- a/src/query/v2/plan/read_write_type_checker.cpp
+++ b/src/query/v2/plan/read_write_type_checker.cpp
@@ -35,7 +35,7 @@ PRE_VISIT(ScanAllByLabel, RWType::R, true)
PRE_VISIT(ScanAllByLabelPropertyRange, RWType::R, true)
PRE_VISIT(ScanAllByLabelPropertyValue, RWType::R, true)
PRE_VISIT(ScanAllByLabelProperty, RWType::R, true)
-PRE_VISIT(ScanAllByPrimaryKey, RWType::R, true)
+PRE_VISIT(ScanByPrimaryKey, RWType::R, true)
PRE_VISIT(Expand, RWType::R, true)
PRE_VISIT(ExpandVariable, RWType::R, true)
diff --git a/src/query/v2/plan/read_write_type_checker.hpp b/src/query/v2/plan/read_write_type_checker.hpp
index 62d7af8b9..efab0267a 100644
--- a/src/query/v2/plan/read_write_type_checker.hpp
+++ b/src/query/v2/plan/read_write_type_checker.hpp
@@ -59,7 +59,7 @@ class ReadWriteTypeChecker : public virtual HierarchicalLogicalOperatorVisitor {
bool PreVisit(ScanAllByLabelPropertyValue &) override;
bool PreVisit(ScanAllByLabelPropertyRange &) override;
bool PreVisit(ScanAllByLabelProperty &) override;
- bool PreVisit(ScanAllByPrimaryKey &) override;
+ bool PreVisit(ScanByPrimaryKey &) override;
bool PreVisit(Expand &) override;
bool PreVisit(ExpandVariable &) override;
diff --git a/src/query/v2/plan/rewrite/index_lookup.hpp b/src/query/v2/plan/rewrite/index_lookup.hpp
index 37c14129e..5b67b464e 100644
--- a/src/query/v2/plan/rewrite/index_lookup.hpp
+++ b/src/query/v2/plan/rewrite/index_lookup.hpp
@@ -273,12 +273,12 @@ class IndexLookupRewriter final : public HierarchicalLogicalOperatorVisitor {
return true;
}
- bool PreVisit(ScanAllByPrimaryKey &op) override {
+ bool PreVisit(ScanByPrimaryKey &op) override {
prev_ops_.push_back(&op);
return true;
}
- bool PostVisit(ScanAllByPrimaryKey &) override {
+ bool PostVisit(ScanByPrimaryKey &) override {
prev_ops_.pop_back();
return true;
}
@@ -639,7 +639,7 @@ class IndexLookupRewriter final : public HierarchicalLogicalOperatorVisitor {
std::vector<query::v2::Expression *> pk_expressions;
std::transform(primary_key.begin(), primary_key.end(), std::back_inserter(pk_expressions),
[](const auto &exp) { return exp.second.property_filter->value_; });
- return std::make_unique<ScanAllByPrimaryKey>(input, node_symbol, GetLabel(prim_label), pk_expressions);
+ return std::make_unique<ScanByPrimaryKey>(input, node_symbol, GetLabel(prim_label), pk_expressions);
}
}
diff --git a/src/query/v2/plan/vertex_count_cache.hpp b/src/query/v2/plan/vertex_count_cache.hpp
index 995584f08..bfe20ffe3 100644
--- a/src/query/v2/plan/vertex_count_cache.hpp
+++ b/src/query/v2/plan/vertex_count_cache.hpp
@@ -55,7 +55,7 @@ class VertexCountCache {
return 1;
}
- bool LabelIndexExists(storage::v3::LabelId /*label*/) { return false; }
+ bool LabelIndexExists(storage::v3::LabelId label) { return PrimaryLabelExists(label); }
bool PrimaryLabelExists(storage::v3::LabelId label) { return request_router_->IsPrimaryLabel(label); }
diff --git a/src/utils/event_counter.cpp b/src/utils/event_counter.cpp
index 8f7475ffb..2928027ee 100644
--- a/src/utils/event_counter.cpp
+++ b/src/utils/event_counter.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -25,7 +25,7 @@
M(ScanAllByLabelPropertyValueOperator, "Number of times ScanAllByLabelPropertyValue operator was used.") \
M(ScanAllByLabelPropertyOperator, "Number of times ScanAllByLabelProperty operator was used.") \
M(ScanAllByIdOperator, "Number of times ScanAllById operator was used.") \
- M(ScanAllByPrimaryKeyOperator, "Number of times ScanAllByPrimaryKey operator was used.") \
+ M(ScanByPrimaryKeyOperator, "Number of times ScanByPrimaryKey operator was used.") \
M(ExpandOperator, "Number of times Expand operator was used.") \
M(ExpandVariableOperator, "Number of times ExpandVariable operator was used.") \
M(ConstructNamedPathOperator, "Number of times ConstructNamedPath operator was used.") \
diff --git a/tests/unit/query_plan_checker.hpp b/tests/unit/query_plan_checker.hpp
index da910ff99..7907f167f 100644
--- a/tests/unit/query_plan_checker.hpp
+++ b/tests/unit/query_plan_checker.hpp
@@ -337,12 +337,12 @@ class ExpectScanAllByLabelProperty : public OpChecker<ScanAllByLabelProperty> {
memgraph::storage::PropertyId property_;
};
-class ExpectScanAllByPrimaryKey : public OpChecker<v2::plan::ScanAllByPrimaryKey> {
+class ExpectScanByPrimaryKey : public OpChecker<v2::plan::ScanByPrimaryKey> {
public:
- ExpectScanAllByPrimaryKey(memgraph::storage::v3::LabelId label, const std::vector<Expression *> &properties)
+ ExpectScanByPrimaryKey(memgraph::storage::v3::LabelId label, const std::vector<Expression *> &properties)
: label_(label), properties_(properties) {}
- void ExpectOp(v2::plan::ScanAllByPrimaryKey &scan_all, const SymbolTable &) override {
+ void ExpectOp(v2::plan::ScanByPrimaryKey &scan_all, const SymbolTable &) override {
EXPECT_EQ(scan_all.label_, label_);
bool primary_property_match = true;
diff --git a/tests/unit/query_plan_checker_v2.hpp b/tests/unit/query_plan_checker_v2.hpp
index 0d7a6bc5f..3604dcb25 100644
--- a/tests/unit/query_plan_checker_v2.hpp
+++ b/tests/unit/query_plan_checker_v2.hpp
@@ -62,7 +62,7 @@ class PlanChecker : public virtual HierarchicalLogicalOperatorVisitor {
PRE_VISIT(ScanAllByLabelPropertyValue);
PRE_VISIT(ScanAllByLabelPropertyRange);
PRE_VISIT(ScanAllByLabelProperty);
- PRE_VISIT(ScanAllByPrimaryKey);
+ PRE_VISIT(ScanByPrimaryKey);
PRE_VISIT(Expand);
PRE_VISIT(ExpandVariable);
PRE_VISIT(Filter);
@@ -175,12 +175,12 @@ class ExpectScanAllByLabelPropertyValue : public OpChecker<ScanAllByLabelPropert
memgraph::query::v2::Expression *expression_;
};
-class ExpectScanAllByPrimaryKey : public OpChecker<v2::plan::ScanAllByPrimaryKey> {
+class ExpectScanByPrimaryKey : public OpChecker<v2::plan::ScanByPrimaryKey> {
public:
- ExpectScanAllByPrimaryKey(memgraph::storage::v3::LabelId label, const std::vector<Expression *> &properties)
+ ExpectScanByPrimaryKey(memgraph::storage::v3::LabelId label, const std::vector<Expression *> &properties)
: label_(label), properties_(properties) {}
- void ExpectOp(v2::plan::ScanAllByPrimaryKey &scan_all, const SymbolTable &) override {
+ void ExpectOp(v2::plan::ScanByPrimaryKey &scan_all, const SymbolTable &) override {
EXPECT_EQ(scan_all.label_, label_);
bool primary_property_match = true;
diff --git a/tests/unit/query_v2_plan.cpp b/tests/unit/query_v2_plan.cpp
index 215b6b8a3..8fc5e11c7 100644
--- a/tests/unit/query_v2_plan.cpp
+++ b/tests/unit/query_v2_plan.cpp
@@ -109,7 +109,7 @@ TYPED_TEST(TestPlanner, MatchFilterPropIsNotNull) {
WHERE(EQ(PROPERTY_LOOKUP("n", prim_prop_one), LITERAL(1))), RETURN("n")));
auto symbol_table = (memgraph::expr::MakeSymbolTable(query));
auto planner = MakePlanner<TypeParam>(&dba, storage, symbol_table, query);
- CheckPlan(planner.plan(), symbol_table, ExpectScanAllByPrimaryKey(label, {expected_primary_key}), ExpectProduce());
+ CheckPlan(planner.plan(), symbol_table, ExpectScanByPrimaryKey(label, {expected_primary_key}), ExpectProduce());
}
// Exact primary key match, two elem as PK.
{
@@ -141,7 +141,7 @@ TYPED_TEST(TestPlanner, MatchFilterPropIsNotNull) {
RETURN("n")));
auto symbol_table = (memgraph::expr::MakeSymbolTable(query));
auto planner = MakePlanner<TypeParam>(&dba, storage, symbol_table, query);
- CheckPlan(planner.plan(), symbol_table, ExpectScanAllByPrimaryKey(label, {expected_primary_key}), ExpectProduce());
+ CheckPlan(planner.plan(), symbol_table, ExpectScanByPrimaryKey(label, {expected_primary_key}), ExpectProduce());
}
// One elem is missing from PK, default to ScanAllByLabelPropertyValue.
{