diff --git a/src/did/datastructures.py b/src/did/datastructures.py index 63a91f5..6d4d0b4 100644 --- a/src/did/datastructures.py +++ b/src/did/datastructures.py @@ -162,6 +162,34 @@ def struct_partial_match(a, b): return True +def or_operands(search_struct): + """The operands of an OR search structure, with nested ORs flattened. + + ``q1 | q2 | q3 ...`` built in a loop nests one OR per term: + ``or(or(or(q1, q2), q3), ...)``. Evaluating that by recursion costs a + stack frame or more per term, and a few hundred terms overrun Python's + recursion limit (issue #100). This walks the chain with a stack instead + and returns the operands in their left-to-right order, so the caller can + evaluate them in a loop. + + An operand that is itself an OR (a dict, or a one-element list holding + one) is flattened into the chain; a negated OR ('~or') is not -- it is + one operand, as its negation applies to its whole result. Operands are + returned as given, empty ones included: what an empty operand means is + the caller's, as before. + """ + operands = [] + pending = [search_struct.get("param2"), search_struct.get("param1")] + while pending: + item = pending.pop() + inner = item[0] if isinstance(item, list) and len(item) == 1 else item + if isinstance(inner, dict) and str(inner.get("operation", "")).lower() == "or": + pending.extend([inner.get("param2"), inner.get("param1")]) + else: + operands.append(item) + return operands + + def field_search(a, search_struct): """ Searches a dictionary to determine if it matches a search structure. @@ -268,7 +296,8 @@ def field_search(a, search_struct): b = True break elif op_lower == "or": - b = field_search(a, param1) or field_search(a, param2) + # A long OR chain is a deep tree; flatten it rather than recurse + b = any(field_search(a, operand) for operand in or_operands(search_struct)) elif op_lower == "depends_on": # param1 = dependency name, param2 = dependency value if "depends_on" in a: diff --git a/src/did/did_matlab_python_bridge.yaml b/src/did/did_matlab_python_bridge.yaml index 75ddf98..510d754 100644 --- a/src/did/did_matlab_python_bridge.yaml +++ b/src/did/did_matlab_python_bridge.yaml @@ -40,7 +40,7 @@ classes: - name: database type: class matlab_path: "+did/database.m" - matlab_last_sync_hash: "83a2bda" + matlab_last_sync_hash: "0de9cf2" status: ported python_path: "did/database.py" python_class: "Database" @@ -57,6 +57,11 @@ classes: In Python the SQL search is SQLiteDB's, not Database's, so the port is in did/implementations/sqlitedb.py -- see the sqlitedb entry. Nothing changes in did/database.py. + DID-matlab 0de9cf2 (2026-10-24): search_doc_ids unions over + did.datastructures.orOperands for an OR that is not all base.id + lookups, instead of recursing once per nested OR. Ported to + SQLiteDB._search_doc_ids (DID-python #100 / #103); nothing changes in + did/database.py. sync_notes: > Schema validation was ported on 2026-08-28, closing what had been the largest capability gap between the two languages. Python previously had diff --git a/src/did/did_matlab_python_bridge_util.yaml b/src/did/did_matlab_python_bridge_util.yaml index 71bd4af..056d5d8 100644 --- a/src/did/did_matlab_python_bridge_util.yaml +++ b/src/did/did_matlab_python_bridge_util.yaml @@ -317,11 +317,28 @@ functions: - name: fieldsearch type: function matlab_path: "+did/+datastructures/fieldsearch.m" - matlab_last_sync_hash: "3aa892d" + matlab_last_sync_hash: "0de9cf2" status: ported python_path: "did/datastructures.py" python_name: "field_search" - decision_log: "Exact match. Synchronized 2026-03-15." + decision_log: > + Exact match. Synchronized 2026-03-15. DID-matlab 0de9cf2 (2026-10-24): + case 'or' loops over did.datastructures.orOperands instead of recursing + once per nested OR, and stops at the first operand that matches; + field_search does the same with or_operands (DID-python #100 / #103). + + - name: orOperands + type: function + matlab_path: "+did/+datastructures/orOperands.m" + matlab_last_sync_hash: "0de9cf2" + status: ported + python_path: "did/datastructures.py" + python_name: "or_operands" + decision_log: > + Added in both languages together (DID-python #100 / #103, DID-matlab + 0de9cf2): flattens a chain of nested ORs into its operands, left to + right, with a stack; a nested '~or' stays one operand. MATLAB returns a + cell array, Python a list. - name: findclosest type: function diff --git a/src/did/implementations/sqlitedb.py b/src/did/implementations/sqlitedb.py index 1031ac5..e0ebf75 100644 --- a/src/did/implementations/sqlitedb.py +++ b/src/did/implementations/sqlitedb.py @@ -12,6 +12,7 @@ from datetime import datetime, timezone from ..database import Database +from ..datastructures import or_operands from ..file import is_safe_uid @@ -1712,12 +1713,14 @@ def _search_doc_ids(self, search_struct, branch_id, candidates=None): if is_lookup and self._search_indexes_ready: # base.id lookups ORed together: search all the ids at once return self._search_base_ids(lookup_ids, branch_id, candidates) - # OR: union results from param1 and param2 - p1 = search_struct.get("param1") - p2 = search_struct.get("param2") - ids1 = self._search_doc_ids(p1, branch_id, candidates) if p1 else [] - ids2 = self._search_doc_ids(p2, branch_id, candidates) if p2 else [] - result = list(set(ids1) | set(ids2)) + # OR: union the results of its operands. A chain of ORs is + # flattened first (or_operands), so a long one built in a loop + # does not recurse once per term (issue #100). + matched = set() + for operand in or_operands(search_struct): + if operand: + matched |= set(self._search_doc_ids(operand, branch_id, candidates)) + result = list(matched) if negation: if universe is None: universe = self._do_get_doc_ids(branch_id) diff --git a/src/did/query.py b/src/did/query.py index 475d4cf..a0cb98f 100644 --- a/src/did/query.py +++ b/src/did/query.py @@ -92,13 +92,83 @@ def to_search_structure(self): @staticmethod def _resolve_search_structure(ss): - """Recursively resolve a search structure.""" - if isinstance(ss, list): - return [Query._resolve_single(item) for item in ss] - return Query._resolve_single(ss) + """Resolve a search structure: a list (an AND) or a single item. + + A chain of ORs built in a loop (``q = q | ...``) nests one level per + term, so this walks the structure with an explicit stack rather than + by recursion, which ran out of stack after about 1,000 terms (issue + #100). Each node is rebuilt once all its children are: a list from + its resolved items, an OR from its resolved operands, anything else + by :meth:`_resolve_leaf`. The result is what the recursive + definition gives -- see :meth:`_resolve_single` for the per-node rule. + """ + # A node is (object, kind). "struct" is anything an OR operand or the + # top level may be (a list is an AND of items); "single" is one item. + done = {} # (id(object), kind) -> resolved + keep_alive = [] # objects whose id() is a key in done + stack = [(ss, "struct")] + while stack: + node, kind = stack[-1] + key = (id(node), kind) + if key in done: + stack.pop() + continue + children = Query._resolve_children(node, kind) + pending = [c for c in children if (id(c[0]), c[1]) not in done] + if pending: + stack.extend(reversed(pending)) + continue + stack.pop() + keep_alive.append(node) + done[key] = Query._resolve_node(node, kind, done) + return done[(id(ss), "struct")] + + @staticmethod + def _is_or(item): + return ( + isinstance(item, dict) + and str(item.get("operation", "")).removeprefix("~").lower() == "or" + ) + + @staticmethod + def _resolve_children(node, kind): + """The nodes that must be resolved before *node* can be.""" + if kind == "struct": + if isinstance(node, list): + return [(item, "single") for item in node] + return [(node, "single")] + if Query._is_or(node): + return [ + (node.get(p), "struct") for p in ("param1", "param2") if node.get(p) + ] + return [] + + @staticmethod + def _resolve_node(node, kind, done): + """Resolve *node*, whose children are already resolved in *done*.""" + if kind == "struct": + if isinstance(node, list): + return [done[(id(item), "single")] for item in node] + return done[(id(node), "single")] + if Query._is_or(node): + p1, p2 = node.get("param1"), node.get("param2") + return { + "field": node.get("field", ""), + "operation": node.get("operation", ""), + "param1": done[(id(p1), "struct")] if p1 else p1, + "param2": done[(id(p2), "struct")] if p2 else p2, + } + return Query._resolve_leaf(node) @staticmethod def _resolve_single(item): + """Resolve one item: an OR's operands in turn, or a leaf operation.""" + if Query._is_or(item): + return Query._resolve_search_structure(item) + return Query._resolve_leaf(item) + + @staticmethod + def _resolve_leaf(item): if not isinstance(item, dict): return item @@ -138,15 +208,4 @@ def _resolve_single(item): } return resolved - elif op_lower == "or": - # Recursively resolve OR sub-structures - p1 = item.get("param1") - p2 = item.get("param2") - return { - "field": item.get("field", ""), - "operation": operation, - "param1": Query._resolve_search_structure(p1) if p1 else p1, - "param2": Query._resolve_search_structure(p2) if p2 else p2, - } - return item diff --git a/tests/test_deep_or.py b/tests/test_deep_or.py new file mode 100644 index 0000000..d1aabaf --- /dev/null +++ b/tests/test_deep_or.py @@ -0,0 +1,180 @@ +"""Long chains of ORs must not run into Python's recursion limit (issue #100). + +``q = q | Query(...)`` in a loop nests one OR per term. Resolving, searching +and brute-force matching such a query used to recurse once (or more) per +level, so a few hundred to a thousand terms raised RecursionError before the +search ran. Each now walks the chain iteratively. +""" + +import os +import random +import sys +import tempfile +import unittest +import uuid + +from did.datastructures import field_search, or_operands +from did.document import Document +from did.implementations.sqlitedb import SQLiteDB +from did.query import Query + +# Comfortably past the default recursion limit of 1000 +DEPTH = 3 * sys.getrecursionlimit() + + +def _old_resolve_structure(ss): + """The recursive resolver Query used before issue #100, for comparison.""" + if isinstance(ss, list): + return [_old_resolve_single(item) for item in ss] + return _old_resolve_single(ss) + + +def _old_resolve_single(item): + if not isinstance(item, dict): + return item + operation = item.get("operation", "") + negation = operation.startswith("~") + op_lower = operation.removeprefix("~").lower() + if op_lower == "depends_on": + name, value = item.get("param1", ""), item.get("param2", "") + keys, values = ["name", "value"], [name, value] + if name == "*": + keys, values = ["value"], [value] + return { + "field": "depends_on", + "operation": ( + "~hasanysubfield_exact_string" + if negation + else "hasanysubfield_exact_string" + ), + "param1": keys, + "param2": values, + } + if op_lower == "or": + p1, p2 = item.get("param1"), item.get("param2") + return { + "field": item.get("field", ""), + "operation": operation, + "param1": _old_resolve_structure(p1) if p1 else p1, + "param2": _old_resolve_structure(p2) if p2 else p2, + } + return item + + +def _random_query(rng, depth): + """A random query mixing leaves, depends_on, isa, ANDs, ORs and ~ORs.""" + if depth == 0 or rng.random() < 0.3: + kind = rng.choice(["exact", "isa", "depends_on", "~depends_on", "dep_star"]) + if kind == "exact": + return Query("base.name", "exact_string", f"n{rng.randrange(5)}") + if kind == "isa": + return Query("", "isa", "demoA") + if kind == "dep_star": + return Query("", "depends_on", "*", "E1") + return Query("", kind, "element_id", "E1") + left, right = _random_query(rng, depth - 1), _random_query(rng, depth - 1) + combine = rng.choice(["and", "or", "not_or"]) + if combine == "and": + return left & right + q = left | right + if combine == "not_or": + q.search_structure[0]["operation"] = "~or" + return q + + +class TestResolverEquivalence(unittest.TestCase): + def test_iterative_resolver_matches_recursive_one(self): + rng = random.Random(100) + for _ in range(300): + q = _random_query(rng, depth=6) + self.assertEqual( + q.to_search_structure(), _old_resolve_structure(q.search_structure) + ) + + def test_shared_subquery(self): + # the same object reachable twice must resolve the same both times + leaf = Query("", "depends_on", "element_id", "E1") + q = leaf | leaf + q = q & q + self.assertEqual( + q.to_search_structure(), _old_resolve_structure(q.search_structure) + ) + + +class TestOrOperands(unittest.TestCase): + def test_flattens_in_order_and_keeps_negated_ors(self): + a, b, c, d = (Query("base.name", "exact_string", x) for x in "abcd") + not_cd = c | d + not_cd.search_structure[0]["operation"] = "~or" + (struct,) = ((a | b) | not_cd).search_structure + operands = or_operands(struct) + self.assertEqual([o[0].get("param1") for o in operands[:2]], ["a", "b"]) + self.assertEqual(operands[2][0]["operation"], "~or") + self.assertEqual(len(operands), 3) + + +class TestDeepOr(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.tmpdir = tempfile.TemporaryDirectory() + cls.db = SQLiteDB(os.path.join(cls.tmpdir.name, "deep_or.sqlite")) + cls.db.add_branch("a") + docs = [ + Document( + { + "base": { + "id": uuid.uuid4().hex, + "session_id": "S", + "name": f"n{i}", + "datestamp": "2026-10-24T00:00:00.000Z", + }, + "document_class": {"class_name": "thing", "superclasses": []}, + } + ) + for i in range(DEPTH) + ] + cls.db.add_docs(docs, branch_id="a", validate=False) + cls.docs = docs + cls.ids = [d.id() for d in docs] + + @classmethod + def tearDownClass(cls): + cls.db._close_db() + cls.tmpdir.cleanup() + + def chain(self, queries): + q = queries[0] + for other in queries[1:]: + q = q | other # nested left to right, one level per term + return q + + def test_or_of_ids_built_in_a_loop(self): + q = self.chain([Query("base.id", "exact_string", i) for i in self.ids]) + self.assertEqual(sorted(self.db.search(q, branch_id="a")), sorted(self.ids)) + + def test_or_of_other_terms_built_in_a_loop(self): + # not base.id lookups, so _search_doc_ids walks the OR itself + wanted = self.ids[::3] + names = [f"n{i}" for i in range(0, DEPTH, 3)] + q = self.chain([Query("base.name", "exact_string", n) for n in names]) + self.assertEqual(sorted(self.db.search(q, branch_id="a")), sorted(wanted)) + + def test_deep_or_with_and(self): + q = self.chain( + [Query("base.name", "exact_string", f"n{i}") for i in range(DEPTH)] + ) + q = q & Query("base.session_id", "exact_string", "S") + self.assertEqual(len(self.db.search(q, branch_id="a")), DEPTH) + + def test_field_search_on_a_deep_or(self): + q = self.chain( + [Query("base.name", "exact_string", f"n{i}") for i in range(DEPTH)] + ) + (struct,) = q.to_search_structure() + props = self.docs[-1].document_properties + self.assertTrue(field_search(props, struct)) + self.assertFalse(field_search({"base": {"name": "nope"}}, struct)) + + +if __name__ == "__main__": + unittest.main()