diff options
-rw-r--r-- | src/mango/src/mango_cursor.erl | 19 | ||||
-rw-r--r-- | src/mango/src/mango_cursor_special.erl | 7 | ||||
-rw-r--r-- | src/mango/src/mango_cursor_text.erl | 10 | ||||
-rw-r--r-- | src/mango/src/mango_cursor_view.erl | 16 | ||||
-rw-r--r-- | src/mango/src/mango_idx_text.erl | 2 | ||||
-rw-r--r-- | src/mango/src/mango_selector.erl | 12 | ||||
-rw-r--r-- | src/mango/src/mango_selector_text.erl | 29 | ||||
-rw-r--r-- | src/mango/src/mango_util.erl | 2 | ||||
-rw-r--r-- | src/mango/test/02-basic-find-test.py | 1 | ||||
-rw-r--r-- | src/mango/test/21-empty-selector-tests.py | 72 | ||||
-rw-r--r-- | src/mango/test/mango.py | 10 |
11 files changed, 164 insertions, 16 deletions
diff --git a/src/mango/src/mango_cursor.erl b/src/mango/src/mango_cursor.erl index dc2ee74c7..d9eefbdf2 100644 --- a/src/mango/src/mango_cursor.erl +++ b/src/mango/src/mango_cursor.erl @@ -19,7 +19,8 @@ execute/3, maybe_filter_indexes_by_ddoc/2, remove_indexes_with_partial_filter_selector/1, - maybe_add_warning/3 + maybe_add_warning/3, + maybe_noop_range/2 ]). @@ -187,6 +188,22 @@ maybe_add_warning_int(Warning, UserFun, UserAcc) -> {_Go, UserAcc0} = UserFun(Arg, UserAcc), UserAcc0. +% When there is an empty array for certain operators, we don't actually +% want to execute the query so we deny it by making the range [empty]. +% To clarify, we don't want this query to execute: {"$or": []}. Results should +% be empty. We do want this query to execute: {"age": 22, "$or": []}. It should +% return the same results as {"age": 22} +maybe_noop_range({[{Op, []}]}, IndexRanges) -> + Noops = [<<"$all">>, <<"$and">>, <<"$or">>, <<"$in">>], + case lists:member(Op, Noops) of + true -> + [empty]; + false -> + IndexRanges + end; +maybe_noop_range(_, IndexRanges) -> + IndexRanges. + fmt(Format, Args) -> iolist_to_binary(io_lib:format(Format, Args)). diff --git a/src/mango/src/mango_cursor_special.erl b/src/mango/src/mango_cursor_special.erl index f4a760d1c..df1f6d655 100644 --- a/src/mango/src/mango_cursor_special.erl +++ b/src/mango/src/mango_cursor_special.erl @@ -41,12 +41,14 @@ create(Db, Indexes, Selector, Opts) -> Limit = couch_util:get_value(limit, Opts, mango_opts:default_limit()), Skip = couch_util:get_value(skip, Opts, 0), Fields = couch_util:get_value(fields, Opts, all_fields), - Bookmark = couch_util:get_value(bookmark, Opts), + Bookmark = couch_util:get_value(bookmark, Opts), + + IndexRanges1 = mango_cursor:maybe_noop_range(Selector, IndexRanges), {ok, #cursor{ db = Db, index = Index, - ranges = IndexRanges, + ranges = IndexRanges1, selector = Selector, opts = Opts, limit = Limit, @@ -55,7 +57,6 @@ create(Db, Indexes, Selector, Opts) -> bookmark = Bookmark }}. - explain(Cursor) -> mango_cursor_view:explain(Cursor). diff --git a/src/mango/src/mango_cursor_text.erl b/src/mango/src/mango_cursor_text.erl index 8938f3557..e05ab0765 100644 --- a/src/mango/src/mango_cursor_text.erl +++ b/src/mango/src/mango_cursor_text.erl @@ -92,8 +92,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, partition = get_partition(Opts, nil), sort = sort_query(Opts, Selector), raw_bookmark = true @@ -113,7 +114,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_cursor_view.erl b/src/mango/src/mango_cursor_view.erl index f1b753bd7..921b1fde1 100644 --- a/src/mango/src/mango_cursor_view.erl +++ b/src/mango/src/mango_cursor_view.erl @@ -46,10 +46,12 @@ create(Db, Indexes, Selector, Opts) -> Fields = couch_util:get_value(fields, Opts, all_fields), Bookmark = couch_util:get_value(bookmark, Opts), + IndexRanges1 = mango_cursor:maybe_noop_range(Selector, IndexRanges), + {ok, #cursor{ db = Db, index = Index, - ranges = IndexRanges, + ranges = IndexRanges1, selector = Selector, opts = Opts, limit = Limit, @@ -99,12 +101,20 @@ maybe_replace_max_json([H | T] = EndKey) when is_list(EndKey) -> maybe_replace_max_json(EndKey) -> EndKey. + base_args(#cursor{index = Idx, selector = Selector} = Cursor) -> + {StartKey, EndKey} = case Cursor#cursor.ranges of + [empty] -> + {null, null}; + _ -> + {mango_idx:start_key(Idx, Cursor#cursor.ranges), + mango_idx:end_key(Idx, Cursor#cursor.ranges)} + end, #mrargs{ view_type = map, reduce = false, - start_key = mango_idx:start_key(Idx, Cursor#cursor.ranges), - end_key = mango_idx:end_key(Idx, Cursor#cursor.ranges), + start_key = StartKey, + end_key = EndKey, include_docs = true, extra = [{callback, {?MODULE, view_cb}}, {selector, Selector}] }. diff --git a/src/mango/src/mango_idx_text.erl b/src/mango/src/mango_idx_text.erl index 50f6cc866..1d4becfb3 100644 --- a/src/mango/src/mango_idx_text.erl +++ b/src/mango/src/mango_idx_text.erl @@ -126,6 +126,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 fffadcd20..005a6afb3 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 cfa3baf6d..9e1116de6 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 a7347178e..0d31f15f9 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/02-basic-find-test.py b/src/mango/test/02-basic-find-test.py index 0fc4248a8..afdba03a2 100644 --- a/src/mango/test/02-basic-find-test.py +++ b/src/mango/test/02-basic-find-test.py @@ -13,6 +13,7 @@ import mango +import user_docs class BasicFindTests(mango.UserDocsTests): diff --git a/src/mango/test/21-empty-selector-tests.py b/src/mango/test/21-empty-selector-tests.py new file mode 100644 index 000000000..fda18f6e4 --- /dev/null +++ b/src/mango/test/21-empty-selector-tests.py @@ -0,0 +1,72 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may not +# use this file except in compliance with the License. You may obtain a copy of +# the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations under +# the License. + +import json +import mango +import unittest +import user_docs +import math + + +def make_empty_selector_suite(klass): + class EmptySelectorTestCase(klass): + 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"], klass.INDEX_TYPE) + 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"], klass.INDEX_TYPE) + 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"], klass.INDEX_TYPE) + 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"], klass.INDEX_TYPE) + 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"], klass.INDEX_TYPE) + docs = self.db.find({"favorites": {"$nin": []}}) + assert len(docs) == len(user_docs.DOCS) + + return EmptySelectorTestCase + + +class EmptySelectorNoIndexTests( + make_empty_selector_suite(mango.UserDocsTestsNoIndexes) +): + pass + +@unittest.skipUnless(mango.has_text_service(), "requires text service") +class EmptySelectorTextTests(make_empty_selector_suite(mango.UserDocsTextTests)): + pass + + +class EmptySelectorUserDocTests(make_empty_selector_suite(mango.UserDocsTests)): + pass diff --git a/src/mango/test/mango.py b/src/mango/test/mango.py index de8a638a8..03cb85f48 100644 --- a/src/mango/test/mango.py +++ b/src/mango/test/mango.py @@ -314,6 +314,8 @@ class DbPerClass(unittest.TestCase): class UserDocsTests(DbPerClass): + INDEX_TYPE = "json" + @classmethod def setUpClass(klass): super(UserDocsTests, klass).setUpClass() @@ -321,14 +323,16 @@ class UserDocsTests(DbPerClass): class UserDocsTestsNoIndexes(DbPerClass): + INDEX_TYPE = "special" + @classmethod def setUpClass(klass): super(UserDocsTestsNoIndexes, klass).setUpClass() - user_docs.setup(klass.db, index_type="_all_docs") + user_docs.setup(klass.db, index_type=klass.INDEX_TYPE) class UserDocsTextTests(DbPerClass): - + INDEX_TYPE = "text" DEFAULT_FIELD = None FIELDS = None @@ -338,7 +342,7 @@ class UserDocsTextTests(DbPerClass): if has_text_service(): user_docs.setup( klass.db, - index_type="text", + index_type=klass.INDEX_TYPE, default_field=klass.DEFAULT_FIELD, fields=klass.FIELDS, ) |