From 0de9cf2f110f33eb5fb4aa796ad477d1d27ab8bf Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 18:13:43 +0000 Subject: [PATCH] Walk long OR chains iteratively instead of recursing q = q | did.query(...) in a loop nests one 'or' per term. Two places recursed once per level, so a few hundred terms (MATLAB's default recursion limit is 500) hit MATLAB:recursionLimit before anything was matched: - did.database/search_doc_ids, for an OR that is not all base.id lookups (ORed base.id lookups have been walked iteratively since #229); - did.datastructures.fieldsearch, case 'or'. New did.datastructures.orOperands flattens a chain of nested ORs into its operands, left to right, with a stack; both now loop over those. A negated '~or' nested in the chain is kept as one operand, since its negation applies to its whole result. fieldsearch still errors if an operand is not a structure, now for nested operands too, and stops at the first operand that matches. did.query/to_searchstructure does not recurse into ORs and needed no change. Mirrors DID-python's or_operands (DID-python#100/#103). Test: TestDeepOr - with chains of 3x the recursion limit (1,500 terms), built in a loop: search of an OR of other terms, the same ANDed with a number range, an OR of base.id lookups, fieldsearch true and false; and orOperands' order and '~or' handling, with fieldsearch on not(c or d). Not run here: no MATLAB in this environment. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Dv7SxVR8MCu3MKpUvwwUia --- src/did/+did/+datastructures/fieldsearch.m | 13 ++- src/did/+did/+datastructures/orOperands.m | 43 ++++++++ src/did/+did/database.m | 14 ++- tests/+did/+unittest/TestDeepOr.m | 108 +++++++++++++++++++++ 4 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 src/did/+did/+datastructures/orOperands.m create mode 100644 tests/+did/+unittest/TestDeepOr.m diff --git a/src/did/+did/+datastructures/fieldsearch.m b/src/did/+did/+datastructures/fieldsearch.m index 8bfa5c2d..705c67d8 100644 --- a/src/did/+did/+datastructures/fieldsearch.m +++ b/src/did/+did/+datastructures/fieldsearch.m @@ -197,10 +197,19 @@ end end case 'or' - if ~isstruct(searchstruct.param1) | ~isstruct(searchstruct.param2) + % A chain of ORs built in a loop (q = q | ...) nests one level per + % term, so walk its operands instead of recursing once per level, + % which overran the recursion limit after a few hundred terms + operands = did.datastructures.orOperands(searchstruct); + if ~all(cellfun(@isstruct, operands)) error('In operation ''or'', searchstruct ''param1'' and ''param2'' must be an array of structures.'); end - b = (did.datastructures.fieldsearch(A,searchstruct.param1) | did.datastructures.fieldsearch(A,searchstruct.param2)); + for k = 1:numel(operands) + if did.datastructures.fieldsearch(A, operands{k}) + b = 1; + break + end + end otherwise error(['Unknown search operation ' searchstruct.operation ]); end diff --git a/src/did/+did/+datastructures/orOperands.m b/src/did/+did/+datastructures/orOperands.m new file mode 100644 index 00000000..27f3503f --- /dev/null +++ b/src/did/+did/+datastructures/orOperands.m @@ -0,0 +1,43 @@ +function operands = orOperands(searchstruct) + % OROPERANDS - the operands of an 'or' search structure, with nested ORs flattened + % + % OPERANDS = did.datastructures.orOperands(SEARCHSTRUCT) + % + % Returns the operands of the 'or' search structure SEARCHSTRUCT as a cell + % array, in their left-to-right order, with any operand that is itself an + % 'or' replaced by its own operands. + % + % A query built as q = q | did.query(...) 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 MATLAB's + % recursion limit (500 by default). This walks the chain with a stack + % instead, so a caller can evaluate the operands in a loop. + % + % The operation of SEARCHSTRUCT itself is not examined, so the caller + % decides what a negated top-level '~or' means. A nested operand is + % flattened only if it is a single search structure whose operation is + % exactly 'or': a negated '~or' stays one operand, since its negation + % applies to its whole result. Operands are returned as given, empty ones + % included. + % + % Mirrors DID-python's did.datastructures.or_operands (issue #100 there). + % + % See also: did.datastructures.fieldsearch, did.query/or + + arguments + searchstruct (1,1) struct + end + + operands = {}; + pending = {searchstruct.param2, searchstruct.param1}; + while ~isempty(pending) + item = pending{end}; + pending(end) = []; + if isstruct(item) && isscalar(item) && isfield(item, 'operation') ... + && strcmpi(strtrim(char(item.operation)), 'or') + pending(end+1:end+2) = {item.param2, item.param1}; + else + operands{end+1} = item; %#ok + end + end +end diff --git a/src/did/+did/database.m b/src/did/+did/database.m index e134f95c..aaf84a83 100644 --- a/src/did/+did/database.m +++ b/src/did/+did/database.m @@ -1272,9 +1272,17 @@ function close_doc(database_obj, file_obj) % base.id lookups ORed together: search all the ids at once doc_ids = search_base_ids(sqlitedb_obj, ids, branch_id, candidate_ids); else - doc_ids1 = search_doc_ids(sqlitedb_obj, query_struct.param1, branch_id, candidate_ids); - doc_ids2 = search_doc_ids(sqlitedb_obj, query_struct.param2, branch_id, candidate_ids); - doc_ids = union(doc_ids1, doc_ids2); + % Union the results of the operands. A chain of ORs + % built in a loop (q = q | ...) nests one level per + % term, so it is flattened first rather than searched + % by recursing once per level, which overran the + % recursion limit after a few hundred terms. + doc_ids = {}; + operands = did.datastructures.orOperands(query_struct); + for k = 1:numel(operands) + new_doc_ids = search_doc_ids(sqlitedb_obj, operands{k}, branch_id, candidate_ids); + doc_ids = union(doc_ids, new_doc_ids); + end end if size(doc_ids,1)==1 && size(doc_ids,2)>1 doc_ids = doc_ids'; % ensure column vector diff --git a/tests/+did/+unittest/TestDeepOr.m b/tests/+did/+unittest/TestDeepOr.m new file mode 100644 index 00000000..576e5ada --- /dev/null +++ b/tests/+did/+unittest/TestDeepOr.m @@ -0,0 +1,108 @@ +classdef TestDeepOr < matlab.unittest.TestCase + % Long chains of ORs must not run into MATLAB's recursion limit + % + % q = q | did.query(...) in a loop nests one 'or' per term. Searching or + % field-matching such a query used to recurse once per level, so a few + % hundred terms (MATLAB's default recursion limit is 500) raised + % MATLAB:recursionLimit before anything was matched. Both now walk the + % chain through did.datastructures.orOperands. Mirrors DID-python's + % tests/test_deep_or.py (issue #100 there). + + properties (Constant) + db_filename = 'test_db_deep_or.sqlite' + numDocs = 50 + end + + properties + db + ids + depth % number of terms in a chain: well past the recursion limit + end + + methods (TestClassSetup) + function setupClass(testCase) + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + testCase.applyFixture(did.test.fixture.PathConstantFixture); + testCase.depth = 3 * get(0, 'RecursionLimit'); + + testCase.db = did.implementations.sqlitedb(testCase.db_filename); + testCase.db.add_branch('a'); + docs = cell(1, testCase.numDocs); + for i = 1:testCase.numDocs + docs{i} = did.document('demoA', 'base.name', sprintf('n%d', i), ... + 'demoA.value', i); + end + testCase.db.add_docs(docs, 'a', 'Validate', false); + testCase.ids = cellfun(@(d) d.id(), docs, 'UniformOutput', false); + end + end + + methods + function q = nameChain(testCase) + % name = n1 OR n2 OR ... OR n, nested one level per term; + % only n1..n exist + q = did.query('base.name', 'exact_string', 'n1'); + for i = 2:testCase.depth + q = q | did.query('base.name', 'exact_string', sprintf('n%d', i)); + end + end + end + + methods (Test) + function testSearchOfOrChainOfOtherTerms(testCase) + % Not base.id lookups, so search_doc_ids walks the OR itself + found = testCase.db.search(testCase.nameChain()); + testCase.verifyEqual(sort(found(:)), sort(testCase.ids(:)), ... + 'every document whose name is in the chain must be found'); + end + + function testSearchOfOrChainWithAnd(testCase) + q = testCase.nameChain() & did.query('demoA.value', 'lessthaneq', 10); + found = testCase.db.search(q); + testCase.verifyEqual(sort(found(:)), sort(testCase.ids(1:10)')); + end + + function testSearchOfOrChainOfIds(testCase) + q = did.query('base.id', 'exact_string', testCase.ids{1}); + for i = 2:testCase.depth + id = sprintf('no_such_id_%06d', i); + if i <= testCase.numDocs + id = testCase.ids{i}; + end + q = q | did.query('base.id', 'exact_string', id); + end + found = testCase.db.search(q); + testCase.verifyEqual(sort(found(:)), sort(testCase.ids(:))); + end + + function testFieldsearchOfOrChain(testCase) + q = testCase.nameChain(); + testCase.verifyTrue(logical(did.datastructures.fieldsearch( ... + struct('base', struct('name', 'n7')), q.searchstructure))); + testCase.verifyFalse(logical(did.datastructures.fieldsearch( ... + struct('base', struct('name', 'nope')), q.searchstructure))); + end + + function testOrOperandsOrderAndNegatedOr(testCase) + % (a OR b) OR NOT(c OR d), built directly: did.query has no '~or' + leaf = @(v) struct('field', 'base.name', 'operation', 'exact_string', ... + 'param1', v, 'param2', ''); + ab = struct('field', '', 'operation', 'or', 'param1', leaf('a'), 'param2', leaf('b')); + notCd = struct('field', '', 'operation', '~or', 'param1', leaf('c'), 'param2', leaf('d')); + top = struct('field', '', 'operation', 'or', 'param1', ab, 'param2', notCd); + + operands = did.datastructures.orOperands(top); + testCase.verifyNumElements(operands, 3); + testCase.verifyEqual(operands{1}.param1, 'a'); + testCase.verifyEqual(operands{2}.param1, 'b'); + testCase.verifyEqual(operands{3}.operation, '~or', ... + 'a negated OR is one operand: its negation applies to its whole result'); + + matches = @(name) logical(did.datastructures.fieldsearch( ... + struct('base', struct('name', name)), top)); + testCase.verifyTrue(matches('a')); + testCase.verifyTrue(matches('zzz'), 'zzz is neither c nor d'); + testCase.verifyFalse(matches('c'), 'c is neither a nor b, and is c or d'); + end + end +end