This is an automated email from the ASF dual-hosted git repository. tonysun83 pushed a commit to branch fix-empty-selector-issues in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 4c9546cfd4ae2fcd419a3f59b1f585441f07b271 Author: Tony Sun <[email protected]> AuthorDate: Thu Nov 29 22:18:45 2018 -0800 fix empty queries Mango text indexes currently throw function clauses when queries in two situations: 1) Empty Selector 2) Operators with empty arrays. We fix this issue and change the behavior as follows: 1) Any empty selector will fall back on _all_docs (like json indexes). 2) $or, $and, $in, $all that specify an empty arrays will be treated as no-op when combined with other operators. 3) A single no-nop query such as {"$or": []} will not be executed and return nothing just like json indexes. 4) $nin with an empty array will return all docs that match the field, just like json indexes. --- src/mango/src/mango_cursor_text.erl | 10 ++++++++-- src/mango/src/mango_idx_text.erl | 2 ++ src/mango/src/mango_selector.erl | 12 ++++++++++++ src/mango/src/mango_selector_text.erl | 29 +++++++++++++++++++++++++---- src/mango/src/mango_util.erl | 2 ++ src/mango/test/06-basic-text-test.py | 35 +++++++++++++++++++++++++++++++++++ 6 files changed, 84 insertions(+), 6 deletions(-) diff --git a/src/mango/src/mango_cursor_text.erl b/src/mango/src/mango_cursor_text.erl index 3883bc8..d70aed2 100644 --- a/src/mango/src/mango_cursor_text.erl +++ b/src/mango/src/mango_cursor_text.erl @@ -91,8 +91,9 @@ execute(Cursor, UserFun, UserAcc) -> opts = Opts, execution_stats = Stats } = Cursor, + Query = mango_selector_text:convert(Selector), QueryArgs = #index_query_args{ - q = mango_selector_text:convert(Selector), + q = Query, sort = sort_query(Opts, Selector), raw_bookmark = true }, @@ -111,7 +112,12 @@ execute(Cursor, UserFun, UserAcc) -> execution_stats = mango_execution_stats:log_start(Stats) }, try - execute(CAcc) + case Query of + <<>> -> + throw({stop, CAcc}); + _ -> + execute(CAcc) + end catch throw:{stop, FinalCAcc} -> #cacc{ diff --git a/src/mango/src/mango_idx_text.erl b/src/mango/src/mango_idx_text.erl index 29b4441..3b2fe52 100644 --- a/src/mango/src/mango_idx_text.erl +++ b/src/mango/src/mango_idx_text.erl @@ -125,6 +125,8 @@ columns(Idx) -> end. +is_usable(_, Selector, _) when Selector =:= {[]} -> + false; is_usable(Idx, Selector, _) -> case columns(Idx) of all_fields -> diff --git a/src/mango/src/mango_selector.erl b/src/mango/src/mango_selector.erl index fffadcd..005a6af 100644 --- a/src/mango/src/mango_selector.erl +++ b/src/mango/src/mango_selector.erl @@ -399,10 +399,16 @@ negate({[{Field, Cond}]}) -> {[{Field, negate(Cond)}]}. +% We need to treat an empty array as always true. This will be applied +% for $or, $in, $all, $nin as well. +match({[{<<"$and">>, []}]}, _, _) -> + true; match({[{<<"$and">>, Args}]}, Value, Cmp) -> Pred = fun(SubSel) -> match(SubSel, Value, Cmp) end, lists:all(Pred, Args); +match({[{<<"$or">>, []}]}, _, _) -> + true; match({[{<<"$or">>, Args}]}, Value, Cmp) -> Pred = fun(SubSel) -> match(SubSel, Value, Cmp) end, lists:any(Pred, Args); @@ -410,6 +416,8 @@ match({[{<<"$or">>, Args}]}, Value, Cmp) -> match({[{<<"$not">>, Arg}]}, Value, Cmp) -> not match(Arg, Value, Cmp); +match({[{<<"$all">>, []}]}, _, _) -> + true; % All of the values in Args must exist in Values or % Values == hd(Args) if Args is a single element list % that contains a list. @@ -493,6 +501,8 @@ match({[{<<"$gte">>, Arg}]}, Value, Cmp) -> match({[{<<"$gt">>, Arg}]}, Value, Cmp) -> Cmp(Value, Arg) > 0; +match({[{<<"$in">>, []}]}, _, _) -> + true; match({[{<<"$in">>, Args}]}, Values, Cmp) when is_list(Values)-> Pred = fun(Arg) -> lists:foldl(fun(Value,Match) -> @@ -504,6 +514,8 @@ match({[{<<"$in">>, Args}]}, Value, Cmp) -> Pred = fun(Arg) -> Cmp(Value, Arg) == 0 end, lists:any(Pred, Args); +match({[{<<"$nin">>, []}]}, _, _) -> + true; match({[{<<"$nin">>, Args}]}, Values, Cmp) when is_list(Values)-> not match({[{<<"$in">>, Args}]}, Values, Cmp); match({[{<<"$nin">>, Args}]}, Value, Cmp) -> diff --git a/src/mango/src/mango_selector_text.erl b/src/mango/src/mango_selector_text.erl index cfa3baf..9e1116d 100644 --- a/src/mango/src/mango_selector_text.erl +++ b/src/mango/src/mango_selector_text.erl @@ -205,15 +205,36 @@ convert(_Path, {Props} = Sel) when length(Props) > 1 -> erlang:error({unnormalized_selector, Sel}). -to_query({op_and, Args}) when is_list(Args) -> +to_query_nested(Args) -> QueryArgs = lists:map(fun to_query/1, Args), - ["(", mango_util:join(<<" AND ">>, QueryArgs), ")"]; + % removes empty queries that result from selectors with empty arrays + FilterFun = fun(A) -> A =/= [] andalso A =/= "()" end, + lists:filter(FilterFun, QueryArgs). + + +to_query({op_and, []}) -> + []; + +to_query({op_and, Args}) when is_list(Args) -> + case to_query_nested(Args) of + [] -> []; + QueryArgs -> ["(", mango_util:join(<<" AND ">>, QueryArgs), ")"] + end; + +to_query({op_or, []}) -> + []; to_query({op_or, Args}) when is_list(Args) -> - ["(", mango_util:join(" OR ", lists:map(fun to_query/1, Args)), ")"]; + case to_query_nested(Args) of + [] -> []; + QueryArgs -> ["(", mango_util:join(" OR ", QueryArgs), ")"] + end; to_query({op_not, {ExistsQuery, Arg}}) when is_tuple(Arg) -> - ["(", to_query(ExistsQuery), " AND NOT (", to_query(Arg), "))"]; + case to_query(Arg) of + [] -> ["(", to_query(ExistsQuery), ")"]; + Query -> ["(", to_query(ExistsQuery), " AND NOT (", Query, "))"] + end; %% For $exists:false to_query({op_not, {ExistsQuery, false}}) -> diff --git a/src/mango/src/mango_util.erl b/src/mango/src/mango_util.erl index a734717..0d31f15 100644 --- a/src/mango/src/mango_util.erl +++ b/src/mango/src/mango_util.erl @@ -344,6 +344,8 @@ has_suffix(Bin, Suffix) when is_binary(Bin), is_binary(Suffix) -> end. +join(_Sep, []) -> + []; join(_Sep, [Item]) -> [Item]; join(Sep, [Item | Rest]) -> diff --git a/src/mango/test/06-basic-text-test.py b/src/mango/test/06-basic-text-test.py index 3783006..42863c8 100644 --- a/src/mango/test/06-basic-text-test.py +++ b/src/mango/test/06-basic-text-test.py @@ -438,6 +438,41 @@ class BasicTextTests(mango.UserDocsTextTests): assert docs[0]["user_id"] == 2 assert docs[1]["user_id"] == 10 + def test_empty(self): + resp = self.db.find({}, explain=True) + self.assertEqual(resp["index"]["type"], "special") + + def test_empty_array_or(self): + resp = self.db.find({"$or": []}, explain=True) + self.assertEqual(resp["index"]["type"], "text") + docs = self.db.find({"$or": []}) + assert len(docs) == 0 + + def test_empty_array_or_with_age(self): + resp = self.db.find({"age": 22, "$or": []}, explain=True) + self.assertEqual(resp["index"]["type"], "text") + docs = self.db.find({"age": 22, "$or": []}) + assert len(docs) == 1 + + def test_empty_array_and_with_age(self): + resp = self.db.find({"age": 22, "$and": [{"b": {"$all":[]}}]}, + explain=True) + self.assertEqual(resp["index"]["type"], "text") + docs = self.db.find({"age": 22, "$and": []}) + assert len(docs) == 1 + + def test_empty_arrays_complex(self): + resp = self.db.find({"$or": [], "a": {"$in" : []}}, explain=True) + self.assertEqual(resp["index"]["type"], "text") + docs = self.db.find({"$or": [], "a": {"$in" : []}}) + assert len(docs) == 0 + + def test_empty_nin(self): + resp = self.db.find({"favorites": {"$nin" : []}}, explain=True) + self.assertEqual(resp["index"]["type"], "text") + docs = self.db.find({"favorites": {"$nin" : []}}) + assert len(docs) == len(user_docs.DOCS) + # test lucene syntax in $text @unittest.skipUnless(mango.has_text_service(), "requires text service")
