From 0fb3ae2d5671f366f60dd1b6973b8f3143ae3107 Mon Sep 17 00:00:00 2001 From: Josipmrden Date: Mon, 4 Dec 2023 00:01:29 +0100 Subject: [PATCH] Fix three match cartesian sequential scanning (#1555) --- src/query/plan/rule_based_planner.hpp | 12 ++++-- tests/e2e/CMakeLists.txt | 1 + tests/e2e/query_planning/CMakeLists.txt | 6 +++ tests/e2e/query_planning/common.py | 24 +++++++++++ .../query_planning_cartesian.py | 42 +++++++++++++++++++ tests/e2e/query_planning/workloads.yaml | 14 +++++++ 6 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 tests/e2e/query_planning/CMakeLists.txt create mode 100644 tests/e2e/query_planning/common.py create mode 100644 tests/e2e/query_planning/query_planning_cartesian.py create mode 100644 tests/e2e/query_planning/workloads.yaml diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp index a61658c90..074bd1c88 100644 --- a/src/query/plan/rule_based_planner.hpp +++ b/src/query/plan/rule_based_planner.hpp @@ -511,10 +511,6 @@ class RuleBasedPlanner { std::set visited_expansion_groups; - last_op = - GenerateExpansionOnAlreadySeenSymbols(std::move(last_op), matching, visited_expansion_groups, symbol_table, - storage, bound_symbols, new_symbols, named_paths, filters, view); - // We want to create separate branches of scan operators for each expansion group group of patterns // Whenever there are 2 scan branches, they will be joined with a Cartesian operator @@ -528,6 +524,14 @@ class RuleBasedPlanner { continue; } + last_op = + GenerateExpansionOnAlreadySeenSymbols(std::move(last_op), matching, visited_expansion_groups, symbol_table, + storage, bound_symbols, new_symbols, named_paths, filters, view); + + if (visited_expansion_groups.contains(expansion.expansion_group_id)) { + continue; + } + std::unique_ptr starting_expansion_operator = nullptr; if (!initial_expansion_done) { starting_expansion_operator = std::move(last_op); diff --git a/tests/e2e/CMakeLists.txt b/tests/e2e/CMakeLists.txt index da96f6d4a..594c9acb0 100644 --- a/tests/e2e/CMakeLists.txt +++ b/tests/e2e/CMakeLists.txt @@ -73,6 +73,7 @@ add_subdirectory(inspect_query) add_subdirectory(filter_info) add_subdirectory(queries) add_subdirectory(garbage_collection) +add_subdirectory(query_planning) copy_e2e_python_files(pytest_runner pytest_runner.sh "") copy_e2e_python_files(x x.sh "") diff --git a/tests/e2e/query_planning/CMakeLists.txt b/tests/e2e/query_planning/CMakeLists.txt new file mode 100644 index 000000000..9c6d39bf9 --- /dev/null +++ b/tests/e2e/query_planning/CMakeLists.txt @@ -0,0 +1,6 @@ +function(copy_query_planning_e2e_python_files FILE_NAME) + copy_e2e_python_files(query_planning ${FILE_NAME}) +endfunction() + +copy_query_planning_e2e_python_files(common.py) +copy_query_planning_e2e_python_files(query_planning_cartesian.py) diff --git a/tests/e2e/query_planning/common.py b/tests/e2e/query_planning/common.py new file mode 100644 index 000000000..6ad52539b --- /dev/null +++ b/tests/e2e/query_planning/common.py @@ -0,0 +1,24 @@ +# Copyright 2023 Memgraph Ltd. +# +# Use of this software is governed by the Business Source License +# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +# License, and you may not use this file except in compliance with the Business Source License. +# +# As of the Change Date specified in that file, in accordance with +# the Business Source License, use of this software will be governed +# by the Apache License, Version 2.0, included in the file +# licenses/APL.txt. + +import pytest +from gqlalchemy import Memgraph + + +@pytest.fixture +def memgraph(**kwargs) -> Memgraph: + memgraph = Memgraph() + + yield memgraph + + memgraph.drop_indexes() + memgraph.ensure_constraints([]) + memgraph.drop_database() diff --git a/tests/e2e/query_planning/query_planning_cartesian.py b/tests/e2e/query_planning/query_planning_cartesian.py new file mode 100644 index 000000000..11bc3f628 --- /dev/null +++ b/tests/e2e/query_planning/query_planning_cartesian.py @@ -0,0 +1,42 @@ +# Copyright 2023 Memgraph Ltd. +# +# Use of this software is governed by the Business Source License +# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +# License, and you may not use this file except in compliance with the Business Source License. +# +# As of the Change Date specified in that file, in accordance with +# the Business Source License, use of this software will be governed +# by the Apache License, Version 2.0, included in the file +# licenses/APL.txt. + +import sys + +import pytest +from common import memgraph + +QUERY_PLAN = "QUERY PLAN" + + +def test_indexed_join_with_indices(memgraph): + memgraph.execute("CREATE INDEX ON :Node(id);") + + expected_explain = [ + f" * Produce {{a, b, r}}", + f" * Filter (a :Node), {{a.id}}", + f" * Expand (b)-[r:EDGE]-(a)", + f" * ScanAllByLabelPropertyValue (b :Node {{id}})", + f" * Once", + ] + + results = list( + memgraph.execute_and_fetch( + "EXPLAIN MATCH (a:Node {id: 1}) MATCH (b:Node {id: 2}) MATCH (a)-[r:EDGE]-(b) return a,b,r;" + ) + ) + actual_explain = [x[QUERY_PLAN] for x in results] + + assert expected_explain == actual_explain + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-rA"])) diff --git a/tests/e2e/query_planning/workloads.yaml b/tests/e2e/query_planning/workloads.yaml new file mode 100644 index 000000000..3fd5fb478 --- /dev/null +++ b/tests/e2e/query_planning/workloads.yaml @@ -0,0 +1,14 @@ +queries_cluster: &queries_cluster + cluster: + main: + args: ["--bolt-port", "7687", "--log-level=TRACE"] + log_file: "query_planning.log" + setup_queries: [] + validation_queries: [] + + +workloads: + - name: "Query planning cartesian" + binary: "tests/e2e/pytest_runner.sh" + args: ["query_planning/query_planning_cartesian.py"] + <<: *queries_cluster