From 36fccc32c2655a72a6e740b990aecbe6f863fd9b Mon Sep 17 00:00:00 2001
From: gvolfing <gabor.volfinger@memgraph.io>
Date: Tue, 24 Jan 2023 16:59:38 +0100
Subject: [PATCH] Address PR comments
---
src/query/v2/plan/operator.cpp | 20 +++++++-------------
src/query/v2/plan/vertex_count_cache.hpp | 2 +-
src/query/v2/request_router.hpp | 4 ++--
tests/unit/mock_helpers.hpp | 2 +-
tests/unit/query_plan_checker_v2.hpp | 2 +-
tests/unit/query_v2_expression_evaluator.cpp | 5 +++--
6 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp
index fa20b4e48..c07d8d4aa 100644
--- a/src/query/v2/plan/operator.cpp
+++ b/src/query/v2/plan/operator.cpp
@@ -573,15 +573,13 @@ class DistributedScanByPrimaryKeyCursor : public Cursor {
throw HintedAbortError();
}
- if (!input_cursor_->Pull(frame, context)) {
- return false;
- }
-
- auto &request_router = *context.request_router;
- auto vertex = MakeRequestSingleFrame(frame, request_router, context);
- if (vertex) {
- frame[output_symbol_] = TypedValue(std::move(*vertex));
- return true;
+ while (input_cursor_->Pull(frame, context)) {
+ auto &request_router = *context.request_router;
+ auto vertex = MakeRequestSingleFrame(frame, request_router, context);
+ if (vertex) {
+ frame[output_symbol_] = TypedValue(std::move(*vertex));
+ return true;
+ }
}
return false;
}
@@ -601,10 +599,6 @@ class DistributedScanByPrimaryKeyCursor : public Cursor {
storage::v3::LabelId label_;
std::optional<std::vector<Expression *>> filter_expressions_;
std::vector<Expression *> primary_key_;
- std::optional<MultiFrame> own_multi_frames_;
- std::optional<ValidFramesConsumer> valid_frames_consumer_;
- ValidFramesConsumer::Iterator valid_frames_it_;
- std::queue<FrameWithValidity> frames_buffer_;
};
ScanAll::ScanAll(const std::shared_ptr<LogicalOperator> &input, Symbol output_symbol, storage::v3::View view)
diff --git a/src/query/v2/plan/vertex_count_cache.hpp b/src/query/v2/plan/vertex_count_cache.hpp
index bfe20ffe3..ae6cdad31 100644
--- a/src/query/v2/plan/vertex_count_cache.hpp
+++ b/src/query/v2/plan/vertex_count_cache.hpp
@@ -61,7 +61,7 @@ class VertexCountCache {
bool LabelPropertyIndexExists(storage::v3::LabelId /*label*/, storage::v3::PropertyId /*property*/) { return false; }
- std::vector<memgraph::storage::v3::SchemaProperty> GetSchemaForLabel(storage::v3::LabelId label) {
+ const std::vector<memgraph::storage::v3::SchemaProperty> &GetSchemaForLabel(storage::v3::LabelId label) {
return request_router_->GetSchemaForLabel(label);
}
diff --git a/src/query/v2/request_router.hpp b/src/query/v2/request_router.hpp
index 3b313e225..c175ba413 100644
--- a/src/query/v2/request_router.hpp
+++ b/src/query/v2/request_router.hpp
@@ -114,7 +114,7 @@ class RequestRouterInterface {
virtual std::optional<storage::v3::LabelId> MaybeNameToLabel(const std::string &name) const = 0;
virtual bool IsPrimaryLabel(storage::v3::LabelId label) const = 0;
virtual bool IsPrimaryKey(storage::v3::LabelId primary_label, storage::v3::PropertyId property) const = 0;
- virtual std::vector<coordinator::SchemaProperty> GetSchemaForLabel(storage::v3::LabelId label) const = 0;
+ virtual const std::vector<coordinator::SchemaProperty> &GetSchemaForLabel(storage::v3::LabelId label) const = 0;
};
// TODO(kostasrim)rename this class template
@@ -233,7 +233,7 @@ class RequestRouter : public RequestRouterInterface {
}) != schema_it->second.end();
}
- std::vector<coordinator::SchemaProperty> GetSchemaForLabel(storage::v3::LabelId label) const override {
+ const std::vector<coordinator::SchemaProperty> &GetSchemaForLabel(storage::v3::LabelId label) const override {
return shards_map_.schemas.at(label);
}
diff --git a/tests/unit/mock_helpers.hpp b/tests/unit/mock_helpers.hpp
index 0f4227a28..b7d764ac8 100644
--- a/tests/unit/mock_helpers.hpp
+++ b/tests/unit/mock_helpers.hpp
@@ -42,7 +42,7 @@ class MockedRequestRouter : public RequestRouterInterface {
MOCK_METHOD(std::optional<storage::v3::LabelId>, MaybeNameToLabel, (const std::string &), (const));
MOCK_METHOD(bool, IsPrimaryLabel, (storage::v3::LabelId), (const));
MOCK_METHOD(bool, IsPrimaryKey, (storage::v3::LabelId, storage::v3::PropertyId), (const));
- MOCK_METHOD(std::vector<coordinator::SchemaProperty>, GetSchemaForLabel, (storage::v3::LabelId), (const));
+ MOCK_METHOD(const std::vector<coordinator::SchemaProperty> &, GetSchemaForLabel, (storage::v3::LabelId), (const));
};
class MockedLogicalOperator : public plan::LogicalOperator {
diff --git a/tests/unit/query_plan_checker_v2.hpp b/tests/unit/query_plan_checker_v2.hpp
index 3604dcb25..014f1c2bb 100644
--- a/tests/unit/query_plan_checker_v2.hpp
+++ b/tests/unit/query_plan_checker_v2.hpp
@@ -381,7 +381,7 @@ class FakeDistributedDbAccessor {
return find_in_secondary_properties->second;
}
- MG_ASSERT(false, "The property does not exist as a primary or a secondary property.");
+ LOG_FATAL("The property does not exist as a primary or a secondary property.");
return memgraph::storage::v3::PropertyId::FromUint(0);
}
diff --git a/tests/unit/query_v2_expression_evaluator.cpp b/tests/unit/query_v2_expression_evaluator.cpp
index ffbebb4ba..b941d3463 100644
--- a/tests/unit/query_v2_expression_evaluator.cpp
+++ b/tests/unit/query_v2_expression_evaluator.cpp
@@ -125,8 +125,9 @@ class MockedRequestRouter : public RequestRouterInterface {
bool IsPrimaryKey(LabelId primary_label, PropertyId property) const override { return true; }
- std::vector<coordinator::SchemaProperty> GetSchemaForLabel(storage::v3::LabelId /*label*/) const override {
- return std::vector<coordinator::SchemaProperty>{};
+ const std::vector<coordinator::SchemaProperty> &GetSchemaForLabel(storage::v3::LabelId /*label*/) const override {
+ static std::vector<coordinator::SchemaProperty> schema;
+ return schema;
};
private: