tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove template from ExpandUniquenessFilter

Browse Source 
Summary:
Instantiation with VertexAccessor was never used, so the template
needlessly complicated the rest of the codebase.

Reviewers: mtomic, llugovic

Reviewed By: mtomic

Subscribers: mferencevic, pullbot

Differential Revision: https://phabricator.memgraph.io/D1736
This commit is contained in:
Teon Banek 2018-11-19 10:58:49 +01:00
parent 8ed1fbbbe1
commit 58b450a2ea
12 changed files with 66 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/query/plan/cost_estimator.hpp
View File

@ -47,7 +47,7 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor {
static constexpr double kExpand{2.0};
static constexpr double kExpandVariable{3.0};
static constexpr double kFilter{1.5};
static constexpr double kExpandUniquenessFilter{1.5};
static constexpr double kEdgeUniquenessFilter{1.5};
static constexpr double kUnwind{1.3};
};
@ -55,7 +55,7 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor {
static constexpr double kExpand{3.0};
static constexpr double kExpandVariable{9.0};
static constexpr double kFilter{0.25};
static constexpr double kExpandUniquenessFilter{0.95};
static constexpr double kEdgeUniquenessFilter{0.95};
};
struct MiscParam {
@ -156,10 +156,7 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor {
}
POST_VISIT_COST_FIRST(Filter, kFilter)
POST_VISIT_COST_FIRST(ExpandUniquenessFilter<VertexAccessor>,
kExpandUniquenessFilter);
POST_VISIT_COST_FIRST(ExpandUniquenessFilter<EdgeAccessor>,
kExpandUniquenessFilter);
POST_VISIT_COST_FIRST(EdgeUniquenessFilter, kEdgeUniquenessFilter);
#undef POST_VISIT_COST_FIRST

11
src/query/plan/distributed.cpp
View File

@ -361,11 +361,11 @@ class IndependentSubtreeFinder : public DistributedOperatorVisitor {
return true;
}
bool PreVisit(ExpandUniquenessFilter<EdgeAccessor> &op) override {
bool PreVisit(EdgeUniquenessFilter &op) override {
prev_ops_.push_back(&op);
return true;
}
bool PostVisit(ExpandUniquenessFilter<EdgeAccessor> &op) override {
bool PostVisit(EdgeUniquenessFilter &op) override {
prev_ops_.pop_back();
if (branch_.subtree) return true;
if (auto found = FindForbidden(op.expand_symbol_)) {
@ -986,11 +986,8 @@ class DistributedPlanner : public HierarchicalLogicalOperatorVisitor {
prev_ops_.push_back(&op);
return true;
}
bool PreVisit(ExpandUniquenessFilter<VertexAccessor> &op) override {
prev_ops_.push_back(&op);
return true;
}
bool PreVisit(ExpandUniquenessFilter<EdgeAccessor> &op) override {
bool PreVisit(EdgeUniquenessFilter &op) override {
prev_ops_.push_back(&op);
return true;
}

50
src/query/plan/operator.cpp
View File

@ -2105,55 +2105,40 @@ void RemoveLabels::RemoveLabelsCursor::Shutdown() { input_cursor_->Shutdown(); }
void RemoveLabels::RemoveLabelsCursor::Reset() { input_cursor_->Reset(); }
template <typename TAccessor>
ExpandUniquenessFilter<TAccessor>::ExpandUniquenessFilter(
EdgeUniquenessFilter::EdgeUniquenessFilter(
const std::shared_ptr<LogicalOperator> &input, Symbol expand_symbol,
const std::vector<Symbol> &previous_symbols)
: input_(input),
expand_symbol_(expand_symbol),
previous_symbols_(previous_symbols) {}
template <typename TAccessor>
ACCEPT_WITH_INPUT(ExpandUniquenessFilter<TAccessor>)
ACCEPT_WITH_INPUT(EdgeUniquenessFilter)
template <typename TAccessor>
std::unique_ptr<Cursor> ExpandUniquenessFilter<TAccessor>::MakeCursor(
std::unique_ptr<Cursor> EdgeUniquenessFilter::MakeCursor(
database::GraphDbAccessor &db) const {
return std::make_unique<ExpandUniquenessFilterCursor>(*this, db);
return std::make_unique<EdgeUniquenessFilterCursor>(*this, db);
}
template <typename TAccessor>
std::vector<Symbol> ExpandUniquenessFilter<TAccessor>::ModifiedSymbols(
std::vector<Symbol> EdgeUniquenessFilter::ModifiedSymbols(
const SymbolTable &table) const {
return input_->ModifiedSymbols(table);
}
template <typename TAccessor>
ExpandUniquenessFilter<TAccessor>::ExpandUniquenessFilterCursor::
ExpandUniquenessFilterCursor(const ExpandUniquenessFilter &self,
EdgeUniquenessFilter::EdgeUniquenessFilterCursor::
EdgeUniquenessFilterCursor(const EdgeUniquenessFilter &self,
database::GraphDbAccessor &db)
: self_(self), input_cursor_(self.input_->MakeCursor(db)) {}
namespace {
/**
* Returns true if:
* - a and b are vertex values and are the same
* - a and b are either edge or edge-list values, and there
* is at least one matching edge in the two values
*/
template <typename TAccessor>
bool ContainsSame(const TypedValue &a, const TypedValue &b);
template <>
bool ContainsSame<VertexAccessor>(const TypedValue &a, const TypedValue &b) {
return a.Value<VertexAccessor>() == b.Value<VertexAccessor>();
}
template <>
bool ContainsSame<EdgeAccessor>(const TypedValue &a, const TypedValue &b) {
bool ContainsSameEdge(const TypedValue &a, const TypedValue &b) {
auto compare_to_list = [](const TypedValue &list, const TypedValue &other) {
for (const TypedValue &list_elem : list.Value<std::vector<TypedValue>>())
if (ContainsSame<EdgeAccessor>(list_elem, other)) return true;
if (ContainsSameEdge(list_elem, other)) return true;
return false;
};
@ -2164,8 +2149,7 @@ bool ContainsSame<EdgeAccessor>(const TypedValue &a, const TypedValue &b) {
}
} // namespace
template <typename TAccessor>
bool ExpandUniquenessFilter<TAccessor>::ExpandUniquenessFilterCursor::Pull(
bool EdgeUniquenessFilter::EdgeUniquenessFilterCursor::Pull(
Frame &frame, Context &context) {
auto expansion_ok = [&]() {
TypedValue &expand_value = frame[self_.expand_symbol_];
@ -2174,7 +2158,7 @@ bool ExpandUniquenessFilter<TAccessor>::ExpandUniquenessFilterCursor::Pull(
// This shouldn't raise a TypedValueException, because the planner
// makes sure these are all of the expected type. In case they are not
// an error should be raised long before this code is executed.
if (ContainsSame<TAccessor>(previous_value, expand_value)) return false;
if (ContainsSameEdge(previous_value, expand_value)) return false;
}
return true;
};
@ -2184,22 +2168,14 @@ bool ExpandUniquenessFilter<TAccessor>::ExpandUniquenessFilterCursor::Pull(
return false;
}
template <typename TAccessor>
void ExpandUniquenessFilter<
TAccessor>::ExpandUniquenessFilterCursor::Shutdown() {
void EdgeUniquenessFilter::EdgeUniquenessFilterCursor::Shutdown() {
input_cursor_->Shutdown();
}
template <typename TAccessor>
void ExpandUniquenessFilter<TAccessor>::ExpandUniquenessFilterCursor::Reset() {
void EdgeUniquenessFilter::EdgeUniquenessFilterCursor::Reset() {
input_cursor_->Reset();
}
// instantiations of the ExpandUniquenessFilter template class
// we only ever need these two
template class ExpandUniquenessFilter<VertexAccessor>;
template class ExpandUniquenessFilter<EdgeAccessor>;
Accumulate::Accumulate(const std::shared_ptr<LogicalOperator> &input,
const std::vector<Symbol> &symbols, bool advance_command)
: input_(input), symbols_(symbols), advance_command_(advance_command) {}

56
src/query/plan/operator.lcp
View File

@ -83,8 +83,7 @@ class SetProperties;
class SetLabels;
class RemoveProperty;
class RemoveLabels;
template <typename TAccessor>
class ExpandUniquenessFilter;
class EdgeUniquenessFilter;
class Accumulate;
class Aggregate;
class Skip;
@ -102,8 +101,7 @@ using LogicalOperatorCompositeVisitor = ::utils::CompositeVisitor<
ScanAllByLabelPropertyRange, ScanAllByLabelPropertyValue, Expand,
ExpandVariable, ConstructNamedPath, Filter, Produce, Delete, SetProperty,
SetProperties, SetLabels, RemoveProperty, RemoveLabels,
ExpandUniquenessFilter<VertexAccessor>,
ExpandUniquenessFilter<EdgeAccessor>, Accumulate, Aggregate, Skip, Limit,
EdgeUniquenessFilter, Accumulate, Aggregate, Skip, Limit,
OrderBy, Merge, Optional, Unwind, Distinct, Union, Cartesian>;
using LogicalOperatorLeafVisitor = ::utils::LeafVisitor<Once>;
@ -1352,7 +1350,7 @@ If a label does not exist on a Vertex, nothing happens.")
cpp<#)
(:serialize :capnp))
(lcp:define-class (expand-uniqueness-filter t-accessor) (logical-operator)
(lcp:define-class edge-uniqueness-filter (logical-operator)
((input "std::shared_ptr<LogicalOperator>" :scope :public
:capnp-save #'save-operator-pointer
:capnp-load #'load-operator-pointer)
@ -1361,34 +1359,26 @@ If a label does not exist on a Vertex, nothing happens.")
:capnp-save (lcp:capnp-save-vector "::query::capnp::Symbol" "Symbol")
:capnp-load (lcp:capnp-load-vector "::query::capnp::Symbol" "Symbol")))
(:documentation
"Filter whose Pull returns true only when the given
expand_symbol frame value (the latest expansion) is not
equal to any of the previous_symbols frame values.
"Filter whose Pull returns true only when the given expand_symbol frame
value (the latest expansion) is not equal to any of the previous_symbols frame
values.
Used for implementing [iso/cypher]morphism.
Isomorphism is vertex-uniqueness. It means that
two different vertices in a pattern can not map to the
same data vertex. For example, if the database
contains one vertex with a recursive relationship,
then the query
MATCH ()-[]->() combined with vertex uniqueness
yields no results (no uniqueness yields one).
Cyphermorphism is edge-uniqueness (the above
explanation applies). By default Neo4j uses
Cyphermorphism (that's where the name stems from,
it is not a valid graph-theory term).
Used for implementing Cyphermorphism.
Isomorphism is vertex-uniqueness. It means that two different vertices in a
pattern can not map to the same data vertex.
Cyphermorphism is edge-uniqueness (the above explanation applies). By default
Neo4j uses Cyphermorphism (that's where the name stems from, it is not a valid
graph-theory term).
Works for both Edge and Vertex uniqueness checks
(provide the accessor type as a template argument).
Supports variable-length-edges (uniqueness comparisons
between edges and an edge lists).")
Supports variable-length-edges (uniqueness comparisons between edges and an
edge lists).")
(:public
#>cpp
ExpandUniquenessFilter() {}
EdgeUniquenessFilter() {}
ExpandUniquenessFilter(const std::shared_ptr<LogicalOperator> &input,
Symbol expand_symbol,
const std::vector<Symbol> &previous_symbols);
EdgeUniquenessFilter(const std::shared_ptr<LogicalOperator> &input,
Symbol expand_symbol,
const std::vector<Symbol> &previous_symbols);
bool Accept(HierarchicalLogicalOperatorVisitor &visitor) override;
std::unique_ptr<Cursor> MakeCursor(
database::GraphDbAccessor &db) const override;
@ -1402,20 +1392,20 @@ between edges and an edge lists).")
cpp<#)
(:private
#>cpp
class ExpandUniquenessFilterCursor : public Cursor {
class EdgeUniquenessFilterCursor : public Cursor {
public:
ExpandUniquenessFilterCursor(const ExpandUniquenessFilter &self,
database::GraphDbAccessor &db);
EdgeUniquenessFilterCursor(const EdgeUniquenessFilter &self,
database::GraphDbAccessor &db);
bool Pull(Frame &, Context &) override;
void Shutdown() override;
void Reset() override;
private:
const ExpandUniquenessFilter &self_;
const EdgeUniquenessFilter &self_;
const std::unique_ptr<Cursor> input_cursor_;
};
cpp<#)
(:serialize :capnp :type-args '(vertex-accessor edge-accessor)))
(:serialize :capnp))
(lcp:define-class accumulate (logical-operator)
((input "std::shared_ptr<LogicalOperator>" :scope :public

3
src/query/plan/pretty_print.cpp
View File

@ -99,8 +99,7 @@ PRE_VISIT(SetProperties);
PRE_VISIT(SetLabels);
PRE_VISIT(RemoveProperty);
PRE_VISIT(RemoveLabels);
PRE_VISIT(ExpandUniquenessFilter<VertexAccessor>);
PRE_VISIT(ExpandUniquenessFilter<EdgeAccessor>);
PRE_VISIT(EdgeUniquenessFilter);
PRE_VISIT(Accumulate);
bool PlanPrinter::PreVisit(query::plan::Aggregate &op) {

3
src/query/plan/pretty_print.hpp
View File

@ -58,8 +58,7 @@ class PlanPrinter : public virtual HierarchicalLogicalOperatorVisitor {
bool PreVisit(ConstructNamedPath &) override;
bool PreVisit(Filter &) override;
bool PreVisit(ExpandUniquenessFilter<VertexAccessor> &) override;
bool PreVisit(ExpandUniquenessFilter<EdgeAccessor> &) override;
bool PreVisit(EdgeUniquenessFilter &) override;
bool PreVisit(Merge &) override;
bool PreVisit(Optional &) override;

2
src/query/plan/rule_based_planner.hpp
View File

@ -454,7 +454,7 @@ class RuleBasedPlanner {
other_symbols.push_back(symbol);
}
if (!other_symbols.empty()) {
last_op = std::make_unique<ExpandUniquenessFilter<EdgeAccessor>>(
last_op = std::make_unique<EdgeUniquenessFilter>(
std::move(last_op), edge_symbol, other_symbols);
}
}

6
tests/unit/distributed_query_plan.cpp
View File

@ -1038,14 +1038,14 @@ TYPED_TEST(TestPlanner, MultiMatch) {
get_symbol(node_h)});
auto right_cart = MakeCheckers(
ExpectScanAll(), ExpectDistributedExpand(), ExpectDistributedExpand(),
ExpectExpandUniquenessFilter<EdgeAccessor>(), right_pull);
ExpectEdgeUniquenessFilter(), right_pull);
auto expected = ExpectDistributed(
MakeCheckers(ExpectDistributedCartesian(left_cart, right_cart),
ExpectProduce()),
MakeCheckers(ExpectScanAll(), ExpectDistributedExpand()),
MakeCheckers(ExpectScanAll(), ExpectDistributedExpand(),
ExpectDistributedExpand(),
ExpectExpandUniquenessFilter<EdgeAccessor>()));
ExpectEdgeUniquenessFilter()));
CheckDistributedPlan(planner.plan(), symbol_table, expected);
}
@ -1962,7 +1962,7 @@ TYPED_TEST(TestPlanner, DistributedCartesianTransitiveDependency) {
// This expand depends on the previous one.
ExpectDistributedExpand(),
// UniquenessFilter depends on both expands.
ExpectExpandUniquenessFilter<EdgeAccessor>(),
ExpectEdgeUniquenessFilter(),
ExpectProduce()),
MakeCheckers(ExpectScanAllByLabel()),
MakeCheckers(ExpectScanAllByLabel()),

10
tests/unit/query_cost_estimator.cpp
View File

@ -193,11 +193,11 @@ TEST_F(QueryCostEstimator, Filter) {
CardParam::kFilter);
}
TEST_F(QueryCostEstimator, ExpandUniquenessFilter) {
TEST_OP(MakeOp<ExpandUniquenessFilter<VertexAccessor>>(last_op_, NextSymbol(),
std::vector<Symbol>()),
CostParam::kExpandUniquenessFilter,
CardParam::kExpandUniquenessFilter);
TEST_F(QueryCostEstimator, EdgeUniquenessFilter) {
TEST_OP(MakeOp<EdgeUniquenessFilter>(last_op_, NextSymbol(),
std::vector<Symbol>()),
CostParam::kEdgeUniquenessFilter,
CardParam::kEdgeUniquenessFilter);
}
TEST_F(QueryCostEstimator, UnwindLiteral) {

7
tests/unit/query_plan.cpp
View File

@ -305,8 +305,7 @@ TYPED_TEST(TestPlanner, MatchMultiPattern) {
// single MATCH clause.
CheckPlan<TypeParam>(query, storage, ExpectScanAll(), ExpectExpand(),
ExpectScanAll(), ExpectExpand(),
ExpectExpandUniquenessFilter<EdgeAccessor>(),
ExpectProduce());
ExpectEdgeUniquenessFilter(), ExpectProduce());
}
TYPED_TEST(TestPlanner, MatchMultiPatternSameStart) {
@ -333,7 +332,7 @@ TYPED_TEST(TestPlanner, MatchMultiPatternSameExpandStart) {
// expansion. Additionally, a uniqueness filter is expected.
CheckPlan<TypeParam>(
query, storage, ExpectScanAll(), ExpectExpand(), ExpectExpand(),
ExpectExpandUniquenessFilter<EdgeAccessor>(), ExpectProduce());
ExpectEdgeUniquenessFilter(), ExpectProduce());
}
TYPED_TEST(TestPlanner, MultiMatch) {
@ -357,7 +356,7 @@ TYPED_TEST(TestPlanner, MultiMatch) {
// not cross MATCH boundaries.
CheckPlan(planner.plan(), symbol_table, ExpectScanAll(), ExpectExpand(),
ExpectScanAll(), ExpectExpand(), ExpectExpand(),
ExpectExpandUniquenessFilter<EdgeAccessor>(), ExpectProduce());
ExpectEdgeUniquenessFilter(), ExpectProduce());
}
TYPED_TEST(TestPlanner, MultiMatchSameStart) {

7
tests/unit/query_plan_checker.hpp
View File

@ -61,8 +61,7 @@ class PlanChecker : public virtual HierarchicalLogicalOperatorVisitor {
PRE_VISIT(SetLabels);
PRE_VISIT(RemoveProperty);
PRE_VISIT(RemoveLabels);
PRE_VISIT(ExpandUniquenessFilter<VertexAccessor>);
PRE_VISIT(ExpandUniquenessFilter<EdgeAccessor>);
PRE_VISIT(EdgeUniquenessFilter);
PRE_VISIT(Accumulate);
PRE_VISIT(Aggregate);
PRE_VISIT(Skip);
@ -130,9 +129,7 @@ using ExpectSetProperties = OpChecker<SetProperties>;
using ExpectSetLabels = OpChecker<SetLabels>;
using ExpectRemoveProperty = OpChecker<RemoveProperty>;
using ExpectRemoveLabels = OpChecker<RemoveLabels>;
template <class TAccessor>
using ExpectExpandUniquenessFilter =
OpChecker<ExpandUniquenessFilter<TAccessor>>;
using ExpectEdgeUniquenessFilter = OpChecker<EdgeUniquenessFilter>;
using ExpectSkip = OpChecker<Skip>;
using ExpectLimit = OpChecker<Limit>;
using ExpectOrderBy = OpChecker<OrderBy>;

24
tests/unit/query_plan_match_filter_return.cpp
View File

@ -710,7 +710,7 @@ TEST_F(QueryPlanExpandVariable, EdgeUniquenessSingleAndVariableExpansion) {
if (add_uniqueness_check) {
auto last_symbol = symbols.back();
symbols.pop_back();
last_op = std::make_shared<ExpandUniquenessFilter<EdgeAccessor>>(
last_op = std::make_shared<EdgeUniquenessFilter>(
last_op, last_symbol, symbols);
}
@ -741,7 +741,7 @@ TEST_F(QueryPlanExpandVariable, EdgeUniquenessTwoVariableExpansions) {
AddMatch<ExpandVariable>(first, "n2", layer, direction, {}, lower,
upper, e2, "m2", GraphView::OLD);
if (add_uniqueness_check) {
last_op = std::make_shared<ExpandUniquenessFilter<EdgeAccessor>>(
last_op = std::make_shared<EdgeUniquenessFilter>(
last_op, e2, std::vector<Symbol>{e1});
}
@ -1547,7 +1547,7 @@ TEST(QueryPlan, Filter) {
EXPECT_EQ(CollectProduce(produce.get(), symbol_table, dba).size(), 2);
}
TEST(QueryPlan, ExpandUniquenessFilter) {
TEST(QueryPlan, EdgeUniquenessFilter) {
database::GraphDb db;
auto dba = db.Access();
@ -1559,8 +1559,7 @@ TEST(QueryPlan, ExpandUniquenessFilter) {
dba->InsertEdge(v1, v1, edge_type);
dba->AdvanceCommand();
auto check_expand_results = [&](bool vertex_uniqueness,
bool edge_uniqueness) {
auto check_expand_results = [&](bool edge_uniqueness) {
AstStorage storage;
SymbolTable symbol_table;
@ -1569,27 +1568,18 @@ TEST(QueryPlan, ExpandUniquenessFilter) {
MakeExpand(storage, symbol_table, n1.op_, n1.sym_, "r1",
EdgeAtom::Direction::OUT, {}, "n2", false, GraphView::OLD);
std::shared_ptr<LogicalOperator> last_op = r1_n2.op_;
if (vertex_uniqueness)
last_op = std::make_shared<ExpandUniquenessFilter<VertexAccessor>>(
last_op, r1_n2.node_sym_, std::vector<Symbol>{n1.sym_});
auto r2_n3 =
MakeExpand(storage, symbol_table, last_op, r1_n2.node_sym_, "r2",
EdgeAtom::Direction::OUT, {}, "n3", false, GraphView::OLD);
last_op = r2_n3.op_;
if (edge_uniqueness)
last_op = std::make_shared<ExpandUniquenessFilter<EdgeAccessor>>(
last_op = std::make_shared<EdgeUniquenessFilter>(
last_op, r2_n3.edge_sym_, std::vector<Symbol>{r1_n2.edge_sym_});
if (vertex_uniqueness)
last_op = std::make_shared<ExpandUniquenessFilter<VertexAccessor>>(
last_op, r2_n3.node_sym_,
std::vector<Symbol>{n1.sym_, r1_n2.node_sym_});
return PullAll(last_op, *dba, symbol_table);
};
EXPECT_EQ(2, check_expand_results(false, false));
EXPECT_EQ(0, check_expand_results(true, false));
EXPECT_EQ(1, check_expand_results(false, true));
EXPECT_EQ(2, check_expand_results(false));
EXPECT_EQ(1, check_expand_results(true));
}
TEST(QueryPlan, Distinct) {