diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 41ca4779d..e6648be9e 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -887,7 +887,6 @@ class ExpandVariableCursor : public Cursor { if (edges_.empty()) return false; // we use this a lot - // TODO handle optional + existing edge std::vector &edges_on_frame = frame[self_.edge_symbol_].Value>(); @@ -1016,7 +1015,6 @@ bool ExpandBreadthFirst::Cursor::Pull(Frame &frame, Context &context) { TypedValue result = self_.where_->Accept(evaluator); switch (result.type()) { case TypedValue::Type::Null: - // TODO review: is treating Null as false desired? return; case TypedValue::Type::Bool: if (!result.Value()) return; @@ -1630,9 +1628,8 @@ bool ExpandUniquenessFilter::ExpandUniquenessFilterCursor::Pull( for (const auto &previous_symbol : self_.previous_symbols_) { TypedValue &previous_value = frame[previous_symbol]; // 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. - // TODO handle possible null due to optional match + // 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(previous_value, expand_value)) return false; } return true; diff --git a/tests/unit/query_plan_edge_cases.cpp b/tests/unit/query_plan_edge_cases.cpp index fe0dc4da6..97a202c65 100644 --- a/tests/unit/query_plan_edge_cases.cpp +++ b/tests/unit/query_plan_edge_cases.cpp @@ -41,23 +41,38 @@ TEST_F(QueryExecution, MissingOptionalIntoExpand) { Commit(); ASSERT_EQ(Execute("MATCH (n) RETURN n").size(), 4); - auto Exec = [this](bool desc, bool variable) { + auto Exec = [this](bool desc, const std::string &edge_pattern) { // this test depends on left-to-right query planning FLAGS_query_cost_planner = false; return Execute(std::string("MATCH (p:Person) WITH p ORDER BY p.id ") + (desc ? "DESC " : "") + "OPTIONAL MATCH (p)-->(d:Dog) WITH p, d " - "MATCH (d)-" + - (variable ? "[*1]" : "") + - "->(f:Food) " + "MATCH (d)" + edge_pattern + + "(f:Food) " "RETURN p, d, f") .size(); }; - EXPECT_EQ(Exec(false, false), 1); - EXPECT_EQ(Exec(true, false), 1); - EXPECT_EQ(Exec(false, true), 1); - EXPECT_EQ(Exec(true, true), 1); + std::string expand = "-->"; + std::string variable = "-[*1]->"; + std::string bfs = "-bfs[](n, e | true, 1)->"; - // TODO test/fix ExpandBreadthFirst once it's operator lands + EXPECT_EQ(Exec(false, expand), 1); + EXPECT_EQ(Exec(true, expand), 1); + EXPECT_EQ(Exec(false, variable), 1); + EXPECT_EQ(Exec(true, bfs), 1); + EXPECT_EQ(Exec(true, bfs), 1); +} + +TEST_F(QueryExecution, EdgeUniquenessInOptional) { + // Validating that an edge uniqueness check can't fail when the edge is Null + // due to optonal match. Since edge-uniqueness only happens in one OPTIONAL + // MATCH, we only need to check that scenario. + Execute("CREATE (), ()-[:Type]->()"); + Commit(); + ASSERT_EQ(Execute("MATCH (n) RETURN n").size(), 3); + EXPECT_EQ(Execute("MATCH (n) OPTIONAL MATCH (n)-[r1]->(), (n)-[r2]->() " + "RETURN n, r1, r2") + .size(), + 3); }