From 92b9bbd4bdabead879e6a7d874a6bad7d7bd702f Mon Sep 17 00:00:00 2001 From: Teon Banek Date: Sat, 9 Sep 2017 12:41:40 +0200 Subject: [PATCH] Remove const requirement from DbAccessor in planning Summary: The constness of the DbAccessor interferes with caching the results. Reviewers: florijan, mislav.bradac Reviewed By: mislav.bradac Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D771 --- src/query/plan/cost_estimator.hpp | 4 +-- src/query/plan/vertex_count_cache.hpp | 43 ++++++++-------------- tests/manual/query_planner.cpp | 52 +++++++++++---------------- 3 files changed, 36 insertions(+), 63 deletions(-) diff --git a/src/query/plan/cost_estimator.hpp b/src/query/plan/cost_estimator.hpp index 64b5b030c..a5fc48fca 100644 --- a/src/query/plan/cost_estimator.hpp +++ b/src/query/plan/cost_estimator.hpp @@ -203,7 +203,7 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor { // cardinality estimation (how many times an operator gets executed) // cardinality is a double to make it easier to work with double cardinality_{1}; - // + // accessor used for cardinality estimates in ScanAll and ScanAllByLabel const TDbAccessor &db_accessor_; @@ -225,7 +225,7 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor { /** Returns the estimated cost of the given plan. */ template -double EstimatePlanCost(TDbAccessor &db, LogicalOperator &plan) { +double EstimatePlanCost(const TDbAccessor &db, LogicalOperator &plan) { CostEstimator estimator(db); plan.Accept(estimator); return estimator.cost(); diff --git a/src/query/plan/vertex_count_cache.hpp b/src/query/plan/vertex_count_cache.hpp index 406e87601..0b78e573c 100644 --- a/src/query/plan/vertex_count_cache.hpp +++ b/src/query/plan/vertex_count_cache.hpp @@ -13,19 +13,13 @@ class VertexCountCache { VertexCountCache(const TDbAccessor &db) : db_(db) {} int64_t VerticesCount() const { - auto non_const_this = const_cast(this); - if (!vertices_count_) non_const_this->vertices_count_ = db_.VerticesCount(); + if (!vertices_count_) vertices_count_ = db_.VerticesCount(); return *vertices_count_; } int64_t VerticesCount(const GraphDbTypes::Label &label) const { - if (label_vertex_count_.find(label) == label_vertex_count_.end()) { - // DbAccessor API needs to be const. Since we know that - // InteractiveDbAccessor should never be const in this file, we use - // const_cast. - auto non_const_this = const_cast(this); - non_const_this->label_vertex_count_[label] = db_.VerticesCount(label); - } + if (label_vertex_count_.find(label) == label_vertex_count_.end()) + label_vertex_count_[label] = db_.VerticesCount(label); return label_vertex_count_.at(label); } @@ -33,11 +27,8 @@ class VertexCountCache { const GraphDbTypes::Property &property) const { auto key = std::make_pair(label, property); if (label_property_vertex_count_.find(key) == - label_property_vertex_count_.end()) { - auto non_const_this = const_cast(this); - non_const_this->label_property_vertex_count_[key] = - db_.VerticesCount(label, property); - } + label_property_vertex_count_.end()) + label_property_vertex_count_[key] = db_.VerticesCount(label, property); return label_property_vertex_count_.at(key); } @@ -45,12 +36,9 @@ class VertexCountCache { const GraphDbTypes::Property &property, const PropertyValue &value) const { auto label_prop = std::make_pair(label, property); - auto non_const_this = const_cast(this); - auto &value_vertex_count = - non_const_this->property_value_vertex_count_[label_prop]; - if (value_vertex_count.find(value) == value_vertex_count.end()) { + auto &value_vertex_count = property_value_vertex_count_[label_prop]; + if (value_vertex_count.find(value) == value_vertex_count.end()) value_vertex_count[value] = db_.VerticesCount(label, property, value); - } return value_vertex_count.at(value); } @@ -60,14 +48,11 @@ class VertexCountCache { const std::experimental::optional> &upper) const { auto label_prop = std::make_pair(label, property); - auto non_const_this = const_cast(this); - auto &bounds_vertex_count = - non_const_this->property_bounds_vertex_count_[label_prop]; + auto &bounds_vertex_count = property_bounds_vertex_count_[label_prop]; BoundsKey bounds = std::make_pair(lower, upper); - if (bounds_vertex_count.find(bounds) == bounds_vertex_count.end()) { + if (bounds_vertex_count.find(bounds) == bounds_vertex_count.end()) bounds_vertex_count[bounds] = db_.VerticesCount(label, property, lower, upper); - } return bounds_vertex_count.at(bounds); } @@ -122,17 +107,17 @@ class VertexCountCache { }; const TDbAccessor &db_; - std::experimental::optional vertices_count_; - std::unordered_map label_vertex_count_; - std::unordered_map + mutable std::experimental::optional vertices_count_; + mutable std::unordered_map label_vertex_count_; + mutable std::unordered_map label_property_vertex_count_; - std::unordered_map< + mutable std::unordered_map< LabelPropertyKey, std::unordered_map, LabelPropertyHash> property_value_vertex_count_; - std::unordered_map< + mutable std::unordered_map< LabelPropertyKey, std::unordered_map, LabelPropertyHash> diff --git a/tests/manual/query_planner.cpp b/tests/manual/query_planner.cpp index a0eed195f..ffac8023e 100644 --- a/tests/manual/query_planner.cpp +++ b/tests/manual/query_planner.cpp @@ -140,12 +140,7 @@ class InteractiveDbAccessor { int64_t VerticesCount(const GraphDbTypes::Label &label) const { if (label_vertex_count_.find(*label) == label_vertex_count_.end()) { - // DbAccessor API needs to be const. Since we know that - // InteractiveDbAccessor should never be const in this file, we use - // const_cast. - auto non_const_this = const_cast(this); - non_const_this->label_vertex_count_[*label] = - non_const_this->ReadVertexCount("label '" + *label + "'"); + label_vertex_count_[*label] = ReadVertexCount("label '" + *label + "'"); } return label_vertex_count_.at(*label); } @@ -155,10 +150,8 @@ class InteractiveDbAccessor { auto key = std::make_pair(*label, *property); if (label_property_vertex_count_.find(key) == label_property_vertex_count_.end()) { - auto non_const_this = const_cast(this); - non_const_this->label_property_vertex_count_[key] = - non_const_this->ReadVertexCount("label '" + *label + - "' and property '" + *property + "'"); + label_property_vertex_count_[key] = ReadVertexCount( + "label '" + *label + "' and property '" + *property + "'"); } return label_property_vertex_count_.at(key); } @@ -170,15 +163,12 @@ class InteractiveDbAccessor { if (label_property_index_.find(label_prop) == label_property_index_.end()) { return 0; } - auto non_const_this = const_cast(this); - auto &value_vertex_count = - non_const_this->property_value_vertex_count_[label_prop]; + auto &value_vertex_count = property_value_vertex_count_[label_prop]; if (value_vertex_count.find(value) == value_vertex_count.end()) { std::stringstream ss; ss << value; - int64_t count = non_const_this->ReadVertexCount( - "label '" + *label + "' and property '" + *property + "' value '" + - ss.str() + "'"); + int64_t count = ReadVertexCount("label '" + *label + "' and property '" + + *property + "' value '" + ss.str() + "'"); value_vertex_count[value] = count; } return value_vertex_count.at(value); @@ -199,22 +189,19 @@ class InteractiveDbAccessor { if (upper) { range_string << upper->value() << (upper->IsInclusive() ? "]" : ")"); } - return const_cast(this)->ReadVertexCount( - "label '" + *label + "' and property '" + *property + "' in range " + - range_string.str()); + return ReadVertexCount("label '" + *label + "' and property '" + *property + + "' in range " + range_string.str()); } bool LabelPropertyIndexExists(const GraphDbTypes::Label &label, const GraphDbTypes::Property &property) const { auto key = std::make_pair(*label, *property); if (label_property_index_.find(key) == label_property_index_.end()) { - bool resp = const_cast(this)->timer_.WithPause( - [&label, &property]() { - return AskYesNo("Index for ':" + *label + "(" + *property + - ")' exists:"); - }); - const_cast(this)->label_property_index_[key] = - resp; + bool resp = timer_.WithPause([&label, &property]() { + return AskYesNo("Index for ':" + *label + "(" + *property + + ")' exists:"); + }); + label_property_index_[key] = resp; } return label_property_index_.at(key); } @@ -323,18 +310,19 @@ class InteractiveDbAccessor { int64_t vertices_count_; Timer &timer_; - std::map label_vertex_count_; - std::map, int64_t> + mutable std::map label_vertex_count_; + mutable std::map, int64_t> label_property_vertex_count_; - std::map, bool> label_property_index_; - std::map< + mutable std::map, bool> + label_property_index_; + mutable std::map< std::pair, std::unordered_map> property_value_vertex_count_; // TODO: Cache faked index counts by range. - int64_t ReadVertexCount(const std::string &message) { + int64_t ReadVertexCount(const std::string &message) const { return timer_.WithPause( [&message]() { return ReadInt("Vertices with " + message + ": "); }); } @@ -621,7 +609,7 @@ query::SymbolTable MakeSymbolTable(const query::AstTreeStorage &ast) { // order by cost. auto MakeLogicalPlans(query::AstTreeStorage &ast, query::SymbolTable &symbol_table, - const InteractiveDbAccessor &dba) { + InteractiveDbAccessor &dba) { std::vector, double>> plans_with_cost; auto plans = query::plan::MakeLogicalPlan(