diff --git a/src/query/plan/distributed.cpp b/src/query/plan/distributed.cpp index 3c4f38211..58c27848d 100644 --- a/src/query/plan/distributed.cpp +++ b/src/query/plan/distributed.cpp @@ -851,20 +851,17 @@ class DistributedPlanner : public HierarchicalLogicalOperatorVisitor { dependent_branches.push_back(right_branch); } auto append_dependants = [&dependent_branches](auto op, int64_t branch_ix) { - // Append dependent parents, iteration is in reverse because we want - // LIFO behaviour. - for (auto it = dependent_branches.rbegin(); - it != dependent_branches.rend(); ++it) { - if (it->depends_on.value() != branch_ix) continue; - it->parent_end->set_input(op); - op = it->parent_start; + // Append dependent parents, using LIFO behaviour, because + // dependent_branches is filled in reverse. + while (!dependent_branches.empty()) { + auto branch = dependent_branches.back(); + // Following branches may depend on this one, so we have to break as + // soon as the first one cannot be appended. + if (branch.depends_on.value() < branch_ix) break; + branch.parent_end->set_input(op); + op = branch.parent_start; + dependent_branches.pop_back(); } - dependent_branches.erase( - std::remove_if(dependent_branches.begin(), dependent_branches.end(), - [branch_ix](const auto &branch) { - return branch.depends_on.value() == branch_ix; - }), - dependent_branches.end()); return op; }; // We use this ordering of operators, so that left hand side can be diff --git a/tests/unit/distributed_query_plan.cpp b/tests/unit/distributed_query_plan.cpp index 2dfc9929b..116a68a2c 100644 --- a/tests/unit/distributed_query_plan.cpp +++ b/tests/unit/distributed_query_plan.cpp @@ -1743,11 +1743,11 @@ TYPED_TEST(TestPlanner, DistributedCartesianFilter) { MakeCheckers(ExpectScanAll(), ExpectFilter(), ExpectPullRemote({sym_a})); auto mid_cart = MakeCheckers(ExpectScanAll(), ExpectPullRemote({sym_b})); auto right_cart = MakeCheckers(ExpectScanAll(), ExpectPullRemote({sym_c})); - auto mid_right_cart = MakeCheckers( - ExpectDistributedCartesian(mid_cart, right_cart), ExpectFilter()); + auto mid_right_cart = + MakeCheckers(ExpectDistributedCartesian(mid_cart, right_cart)); auto expected = ExpectDistributed( MakeCheckers(ExpectDistributedCartesian(left_cart, mid_right_cart), - ExpectFilter(), ExpectProduce()), + ExpectFilter(), ExpectFilter(), ExpectProduce()), MakeCheckers(ExpectScanAll(), ExpectFilter()), MakeCheckers(ExpectScanAll()), MakeCheckers(ExpectScanAll())); FakeDbAccessor dba; @@ -2106,6 +2106,50 @@ TYPED_TEST(TestPlanner, DistributedOptionalCartesian) { CheckDistributedPlan(planner.plan(), symbol_table, expected); } +TYPED_TEST(TestPlanner, DistributedCartesianTransitiveDependency) { + // Test MATCH (n:L)-[a]-(m:L)-[b]-(l:L) RETURN l; + AstStorage storage; + FakeDbAccessor dba; + auto label = dba.Label("L"); + // Set indexes so that multiple scans and expanding to existing is preferred. + dba.SetIndexCount(label, 1); + auto *node_n = NODE("n", label); + auto *node_m = NODE("m", label); + auto *node_l = NODE("l", label); + auto *edge_a = EDGE("a"); + auto *edge_b = EDGE("b"); + QUERY(SINGLE_QUERY(MATCH(PATTERN(node_n, edge_a, node_m, edge_b, node_l)), + RETURN("l"))); + auto symbol_table = MakeSymbolTable(*storage.query()); + auto sym_a = symbol_table.at(*edge_a->identifier_); + auto sym_b = symbol_table.at(*edge_b->identifier_); + auto sym_n = symbol_table.at(*node_n->identifier_); + auto sym_m = symbol_table.at(*node_m->identifier_); + auto sym_l = symbol_table.at(*node_l->identifier_); + auto right_cart = + MakeCheckers(ExpectScanAllByLabel(), ExpectPullRemote({sym_l})); + auto mid_cart = + MakeCheckers(ExpectScanAllByLabel(), ExpectPullRemote({sym_m})); + auto left_cart = + MakeCheckers(ExpectScanAllByLabel(), ExpectPullRemote({sym_n})); + auto mid_right_cart = + MakeCheckers(ExpectDistributedCartesian(mid_cart, right_cart)); + auto expected = ExpectDistributed( + MakeCheckers(ExpectDistributedCartesian(left_cart, mid_right_cart), + // This expand depends on Cartesian branches. + ExpectDistributedExpand(), + // This expand depends on the previous one. + ExpectDistributedExpand(), + // UniquenessFilter depends on both expands. + ExpectExpandUniquenessFilter(), + ExpectProduce()), + MakeCheckers(ExpectScanAllByLabel()), + MakeCheckers(ExpectScanAllByLabel()), + MakeCheckers(ExpectScanAllByLabel())); + auto planner = MakePlanner(dba, storage, symbol_table); + CheckDistributedPlan(planner.plan(), symbol_table, expected); +} + TYPED_TEST(TestPlanner, DistributedOptionalScanExpandExisting) { // Test MATCH (a) OPTIONAL MATCH (b)-[e]-(a) RETURN e; AstStorage storage;