From 3abbf5cb96720479a89efd0348cf796a67827686 Mon Sep 17 00:00:00 2001 From: Matthias Veit Date: Fri, 7 Jun 2024 13:51:16 +0200 Subject: [PATCH] handle context correctly --- fixcore/fixcore/db/arango_query.py | 23 +++++++++++++++---- fixcore/fixcore/query/model.py | 2 +- fixcore/tests/fixcore/db/arango_query_test.py | 17 ++++++++++---- fixcore/tests/fixcore/query/model_test.py | 9 +++++++- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/fixcore/fixcore/db/arango_query.py b/fixcore/fixcore/db/arango_query.py index de898e36f0..41a073395a 100644 --- a/fixcore/fixcore/db/arango_query.py +++ b/fixcore/fixcore/db/arango_query.py @@ -119,6 +119,7 @@ def graph_query( ) last_limit = f" LIMIT {ll.offset}, {ll.length}" if (ll := query.current_part.limit) else "" final = f"""{query_str} FOR result in {cursor}{last_limit} RETURN UNSET(result, {unset_props})""".strip() + print(f"\n-----\nQuery : {query}\nAQL : {final}\nBindVars: {json.dumps(ctx.bind_vars)}\n-----\n") return final, ctx.bind_vars @@ -142,6 +143,9 @@ def query_view_string( part = query.first_part crs = ctx.next_crs("v") + def empty_value(value: Any) -> bool: + return not value if isinstance(value, (list, dict)) else value is None + def predicate_term(p: Predicate) -> Tuple[Optional[str], Term]: # TODO: check regex to see if we can use like instead of regex if p.op == "=~": # regex is not supported in the view search @@ -151,7 +155,7 @@ def predicate_term(p: Predicate) -> Tuple[Optional[str], Term]: prop_name_maybe_arr, prop, _ = prop_name_kind(query_model, p.name) prop_name = array_index_re.sub("", prop_name_maybe_arr) var_name = f"{crs}.{prop_name}" - exhaustive = prop_name_maybe_arr == prop_name # mark if this predicate is backed by the view exhaustively + exhaustive = True # mark if this predicate is backed by the view exhaustively # coerce value op = lgt_ops[p.op] if prop.simple_kind.reverse_order and p.op in lgt_ops else p.op @@ -162,14 +166,21 @@ def predicate_term(p: Predicate) -> Tuple[Optional[str], Term]: # handle special cases p_term: Optional[str] = None - if op == "==" and value is None: + if (op == "==" and empty_value(value)) or ( + op == "in" and isinstance(value, list) and all(empty_value(v) for v in value) + ): p_term = f"NOT EXISTS({var_name})" - elif op == "!=" and value is None: + exhaustive = False # for the view: null and [] are not distinguishable + elif (op == "!=" and empty_value(value)) or ( + op == "not in" and isinstance(value, list) and all(empty_value(v) for v in value) + ): p_term = f"EXISTS({var_name})" elif op == "==" and isinstance(value, list): + # the view cannot compare lists, but we can use the IN operator - still needs filtering p_term = f"{var_name} IN @{ctx.add_bind_var(value)}" exhaustive = False elif op not in ("in", "not_in") and isinstance(value, (list, dict)): + # the view is not able to compare lists or dicts -> we need to filter the results exhaustive = False else: p_term = f"{var_name} {op} @{ctx.add_bind_var(value)}" @@ -217,7 +228,8 @@ def view_term(term: Term) -> Tuple[Optional[str], Term]: search_part, term = view_term(part.term) qs = "" if search_part: - query = evolve(query, parts=query.parts[:-1] + [evolve(part, term=term)]) + part = evolve(part, term=term) + query = evolve(query, parts=query.parts[:-1] + [part]) nxt = ctx.next_crs("view") qs = f"LET {nxt} = (FOR {crs} in {start_cursor} SEARCH {search_part} RETURN {crs}) " start_cursor = nxt @@ -769,11 +781,12 @@ def collect_filter(cl: WithClause, depth: int) -> str: out = ctx.next_crs() limited = f" LIMIT {limit.offset}, {limit.length} " if limit else " " + need_sort = p.sort and not query.aggregate query_part += ( f"LET {out} =( FOR {l0crsr} in {in_crsr} " + traversal_filter(clause, l0crsr, 1) + collect_filter(clause, 1) - + (sort("l0_l0_res", p.sort) if p.sort else "") + + (sort("l0_l0_res", p.sort) if need_sort else "") + limited + "RETURN l0_l0_res) " ) diff --git a/fixcore/fixcore/query/model.py b/fixcore/fixcore/query/model.py index 88c5bcb2d4..c82310c5d0 100644 --- a/fixcore/fixcore/query/model.py +++ b/fixcore/fixcore/query/model.py @@ -484,7 +484,7 @@ def with_context(path: str, term: Term) -> Term: else: return term - return with_context(self.name, self) + return with_context(self.name, self.term) def visible_predicates(self) -> List[Predicate]: """ diff --git a/fixcore/tests/fixcore/db/arango_query_test.py b/fixcore/tests/fixcore/db/arango_query_test.py index 110cc7a1fb..d4fdb40cb1 100644 --- a/fixcore/tests/fixcore/db/arango_query_test.py +++ b/fixcore/tests/fixcore/db/arango_query_test.py @@ -384,12 +384,19 @@ def assert_view(query: str, expected: str) -> None: # all reads plain from the view: no search and no filter assert_view("all", 'FOR result in `ns_view` RETURN UNSET(result, ["flat"])') # read only from view - assert_view("is(foo)", 'LET view0 = (FOR v0 in `ns_view` SEARCH v0.kinds == @b0 RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip + assert_view("is(foo)", + 'LET view0 = (FOR v0 in `ns_view` SEARCH v0.kinds == @b0 RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip # read only from view via phrase - assert_view('"test"', 'LET view0 = (FOR v0 in `ns_view` SEARCH ANALYZER(PHRASE(v0.flat, @b0), "delimited") RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip + assert_view('"test"', + 'LET view0 = (FOR v0 in `ns_view` SEARCH ANALYZER(PHRASE(v0.flat, @b0), "delimited") RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip # read only from view via property - assert_view("name==123", 'LET view0 = (FOR v0 in `ns_view` SEARCH v0.name == @b0 RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip + assert_view("name==123", + 'LET view0 = (FOR v0 in `ns_view` SEARCH v0.name == @b0 RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip # cannot use search but needs filter - assert_view("name=~123", 'LET filter0 = (FOR m0 in `ns_view` FILTER (m0.name!=null and REGEX_TEST(m0.name, @b0, true)) RETURN m0) FOR result in filter0 RETURN UNSET(result, ["flat"])') # fmt: skip + assert_view("name=~123", + 'LET filter0 = (FOR m0 in `ns_view` FILTER (m0.name!=null and REGEX_TEST(m0.name, @b0, true)) RETURN m0) FOR result in filter0 RETURN UNSET(result, ["flat"])') # fmt: skip # use search to select the documents, but needs filter for array handling - assert_view("name[*].foo[*].bla=12", 'LET view0 = (FOR v0 in `ns_view` SEARCH v0.name.foo.bla == @b0 RETURN v0) LET filter0 = (LET nested_distinct0 = (FOR m0 in view0 FOR pre0 IN APPEND(TO_ARRAY(m0.name), {_internal: true}) FOR pre1 IN APPEND(TO_ARRAY(pre0.foo), {_internal: true}) FILTER pre1.bla == @b1 AND (pre0._internal!=true AND pre1._internal!=true) RETURN DISTINCT m0) FOR m1 in nested_distinct0 RETURN m1) FOR result in filter0 RETURN UNSET(result, ["flat"])') # fmt: skip + assert_view("name[*].foo[*].bla=12", + 'LET view0 = (FOR v0 in `ns_view` SEARCH v0.name.foo.bla == @b0 RETURN v0) LET filter0 = (LET nested_distinct0 = (FOR m0 in view0 FOR pre0 IN APPEND(TO_ARRAY(m0.name), {_internal: true}) FOR pre1 IN APPEND(TO_ARRAY(pre0.foo), {_internal: true}) FILTER pre1.bla == @b1 AND (pre0._internal!=true AND pre1._internal!=true) RETURN DISTINCT m0) FOR m1 in nested_distinct0 RETURN m1) FOR result in filter0 RETURN UNSET(result, ["flat"])') # fmt: skip + assert_view('is("aws_ec2_instance") and "deleteme" and reported.instance_placement.tenancy == "default"', + 'LET view0 = (FOR v0 in `ns_view` SEARCH ((v0.kinds == @b0 and ANALYZER(PHRASE(v0.flat, @b1), "delimited")) and v0.reported.instance_placement.tenancy == @b2) RETURN v0) FOR result in view0 RETURN UNSET(result, ["flat"])') # fmt: skip diff --git a/fixcore/tests/fixcore/query/model_test.py b/fixcore/tests/fixcore/query/model_test.py index a747184b36..750f5bd08f 100644 --- a/fixcore/tests/fixcore/query/model_test.py +++ b/fixcore/tests/fixcore/query/model_test.py @@ -1,3 +1,5 @@ +from typing import cast + import pytest from hypothesis import given, settings, HealthCheck @@ -19,6 +21,7 @@ NotTerm, FunctionTerm, Limit, + ContextTerm, ) from fixcore.query.query_parser import parse_query from tests.fixcore.query import query @@ -295,9 +298,13 @@ def test_term_contains() -> None: def test_context_predicates() -> None: - query: Query = parse_query("a.b[*].{ a=2 and b[1].bla=3 and c.d[*].{ e=4 and f=5 } }") + query: Query = parse_query("a.b[*].{ a=2 and b[1].bla=3 and c.d[*].{ e=4 or f=5 } }") expected = ["a.b[*].a", "a.b[*].b[1].bla", "a.b[*].c.d[*].e", "a.b[*].c.d[*].f"] assert [str(a.name) for a in query.visible_predicates] == expected + assert ( + str(cast(ContextTerm, query.current_part.term).predicate_term()) + == "((a.b[*].a == 2 and a.b[*].b[1].bla == 3) and (a.b[*].c.d[*].e == 4 or a.b[*].c.d[*].f == 5))" + ) def test_merge_term_combination() -> None: