From 5d90df6100ba57c9826f354911ef41c28fc89516 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 3 Nov 2021 13:56:20 +0200 Subject: [PATCH 01/17] structure execution plan --- redisgraph/execution_plan.py | 35 +++++++++++++++++++++++++++++++++++ redisgraph/graph.py | 6 ++---- tests/functional/test_all.py | 2 +- 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 redisgraph/execution_plan.py diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py new file mode 100644 index 0000000..3ef23e6 --- /dev/null +++ b/redisgraph/execution_plan.py @@ -0,0 +1,35 @@ +class ExecutionPlan: + def __init__(self, plan): + self.plan = plan + + def __str__(self) -> str: + return "\n".join(self.plan) + + def operation_tree(self): + stack = [] + level = 0 + current = None + i = 0 + while i < len(self.plan): + op = self.plan[i] + op_level = op.count(" ") + if op_level == level: + current = { "op": op.replace(" ", "")} + i += 1 + elif op_level == level + 1: + child = { "op": op.replace(" ", "")} + if "children" not in current: + current["children"] = [] + current["children"].append(child) + stack.append(current) + current = child + level += 1 + i += 1 + elif op_level < level: + levels_back = level - op_level + 1 + for _ in range(levels_back): + current = stack.pop() + level -= levels_back + else: + raise Exception("corrupted plan") + return stack[0] \ No newline at end of file diff --git a/redisgraph/graph.py b/redisgraph/graph.py index 2322187..aecb97a 100644 --- a/redisgraph/graph.py +++ b/redisgraph/graph.py @@ -3,6 +3,7 @@ from redisgraph.util import random_string, quote_string, stringify_param_value from redisgraph.query_result import QueryResult from redisgraph.exceptions import VersionMismatchException +from redisgraph.execution_plan import ExecutionPlan class Graph: @@ -215,9 +216,6 @@ def query(self, q, params=None, timeout=None, read_only=False): # re-issue query return self.query(q, params, timeout, read_only) - def _execution_plan_to_string(self, plan): - return "\n".join(plan) - def execution_plan(self, query, params=None): """ Get the execution plan for given query, @@ -231,7 +229,7 @@ def execution_plan(self, query, params=None): query = self._build_params_header(params) + query plan = self.redis_con.execute_command("GRAPH.EXPLAIN", self.name, query) - return self._execution_plan_to_string(plan) + return ExecutionPlan(plan) def delete(self): """ diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 30e8040..df409c3 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -252,7 +252,7 @@ def test_execution_plan(self): result = redis_graph.execution_plan("MATCH (r:Rider)-[:rides]->(t:Team) WHERE t.name = $name RETURN r.name, t.name, $params", {'name': 'Yehuda'}) expected = "Results\n Project\n Conditional Traverse | (t:Team)->(r:Rider)\n Filter\n Node By Label Scan | (t:Team)" - self.assertEqual(result, expected) + self.assertEqual(str(result), expected) redis_graph.delete() From f621dce80d455945edcfb56d0ccab54ad83fb9f8 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 3 Nov 2021 14:00:19 +0200 Subject: [PATCH 02/17] lint --- redisgraph/execution_plan.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index 3ef23e6..117f983 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -14,10 +14,10 @@ def operation_tree(self): op = self.plan[i] op_level = op.count(" ") if op_level == level: - current = { "op": op.replace(" ", "")} + current = {"op": op.replace(" ", "")} i += 1 elif op_level == level + 1: - child = { "op": op.replace(" ", "")} + child = {"op": op.replace(" ", "")} if "children" not in current: current["children"] = [] current["children"].append(child) @@ -32,4 +32,4 @@ def operation_tree(self): level -= levels_back else: raise Exception("corrupted plan") - return stack[0] \ No newline at end of file + return stack[0] From 9b956cd53b8d2792a1455f521848a24a6cb9062d Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 3 Nov 2021 14:14:02 +0200 Subject: [PATCH 03/17] test operation tree --- tests/functional/test_all.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index df409c3..ac3fe9d 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -254,6 +254,16 @@ def test_execution_plan(self): expected = "Results\n Project\n Conditional Traverse | (t:Team)->(r:Rider)\n Filter\n Node By Label Scan | (t:Team)" self.assertEqual(str(result), expected) + expected = {'children': [{'children': [{'children': [{'children': [{'op': 'Node By Label ' + 'Scan | ' + '(t:Team)'}], + 'op': 'Filter'}], + 'op': 'Conditional Traverse | ' + '(t:Team)->(r:Rider)'}], + 'op': 'Project'}], + 'op': 'Results'} + self.assertEqual(result.operation_tree(), expected) + redis_graph.delete() def test_query_timeout(self): From 0e5b8c70188c9b083bdd6c8bccab443f90a8aa93 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 3 Nov 2021 14:19:25 +0200 Subject: [PATCH 04/17] fix format --- tests/functional/test_all.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index ac3fe9d..2344437 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -254,14 +254,11 @@ def test_execution_plan(self): expected = "Results\n Project\n Conditional Traverse | (t:Team)->(r:Rider)\n Filter\n Node By Label Scan | (t:Team)" self.assertEqual(str(result), expected) - expected = {'children': [{'children': [{'children': [{'children': [{'op': 'Node By Label ' - 'Scan | ' - '(t:Team)'}], - 'op': 'Filter'}], - 'op': 'Conditional Traverse | ' - '(t:Team)->(r:Rider)'}], - 'op': 'Project'}], - 'op': 'Results'} + expected = {'children': [{'children': [{'children': [{'children': [{'op': 'Node By Label Scan | (t:Team)'}], + 'op': 'Filter'}], + 'op': 'Conditional Traverse | (t:Team)->(r:Rider)'}], + 'op': 'Project'}], + 'op': 'Results'} self.assertEqual(result.operation_tree(), expected) redis_graph.delete() From b4d6d5dbf63a50507d20135689bc38d76a744426 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 3 Nov 2021 14:21:07 +0200 Subject: [PATCH 05/17] lint --- tests/functional/test_all.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 2344437..4078d08 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -254,7 +254,7 @@ def test_execution_plan(self): expected = "Results\n Project\n Conditional Traverse | (t:Team)->(r:Rider)\n Filter\n Node By Label Scan | (t:Team)" self.assertEqual(str(result), expected) - expected = {'children': [{'children': [{'children': [{'children': [{'op': 'Node By Label Scan | (t:Team)'}], + expected = {'children': [{'children': [{'children': [{'children': [{'op': 'Node By Label Scan | (t:Team)'}], 'op': 'Filter'}], 'op': 'Conditional Traverse | (t:Team)->(r:Rider)'}], 'op': 'Project'}], From 5044b11a422c6fadb01a3dccef7b4c46a9877144 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 17 Nov 2021 17:39:17 +0200 Subject: [PATCH 06/17] introduce operation class --- redisgraph/execution_plan.py | 44 ++++++++++++++++++++++++++++-------- tests/functional/test_all.py | 14 +++++++----- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index 117f983..6d97c47 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -1,26 +1,52 @@ +class Operation: + def __init__(self, name, args=None): + self.name = name + self.args = args + self.children = [] + + def append_child(self, child): + self.children.append(child) + return self + + def __eq__(self, o: object) -> bool: + if not isinstance(o, Operation): + return False + + if self.name != o.name or self.args != self.args or len(self.children) != len(o.children): + return False + + for i in range(len(self.children)): + if not self.children[i] == o.children[i]: + return False + + return True + + class ExecutionPlan: def __init__(self, plan): self.plan = plan + self.structured_plan = self._operation_tree() def __str__(self) -> str: return "\n".join(self.plan) - def operation_tree(self): - stack = [] - level = 0 + def _operation_tree(self): + i = 0 + level = 0 + stack = [] current = None - i = 0 + while i < len(self.plan): op = self.plan[i] op_level = op.count(" ") if op_level == level: - current = {"op": op.replace(" ", "")} + args = op.split("|") + current = Operation(args[0].strip(), None if len(args) == 1 else args[1].strip()) i += 1 elif op_level == level + 1: - child = {"op": op.replace(" ", "")} - if "children" not in current: - current["children"] = [] - current["children"].append(child) + args = op.split("|") + child = Operation(args[0].strip(), None if len(args) == 1 else args[1].strip()) + current.children.append(child) stack.append(current) current = child level += 1 diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 4078d08..89e9e29 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -1,4 +1,5 @@ import unittest +from redisgraph.execution_plan import Operation from tests.utils import base import redis @@ -254,12 +255,13 @@ def test_execution_plan(self): expected = "Results\n Project\n Conditional Traverse | (t:Team)->(r:Rider)\n Filter\n Node By Label Scan | (t:Team)" self.assertEqual(str(result), expected) - expected = {'children': [{'children': [{'children': [{'children': [{'op': 'Node By Label Scan | (t:Team)'}], - 'op': 'Filter'}], - 'op': 'Conditional Traverse | (t:Team)->(r:Rider)'}], - 'op': 'Project'}], - 'op': 'Results'} - self.assertEqual(result.operation_tree(), expected) + expected = Operation('Results') \ + .append_child(Operation('Project') \ + .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") \ + .append_child(Operation("Filter") \ + .append_child(Operation('Node By Label Scan', "(t:Team)"))))) + + self.assertEqual(result.structured_plan, expected) redis_graph.delete() From 94014c381c9af004ce3c9b7a8321bc5a47d9aa4c Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 17 Nov 2021 17:43:45 +0200 Subject: [PATCH 07/17] linter issues --- redisgraph/execution_plan.py | 10 +++++----- tests/functional/test_all.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index 6d97c47..3361063 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -11,14 +11,14 @@ def append_child(self, child): def __eq__(self, o: object) -> bool: if not isinstance(o, Operation): return False - + if self.name != o.name or self.args != self.args or len(self.children) != len(o.children): return False for i in range(len(self.children)): if not self.children[i] == o.children[i]: return False - + return True @@ -31,9 +31,9 @@ def __str__(self) -> str: return "\n".join(self.plan) def _operation_tree(self): - i = 0 - level = 0 - stack = [] + i = 0 + level = 0 + stack = [] current = None while i < len(self.plan): diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 89e9e29..f76e6dc 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -256,9 +256,9 @@ def test_execution_plan(self): self.assertEqual(str(result), expected) expected = Operation('Results') \ - .append_child(Operation('Project') \ - .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") \ - .append_child(Operation("Filter") \ + .append_child(Operation('Project') + .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") + .append_child(Operation("Filter") .append_child(Operation('Node By Label Scan', "(t:Team)"))))) self.assertEqual(result.structured_plan, expected) From 7b83651d3471939bae7283b007db249b262e14fb Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 17 Nov 2021 17:56:41 +0200 Subject: [PATCH 08/17] fix indent --- tests/functional/test_all.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index f76e6dc..b164f40 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -257,9 +257,9 @@ def test_execution_plan(self): expected = Operation('Results') \ .append_child(Operation('Project') - .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") - .append_child(Operation("Filter") - .append_child(Operation('Node By Label Scan', "(t:Team)"))))) + .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") + .append_child(Operation("Filter") + .append_child(Operation('Node By Label Scan', "(t:Team)"))))) self.assertEqual(result.structured_plan, expected) From c5d7a518c66d242d956902d2c8719471021ed049 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 17 Nov 2021 18:02:43 +0200 Subject: [PATCH 09/17] fix indent --- tests/functional/test_all.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index b164f40..55b231b 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -257,9 +257,9 @@ def test_execution_plan(self): expected = Operation('Results') \ .append_child(Operation('Project') - .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") - .append_child(Operation("Filter") - .append_child(Operation('Node By Label Scan', "(t:Team)"))))) + .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") + .append_child(Operation("Filter") + .append_child(Operation('Node By Label Scan', "(t:Team)"))))) self.assertEqual(result.structured_plan, expected) From fa6c27128b5b41925f392902c3818fb1df9fe19f Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Wed, 17 Nov 2021 18:08:20 +0200 Subject: [PATCH 10/17] fix comparison --- redisgraph/execution_plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index 3361063..d8f8e59 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -12,7 +12,7 @@ def __eq__(self, o: object) -> bool: if not isinstance(o, Operation): return False - if self.name != o.name or self.args != self.args or len(self.children) != len(o.children): + if self.name != o.name or self.args != o.args or len(self.children) != len(o.children): return False for i in range(len(self.children)): From fa56af0c3fb9b68cb5ffa989bd11f455054044df Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Sun, 21 Nov 2021 16:26:27 +0200 Subject: [PATCH 11/17] address review --- redisgraph/execution_plan.py | 16 +++++++++++++++- tests/functional/test_all.py | 31 +++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index d8f8e59..5d8624f 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -5,6 +5,9 @@ def __init__(self, name, args=None): self.children = [] def append_child(self, child): + if not isinstance(child, Operation) or self is child: + raise Exception("child must be Operation") + self.children.append(child) return self @@ -21,9 +24,20 @@ def __eq__(self, o: object) -> bool: return True + def __str__(self) -> str: + args_str = "" if self.args is None else f" | {self.args}" + if len(self.children) == 0: + return f"{self.name}{args_str}" + else: + children = "\n".join([" " + line for op in self.children for line in str(op).splitlines()]) + return f"{self.name}{args_str}\n{children}" + class ExecutionPlan: def __init__(self, plan): + if not isinstance(plan, list): + raise Exception("plan must be array") + self.plan = plan self.structured_plan = self._operation_tree() @@ -46,7 +60,7 @@ def _operation_tree(self): elif op_level == level + 1: args = op.split("|") child = Operation(args[0].strip(), None if len(args) == 1 else args[1].strip()) - current.children.append(child) + current.append_child(child) stack.append(current) current = child level += 1 diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 55b231b..b14567e 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -251,15 +251,34 @@ def test_execution_plan(self): (:Rider {name:'Andrea Dovizioso'})-[:rides]->(:Team {name:'Ducati'})""" redis_graph.query(create_query) - result = redis_graph.execution_plan("MATCH (r:Rider)-[:rides]->(t:Team) WHERE t.name = $name RETURN r.name, t.name, $params", {'name': 'Yehuda'}) - expected = "Results\n Project\n Conditional Traverse | (t:Team)->(r:Rider)\n Filter\n Node By Label Scan | (t:Team)" + result = redis_graph.execution_plan("MATCH (r:Rider)-[:rides]->(t:Team) WHERE t.name = $name RETURN r.name, t.name, $params UNION MATCH (r:Rider)-[:rides]->(t:Team) WHERE t.name = $name RETURN r.name, t.name, $params", {'name': 'Yehuda'}) + expected = '''\ +Results + Distinct + Join + Project + Conditional Traverse | (t:Team)->(r:Rider) + Filter + Node By Label Scan | (t:Team) + Project + Conditional Traverse | (t:Team)->(r:Rider) + Filter + Node By Label Scan | (t:Team)''' + self.assertEquals(str(result.structured_plan), expected) self.assertEqual(str(result), expected) expected = Operation('Results') \ - .append_child(Operation('Project') - .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") - .append_child(Operation("Filter") - .append_child(Operation('Node By Label Scan', "(t:Team)"))))) + .append_child(Operation('Distinct') + .append_child(Operation('Join') + .append_child(Operation('Project') + .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") + .append_child(Operation("Filter") + .append_child(Operation('Node By Label Scan', "(t:Team)"))))) + .append_child(Operation('Project') + .append_child(Operation('Conditional Traverse', "(t:Team)->(r:Rider)") + .append_child(Operation("Filter") + .append_child(Operation('Node By Label Scan', "(t:Team)"))))) + )) self.assertEqual(result.structured_plan, expected) From 92b8b886fe9904744836ea7abcdf23508aefb948 Mon Sep 17 00:00:00 2001 From: swilly22 Date: Mon, 22 Nov 2021 09:21:49 +0200 Subject: [PATCH 12/17] indentation --- tests/functional/test_all.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index b14567e..632e671 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -246,12 +246,19 @@ def test_cached_execution(self): def test_execution_plan(self): redis_graph = Graph('execution_plan', self.r) - create_query = """CREATE (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), - (:Rider {name:'Dani Pedrosa'})-[:rides]->(:Team {name:'Honda'}), - (:Rider {name:'Andrea Dovizioso'})-[:rides]->(:Team {name:'Ducati'})""" + create_query = """CREATE + (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), + (:Rider {name:'Dani Pedrosa'})-[:rides]->(:Team {name:'Honda'}), + (:Rider {name:'Andrea Dovizioso'})-[:rides]->(:Team {name:'Ducati'})""" redis_graph.query(create_query) - result = redis_graph.execution_plan("MATCH (r:Rider)-[:rides]->(t:Team) WHERE t.name = $name RETURN r.name, t.name, $params UNION MATCH (r:Rider)-[:rides]->(t:Team) WHERE t.name = $name RETURN r.name, t.name, $params", {'name': 'Yehuda'}) + result = redis_graph.execution_plan("""MATCH (r:Rider)-[:rides]->(t:Team) + WHERE t.name = $name + RETURN r.name, t.name, $params + UNION + MATCH (r:Rider)-[:rides]->(t:Team) + WHERE t.name = $name + RETURN r.name, t.name, $params""", {'name': 'Yehuda'}) expected = '''\ Results Distinct From 2a18643c64b460f635716b53dbb5f72104874b03 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Mon, 22 Nov 2021 09:59:47 +0200 Subject: [PATCH 13/17] address review --- redisgraph/execution_plan.py | 40 ++++++++++++++++++++++++++++++------ tests/functional/test_all.py | 2 +- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index 5d8624f..bb785be 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -1,5 +1,15 @@ class Operation: + """ + Operation, single operation of execution plan. + """ def __init__(self, name, args=None): + """ + Create a new operation. + + Args: + name: string that represents the name of the operation + args: coperation arguments + """ self.name = name self.args = args self.children = [] @@ -26,23 +36,41 @@ def __eq__(self, o: object) -> bool: def __str__(self) -> str: args_str = "" if self.args is None else f" | {self.args}" - if len(self.children) == 0: - return f"{self.name}{args_str}" - else: - children = "\n".join([" " + line for op in self.children for line in str(op).splitlines()]) - return f"{self.name}{args_str}\n{children}" + return f"{self.name}{args_str}" class ExecutionPlan: + """ + ExecutionPlan, collection of operations. + """ def __init__(self, plan): + """ + Create a new execution plan. + + Args: + plan: array of strings that represents the collection operations + """ if not isinstance(plan, list): raise Exception("plan must be array") self.plan = plan self.structured_plan = self._operation_tree() + def _operation_traverse(self, op, op_f, aggregate_f, combine_f): + child_res = op_f(op) + if len(op.children) == 0: + return child_res + else: + children = [self._operation_traverse(op, op_f, aggregate_f, combine_f) for op in op.children] + return combine_f(child_res, aggregate_f(children)) + def __str__(self) -> str: - return "\n".join(self.plan) + def aggraget_str(str_ops): + return "\n".join([" " + line for str_op in str_ops for line in str_op.splitlines()]) + + def combine_str(x, y): + return f"{x}\n{y}" + return self._operation_traverse(self.structured_plan, str, aggraget_str, combine_str) def _operation_tree(self): i = 0 diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 632e671..12ec968 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -271,7 +271,7 @@ def test_execution_plan(self): Conditional Traverse | (t:Team)->(r:Rider) Filter Node By Label Scan | (t:Team)''' - self.assertEquals(str(result.structured_plan), expected) + self.assertEquals(str(result), expected) self.assertEqual(str(result), expected) expected = Operation('Results') \ From ef428b2ed852f84cb3d72c8f791253e8a68016e7 Mon Sep 17 00:00:00 2001 From: swilly22 Date: Mon, 22 Nov 2021 14:34:44 +0200 Subject: [PATCH 14/17] compare plans --- redisgraph/execution_plan.py | 79 ++++++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 18 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index bb785be..2948f6d 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -1,14 +1,15 @@ class Operation: """ - Operation, single operation of execution plan. + Operation, single operation within execution plan. """ + def __init__(self, name, args=None): """ Create a new operation. Args: name: string that represents the name of the operation - args: coperation arguments + args: operation arguments """ self.name = name self.args = args @@ -21,58 +22,99 @@ def append_child(self, child): self.children.append(child) return self + def child_count(self): + return array_len(self.children) + def __eq__(self, o: object) -> bool: if not isinstance(o, Operation): return False - if self.name != o.name or self.args != o.args or len(self.children) != len(o.children): + if self.name != o.name or self.args != o.args: return False - for i in range(len(self.children)): - if not self.children[i] == o.children[i]: - return False - return True def __str__(self) -> str: args_str = "" if self.args is None else f" | {self.args}" return f"{self.name}{args_str}" - class ExecutionPlan: """ ExecutionPlan, collection of operations. """ + def __init__(self, plan): """ Create a new execution plan. Args: plan: array of strings that represents the collection operations + the output from GRAPH.EXPLAIN """ if not isinstance(plan, list): - raise Exception("plan must be array") + raise Exception("plan must be an array") self.plan = plan self.structured_plan = self._operation_tree() - def _operation_traverse(self, op, op_f, aggregate_f, combine_f): - child_res = op_f(op) - if len(op.children) == 0: - return child_res - else: - children = [self._operation_traverse(op, op_f, aggregate_f, combine_f) for op in op.children] - return combine_f(child_res, aggregate_f(children)) + def _compare_operations(self, root_a, root_b): + """ + Compare execution plan operation tree + + Return: True if operation trees are equal, False otherwise + """ + + # compare current root + if root_a != root_b: + return False + + # make sure root have the same number of children + if root_a.child_count() != root_b.child_count(): + return False + + # recursively compare children + for i in range(root_a.child_count()): + if not self._compare_operations(root_a.children[i], root_b.children[i]): + return False + + return True def __str__(self) -> str: - def aggraget_str(str_ops): - return "\n".join([" " + line for str_op in str_ops for line in str_op.splitlines()]) + def aggraget_str(str_children): + return "\n".join([" " + line for str_child in str_children for line in str_child.splitlines()]) def combine_str(x, y): return f"{x}\n{y}" + return self._operation_traverse(self.structured_plan, str, aggraget_str, combine_str) + def __eq__(self, o: object) -> bool: + """ Compares two execution plans + + Return: True if the two plans are equal False otherwise + """ + # make sure 'o' is an execution-plan + if not isinstance(o, ExecutionPlan): + return False + + # get root for both plans + root_a = self.structured_plan + root_b = o.structured_plan + + # compare execution trees + return self._compare_operations(root_a, root_b) + + def _operation_traverse(self, op, op_f, aggregate_f, combine_f): + # TODO: document function and its arguments + child_res = op_f(op) + if len(op.children) == 0: + return child_res + else: + children = [self._operation_traverse(child, op_f, aggregate_f, combine_f) for child in op.children] + return combine_f(child_res, aggregate_f(children)) + def _operation_tree(self): + # TODO: document! i = 0 level = 0 stack = [] @@ -101,3 +143,4 @@ def _operation_tree(self): else: raise Exception("corrupted plan") return stack[0] + From 412393a5fab73a551f8a0df3cdfbb4d5965a8cdc Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Mon, 22 Nov 2021 15:00:19 +0200 Subject: [PATCH 15/17] document --- redisgraph/execution_plan.py | 46 ++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index 2948f6d..d7b283e 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -23,7 +23,7 @@ def append_child(self, child): return self def child_count(self): - return array_len(self.children) + return len(self.children) def __eq__(self, o: object) -> bool: if not isinstance(o, Operation): @@ -38,6 +38,7 @@ def __str__(self) -> str: args_str = "" if self.args is None else f" | {self.args}" return f"{self.name}{args_str}" + class ExecutionPlan: """ ExecutionPlan, collection of operations. @@ -60,7 +61,7 @@ def __init__(self, plan): def _compare_operations(self, root_a, root_b): """ Compare execution plan operation tree - + Return: True if operation trees are equal, False otherwise """ @@ -103,32 +104,50 @@ def __eq__(self, o: object) -> bool: # compare execution trees return self._compare_operations(root_a, root_b) - + def _operation_traverse(self, op, op_f, aggregate_f, combine_f): - # TODO: document function and its arguments - child_res = op_f(op) + """ + Traverse operation tree recursively applying functions + + Args: + op: operation to traverse + op_f: function applied for each operation + aggregate_f: aggregation function applied for all children of a single operation + combine_f: combine function applied for the operation result and the children result + """ + # apply op_f for each operation + op_res = op_f(op) if len(op.children) == 0: - return child_res + return op_res # no children return else: + # apply _operation_traverse recursively children = [self._operation_traverse(child, op_f, aggregate_f, combine_f) for child in op.children] - return combine_f(child_res, aggregate_f(children)) + # combine the operation result with the children aggregated result + return combine_f(op_res, aggregate_f(children)) def _operation_tree(self): - # TODO: document! + """ Build the operation tree from the string representation """ + + # initial state i = 0 level = 0 stack = [] current = None + # iterate plan operations while i < len(self.plan): - op = self.plan[i] - op_level = op.count(" ") + current_op = self.plan[i] + op_level = current_op.count(" ") if op_level == level: - args = op.split("|") + # if the operation level equal to the current level + # set the current operation and move next + args = current_op.split("|") current = Operation(args[0].strip(), None if len(args) == 1 else args[1].strip()) i += 1 elif op_level == level + 1: - args = op.split("|") + # if the operation is child of the current operation + # add it as child and set as current operation + args = current_op.split("|") child = Operation(args[0].strip(), None if len(args) == 1 else args[1].strip()) current.append_child(child) stack.append(current) @@ -136,6 +155,8 @@ def _operation_tree(self): level += 1 i += 1 elif op_level < level: + # if the operation is not child of current operation + # go back to it's parent operation levels_back = level - op_level + 1 for _ in range(levels_back): current = stack.pop() @@ -143,4 +164,3 @@ def _operation_tree(self): else: raise Exception("corrupted plan") return stack[0] - From 5d87a359caf0dbc1f76b0576550c8da4183869a8 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Tue, 23 Nov 2021 10:07:03 +0200 Subject: [PATCH 16/17] remove redundant equal --- tests/functional/test_all.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/test_all.py b/tests/functional/test_all.py index 12ec968..f750955 100644 --- a/tests/functional/test_all.py +++ b/tests/functional/test_all.py @@ -271,7 +271,6 @@ def test_execution_plan(self): Conditional Traverse | (t:Team)->(r:Rider) Filter Node By Label Scan | (t:Team)''' - self.assertEquals(str(result), expected) self.assertEqual(str(result), expected) expected = Operation('Results') \ From 247b3f2c144f921639ed9e63cd7192ae2bfbed58 Mon Sep 17 00:00:00 2001 From: Avi Avni Date: Sun, 5 Dec 2021 11:24:10 +0200 Subject: [PATCH 17/17] review --- redisgraph/execution_plan.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/redisgraph/execution_plan.py b/redisgraph/execution_plan.py index d7b283e..c13bbfb 100644 --- a/redisgraph/execution_plan.py +++ b/redisgraph/execution_plan.py @@ -29,10 +29,7 @@ def __eq__(self, o: object) -> bool: if not isinstance(o, Operation): return False - if self.name != o.name or self.args != o.args: - return False - - return True + return (self.name == o.name and self.args == o.args) def __str__(self) -> str: args_str = "" if self.args is None else f" | {self.args}"