diff --git a/src/did/+did/+datastructures/fieldsearch.m b/src/did/+did/+datastructures/fieldsearch.m index 705c67d8..270467eb 100644 --- a/src/did/+did/+datastructures/fieldsearch.m +++ b/src/did/+did/+datastructures/fieldsearch.m @@ -161,8 +161,9 @@ end end case {'hasanysubfield_contains_string','hasanysubfield_exact_string'} - if isthere - if ~ (isstruct(value) | iscell(value) ), return; end % must be a structure or cell + % must be a structure or cell; anything else does not match (and + % so, negated, does - hence no early return past the negation) + if isthere && (isstruct(value) || iscell(value)) for i=1:numel(value) if isstruct(value) item = value(i); diff --git a/src/did/+did/database.m b/src/did/+did/database.m index aaf84a83..33fe4214 100644 --- a/src/did/+did/database.m +++ b/src/did/+did/database.m @@ -1053,7 +1053,6 @@ function close_doc(database_obj, file_obj) end param1Like = did.database.sqlEscapeLiteral( ... regexprep(num2str(param1),{'\\','\*','_'},{'\\\\','%','\\_'})); - param2Esc = did.database.sqlEscapeLiteral(num2str(param2)); field_check = ['fields.field_name="' field '"']; op = strtrim(lower(query_struct.operation)); isNot = op(1)=='~'; @@ -1101,8 +1100,16 @@ function close_doc(database_obj, file_obj) fieldNameLike = regexprep(field,{'\\','\*','_'},{'\\\\','%','\\_'}); sql_str = [field_check ' OR fields.field_name like "' fieldNameLike '.%" ESCAPE "\"']; case 'depends_on' + % Matched exactly as did.query/fieldsearch do (see + % dependsOnSqlMatch); a query that cannot be is searched + % by brute force in search_doc_ids and never gets here + valueMatch = did.database.dependsOnSqlMatch(param1, param2); + if isempty(valueMatch) + error('DID:Database:SQL', ['depends_on with name "%s" and value "%s" ' ... + 'cannot be searched in SQL'], num2str(param1), num2str(param2)); + end field_check = 'fields.field_name="meta.depends_on"'; - sql_str = [field_check ' AND ' notStr 'doc_data.value like "%' param1Like ',' param2Esc ';%" ESCAPE "\"']; + sql_str = [field_check ' AND ' notStr valueMatch]; case 'partial_struct' %TODO error('DID:Database:SQL','Query operation "%s" is not yet implemented',op); case 'hasanysubfield_contains_string' %TODO @@ -1238,7 +1245,8 @@ function close_doc(database_obj, file_obj) termOp(termOp=='~') = ''; if did.database.baseIdLookupValues(query_struct(i)) rank(i) = 1; - elseif any(strcmp(termOp, {'regexp','contains_string','hasmember'})) + elseif any(strcmp(termOp, {'regexp','contains_string','hasmember'})) ... + || did.database.isBruteForceDependsOn(query_struct(i)) rank(i) = 4; elseif strcmp(char(query_struct(i).field), 'base.session_id') rank(i) = 3; @@ -1287,6 +1295,8 @@ function close_doc(database_obj, file_obj) if size(doc_ids,1)==1 && size(doc_ids,2)>1 doc_ids = doc_ids'; % ensure column vector end + elseif did.database.isBruteForceDependsOn(query_struct) + doc_ids = brute_force_search(sqlitedb_obj, query_struct, branch_id, candidate_ids); else % leaf scalar query query_str = get_sql_query_str(sqlitedb_obj, query_struct, branch_id, candidate_ids); doc_ids = run_sql_query(sqlitedb_obj, query_str); @@ -1296,6 +1306,36 @@ function close_doc(database_obj, file_obj) doc_ids = doc_ids{1}; %de-cell end end + function doc_ids = brute_force_search(sqlitedb_obj, query_struct, branch_id, candidate_ids) + % brute_force_search - test documents one by one against a query + % + % DOC_IDS = brute_force_search(SQLITEDB_OBJ, QUERY_STRUCT, BRANCH_ID, [CANDIDATE_IDS]) + % + % Loads every document of BRANCH_ID - or, if CANDIDATE_IDS is a + % cell array of document ids, only those - and returns the ids of + % the ones that did.datastructures.fieldsearch matches against the + % search structure of QUERY_STRUCT (see did.query/to_searchstructure). + % Used for the queries that SQL cannot match exactly (see + % isBruteForceDependsOn), as DID-python does. + if nargin > 3 && iscell(candidate_ids) + ids = candidate_ids; + else + ids = sqlitedb_obj.get_doc_ids(branch_id); + end + doc_ids = cell(0,1); + if isempty(ids) + return + end + op = strtrim(lower(char(query_struct.operation))); + searchstruct = did.query(query_struct.field, op, ... + query_struct.param1, query_struct.param2).to_searchstructure(); + docs = sqlitedb_obj.get_docs(ids); + for k = 1 : numel(docs) + if did.datastructures.fieldsearch(docs(k).document_properties, searchstruct) + doc_ids{end+1,1} = docs(k).id(); %#ok + end + end + end function doc_ids = search_base_ids(sqlitedb_obj, ids, branch_id, candidate_ids) % search_base_ids - find the documents whose base.id is one of IDS % @@ -2127,6 +2167,75 @@ function validate_field_type_and_value(database_obj, doc_name, field_name, value s = strjoin(values, ','); end % sqlStringList() + function valueMatch = dependsOnSqlMatch(name, value) + % dependsOnSqlMatch - SQL test of meta.depends_on for depends_on(NAME, VALUE) + % + % VALUE_MATCH = dependsOnSqlMatch(NAME, VALUE) + % + % Returns the SQL condition on doc_data.value (the document's + % meta.depends_on row) that is true exactly when the document + % has a dependency named NAME (any name, if NAME is '*') with + % value VALUE - as did.query/to_searchstructure and fieldsearch + % match it: case-sensitive, and only '*' is a wildcard. + % + % doc2sql stores the dependencies as 'name1,value1;name2,value2;', + % so the test is whether that string, with ';' put in front, + % contains ';NAME,VALUE;' (or, for '*', whether it contains + % ',VALUE;'). instr is a plain case-sensitive substring test; a + % LIKE "%NAME,VALUE;%" was neither anchored ('id,X;' also matched + % 'subject_id,X;', and an empty NAME matched any name) nor + % case-sensitive. This mirrors DID-python's _depends_on_sql_clause. + % + % The test is exact only if NAME and VALUE are non-empty text + % without the ',' and ';' separators. Otherwise VALUE_MATCH is '' + % and the query is searched by brute force (see + % isBruteForceDependsOn). + valueMatch = ''; + if ~did.database.isDependsOnToken(value) + return + end + value = char(value); + if (ischar(name) || isStringScalar(name)) && strcmp(name, '*') + haystack = 'ifnull(doc_data.value,'''')'; + pattern = [',' value ';']; + elseif did.database.isDependsOnToken(name) + haystack = '('';'' || ifnull(doc_data.value,''''))'; + pattern = [';' char(name) ',' value ';']; + else + return + end + % ifnull, so that a document with no dependencies is matched by + % the negated (NOT ...) test as well + valueMatch = ['(instr(' haystack ',' did.database.sqlStringList({pattern}) ') > 0)']; + end % dependsOnSqlMatch() + + function tf = isDependsOnToken(s) + % isDependsOnToken - can S be matched within meta.depends_on? + % + % TF = isDependsOnToken(S) + % + % True if S is non-empty text with no ',' or ';' (the separators + % of the stored 'name,value;' list). See dependsOnSqlMatch. + tf = (ischar(s) || isStringScalar(s)) && ~isempty(char(s)) ... + && ~any(char(s) == ',' | char(s) == ';'); + end % isDependsOnToken() + + function tf = isBruteForceDependsOn(query_struct) + % isBruteForceDependsOn - must a depends_on query be brute-forced? + % + % TF = isBruteForceDependsOn(QUERY_STRUCT) + % + % True if QUERY_STRUCT (a single search structure) is a depends_on + % or ~depends_on query that SQL cannot match exactly (see + % dependsOnSqlMatch), such as one with an empty name. Such a query + % is tested document by document with fieldsearch instead, so it + % gives the same answer as on any other DID backend. + op = strtrim(lower(char(query_struct.operation))); + op(op=='~') = ''; + tf = strcmp(op, 'depends_on') && ... + isempty(did.database.dependsOnSqlMatch(query_struct.param1, query_struct.param2)); + end % isBruteForceDependsOn() + function [tf, ids] = baseIdLookupValues(query_struct) % baseIdLookupValues - is a search structure a lookup by base.id? % diff --git a/src/did/+did/query.m b/src/did/+did/query.m index 0da67bf6..2b30f4ef 100644 --- a/src/did/+did/query.m +++ b/src/did/+did/query.m @@ -59,6 +59,8 @@ % | 'or' - are any of the searchstruct elements specified in 'param1' true? % |***'isa' - is 'param1' either a superclass or the document class itself of the DID_DOCUMENT? % |***'depends_on' - does the document depend on an item with name 'param1' and value 'param2'? + % | 'param1' '*' means any name; it is the only wildcard. Both are + % | matched exactly and case-sensitively, by every database. % | % |***to be implemented % |----------------------| diff --git a/tests/+did/+unittest/TestDependsOnParity.m b/tests/+did/+unittest/TestDependsOnParity.m new file mode 100644 index 00000000..69ebec42 --- /dev/null +++ b/tests/+did/+unittest/TestDependsOnParity.m @@ -0,0 +1,137 @@ +classdef TestDependsOnParity < matlab.unittest.TestCase + % depends_on means the same thing on every search path + % + % did.query documents depends_on(NAME, VALUE) as "the document has a + % dependency named NAME with value VALUE", where only NAME '*' is a + % wildcard (any name). did.query/to_searchstructure gives fieldsearch + % exactly that (an exact, case-sensitive match), but sqlitedb searched + % meta.depends_on with LIKE "%NAME,VALUE;%", which is neither anchored + % nor case-sensitive: an empty NAME matched any name, 'id' also matched + % 'subject_id', and 'ID' matched 'id'. + % + % Each case is run on the SQLite search and on fieldsearch, and both + % must return the documents listed. The same documents and cases are in + % DID-python's tests/test_depends_on_parity.py; keep the two identical, + % so that the languages cannot drift apart again. + + properties (Constant) + db_filename = 'test_db_depends_on.sqlite' + end + + properties (TestParameter) + % label = {operation, name, value, expected documents} + searchCase = struct( ... + 'emptyName', {{'depends_on', '', 'X', {}}}, ... + 'starName', {{'depends_on', '*', 'X', {'A','B','C'}}}, ... + 'exactName', {{'depends_on', 'id', 'X', {'B'}}}, ... + 'longerName', {{'depends_on', 'subject_id', 'X', {'A'}}}, ... + 'nameCase', {{'depends_on', 'ID', 'X', {'C'}}}, ... + 'valueCase', {{'depends_on', 'element_id', 'X', {}}}, ... + 'valueCaseLower', {{'depends_on', 'element_id', 'x', {'E'}}}, ... + 'secondDependency', {{'depends_on', 'element_id', 'Y', {'B'}}}, ... + 'starSecondDependency', {{'depends_on', '*', 'Y', {'B'}}}, ... + 'nameValueApart', {{'depends_on', 'id', 'Y', {}}}, ... + 'starIsNotGlob', {{'depends_on', 'subject*', 'X', {}}}, ... + 'noSuchValue', {{'depends_on', '*', 'Z', {}}}, ... + 'notExactName', {{'~depends_on', 'id', 'X', {'A','C','D','E'}}}, ... + 'notStarName', {{'~depends_on', '*', 'X', {'D','E'}}}, ... + 'notEmptyName', {{'~depends_on', '', 'X', {'A','B','C','D','E'}}}, ... + 'notNameCase', {{'~depends_on', 'ID', 'X', {'A','B','D','E'}}}) + end + + properties + db + docs % struct: document label -> did.document + ids % struct: document label -> document id + end + + methods (TestClassSetup) + function setupClass(testCase) + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + testCase.applyFixture(did.test.fixture.PathConstantFixture); + + % label -> {dependency names; dependency values} + dependencies = struct( ... + 'A', {{ {'subject_id'}; {'X'} }}, ... + 'B', {{ {'id', 'element_id'}; {'X', 'Y'} }}, ... + 'C', {{ {'ID'}; {'X'} }}, ... + 'D', {{ {}; {} }}, ... + 'E', {{ {'element_id'}; {'x'} }}); + labels = fieldnames(dependencies); + docList = cell(1, numel(labels)); + for i = 1:numel(labels) + dep = dependencies.(labels{i}); + props = did.document('demoC').document_properties; + props.depends_on = struct('name', dep{1}, 'value', dep{2}); + docList{i} = did.document(props); + testCase.docs.(labels{i}) = docList{i}; + testCase.ids.(labels{i}) = docList{i}.id(); + end + + testCase.db = did.implementations.sqlitedb(testCase.db_filename); + testCase.db.add_branch('a'); + testCase.db.add_docs(docList, 'a', 'Validate', false); + end + end + + methods + function q = caseQuery(~, searchCase) + q = did.query('', searchCase{1}, searchCase{2}, searchCase{3}); + end + + function expected = expectedIds(testCase, searchCase) + expected = cellfun(@(label) testCase.ids.(label), searchCase{4}, ... + 'UniformOutput', false); + expected = did.unittest.TestDependsOnParity.sortedIds(expected); + end + end + + methods (Static) + function ids = sortedIds(ids) + % a sorted column cell array of ids, cell(0,1) if there are none + if isempty(ids) + ids = cell(0, 1); + else + ids = sort(ids(:)); + end + end + end + + methods (Test) + function testSqliteSearch(testCase, searchCase) + found = testCase.db.search(testCase.caseQuery(searchCase)); + testCase.verifyEqual(did.unittest.TestDependsOnParity.sortedIds(found), ... + testCase.expectedIds(searchCase)); + end + + function testFieldsearch(testCase, searchCase) + searchstruct = testCase.caseQuery(searchCase).to_searchstructure(); + labels = fieldnames(testCase.docs); + found = cell(0, 1); + for i = 1:numel(labels) + doc = testCase.docs.(labels{i}); + if did.datastructures.fieldsearch(doc.document_properties, searchstruct) + found{end+1, 1} = doc.id(); %#ok + end + end + testCase.verifyEqual(did.unittest.TestDependsOnParity.sortedIds(found), ... + testCase.expectedIds(searchCase)); + end + + function testSqliteSearchWithinAnd(testCase, searchCase) + % The same, as the second term of an AND: then it is checked + % against the documents the first term found only + q = did.query('', 'isa', 'demoC') & testCase.caseQuery(searchCase); + found = testCase.db.search(q); + testCase.verifyEqual(did.unittest.TestDependsOnParity.sortedIds(found), ... + testCase.expectedIds(searchCase)); + end + + function testTwoDependencies(testCase) + q = did.query('', 'depends_on', 'id', 'X') & ... + did.query('', 'depends_on', 'element_id', 'Y'); + found = testCase.db.search(q); + testCase.verifyEqual(found(:), {testCase.ids.B}); + end + end +end