diff --git a/src/query/plan/cost_estimator.hpp b/src/query/plan/cost_estimator.hpp index 65210f4d5..7de0a43d3 100644 --- a/src/query/plan/cost_estimator.hpp +++ b/src/query/plan/cost_estimator.hpp @@ -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 diff --git a/src/query/plan/distributed.cpp b/src/query/plan/distributed.cpp index e62e8375f..cd5315b88 100644 --- a/src/query/plan/distributed.cpp +++ b/src/query/plan/distributed.cpp @@ -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; } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 1dcab81b6..d470c5557 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -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) {} diff --git a/src/query/plan/operator.lcp b/src/query/plan/operator.lcp index 92d72d07a..e7da102f5 100644 --- a/src/query/plan/operator.lcp +++ b/src/query/plan/operator.lcp @@ -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 diff --git a/src/query/plan/pretty_print.cpp b/src/query/plan/pretty_print.cpp index ee9e644c6..6a7be00db 100644 --- a/src/query/plan/pretty_print.cpp +++ b/src/query/plan/pretty_print.cpp @@ -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) { diff --git a/src/query/plan/pretty_print.hpp b/src/query/plan/pretty_print.hpp index b480db57d..a422c0bd7 100644 --- a/src/query/plan/pretty_print.hpp +++ b/src/query/plan/pretty_print.hpp @@ -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; diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp index c3c763a17..51e14a0c9 100644 --- a/src/query/plan/rule_based_planner.hpp +++ b/src/query/plan/rule_based_planner.hpp @@ -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); } } diff --git a/tests/unit/distributed_query_plan.cpp b/tests/unit/distributed_query_plan.cpp index 3089d5f5d..74ab8b7fe 100644 --- a/tests/unit/distributed_query_plan.cpp +++ b/tests/unit/distributed_query_plan.cpp @@ -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()), diff --git a/tests/unit/query_cost_estimator.cpp b/tests/unit/query_cost_estimator.cpp index 43c34db5d..62cd97acd 100644 --- a/tests/unit/query_cost_estimator.cpp +++ b/tests/unit/query_cost_estimator.cpp @@ -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) { diff --git a/tests/unit/query_plan.cpp b/tests/unit/query_plan.cpp index 9f3d508c3..a033c05c2 100644 --- a/tests/unit/query_plan.cpp +++ b/tests/unit/query_plan.cpp @@ -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) { diff --git a/tests/unit/query_plan_checker.hpp b/tests/unit/query_plan_checker.hpp index c828cbb3d..7c0ca8641 100644 --- a/tests/unit/query_plan_checker.hpp +++ b/tests/unit/query_plan_checker.hpp @@ -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>; diff --git a/tests/unit/query_plan_match_filter_return.cpp b/tests/unit/query_plan_match_filter_return.cpp index 3dec32810..a534c1c46 100644 --- a/tests/unit/query_plan_match_filter_return.cpp +++ b/tests/unit/query_plan_match_filter_return.cpp @@ -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) {