You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by to...@apache.org on 2020/01/17 05:56:01 UTC
[couchdb] branch master updated: fix empty queries (#1783)
This is an automated email from the ASF dual-hosted git repository.
tonysun83 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/master by this push:
new 018ec6b fix empty queries (#1783)
018ec6b is described below
commit 018ec6bf51b64fba48fe960dbbf7cb92b943fc81
Author: Tony Sun <to...@gmail.com>
AuthorDate: Thu Jan 16 21:55:52 2020 -0800
fix empty queries (#1783)
* 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.
Co-authored-by: Jan Lehnardt <ja...@apache.org>
Co-authored-by: Peng Hui Jiang <ji...@apache.org>
---
src/mango/src/mango_cursor.erl | 19 +++++++-
src/mango/src/mango_cursor_special.erl | 7 +--
src/mango/src/mango_cursor_text.erl | 10 ++++-
src/mango/src/mango_cursor_view.erl | 16 +++++--
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/02-basic-find-test.py | 1 +
src/mango/test/21-empty-selector-tests.py | 72 +++++++++++++++++++++++++++++++
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 dc2ee74..d9eefbd 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 f4a760d..df1f6d6 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 8938f35..e05ab07 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 f1b753b..921b1fd 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 50f6cc8..1d4becf 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 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/02-basic-find-test.py b/src/mango/test/02-basic-find-test.py
index 0fc4248..afdba03 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 0000000..fda18f6
--- /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 de8a638..03cb85f 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,
)