Walk long OR chains iteratively instead of recursing

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

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<AGROW>
+        end
+    end
+end

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

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<depth>, nested one level per term;
+            % only n1..n<numDocs> 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