mango: fix definition of index coverage

Covering indexes shall provide all the fields that the selector
may contain, otherwise the derived documents would get dropped on
the "match and extract" phase even if they were matching.  Extend
the integration tests to check this case as well.

3 files changed, 107 insertions, 5 deletions

diff --git a/src/mango/src/mango_cursor_view.erl b/src/mango/src/mango_cursor_view.erl
index 474d3bfe6..4dbea77c8 100644
--- a/src/mango/src/mango_cursor_view.erl
+++ b/src/mango/src/mango_cursor_view.erl
@@ -108,11 +108,18 @@ create(Db, Indexes, Selector, Opts) ->
         bookmark = Bookmark
     }}.
 
+-spec required_fields(#cursor{}) -> fields().
+required_fields(#cursor{fields = all_fields}) ->
+    all_fields;
+required_fields(#cursor{fields = Fields, selector = Selector}) ->
+    lists:usort(Fields ++ mango_selector:fields(Selector)).
+
 -spec explain(#cursor{}) -> nonempty_list(term()).
 explain(#cursor{opts = Opts} = Cursor) ->
     BaseArgs = base_args(Cursor),
     Args0 = apply_opts(Opts, BaseArgs),
-    #cursor{index = Index, fields = Fields} = Cursor,
+    #cursor{index = Index} = Cursor,
+    Fields = required_fields(Cursor),
     Args = consider_index_coverage(Index, Fields, Args0),
 
     [
@@ -197,7 +204,8 @@ execute(#cursor{db = Db, index = Idx, execution_stats = Stats} = Cursor0, UserFu
             BaseArgs = base_args(Cursor),
             #cursor{opts = Opts, bookmark = Bookmark} = Cursor,
             Args0 = apply_opts(Opts, BaseArgs),
-            Args1 = consider_index_coverage(Idx, Cursor#cursor.fields, Args0),
+            Fields = required_fields(Cursor),
+            Args1 = consider_index_coverage(Idx, Fields, Args0),
             Args = mango_json_bookmark:update_args(Bookmark, Args1),
             UserCtx = couch_util:get_value(user_ctx, Opts, #user_ctx{}),
             DbOpts = [{user_ctx, UserCtx}],
@@ -859,6 +867,39 @@ create_test() ->
         },
     ?assertEqual({ok, Cursor}, create(db, Indexes, Selector, Options)).
 
+to_selector(Map) ->
+    test_util:as_selector(Map).
+
+required_fields_all_fields_test() ->
+    Cursor = #cursor{fields = all_fields},
+    ?assertEqual(all_fields, required_fields(Cursor)).
+
+required_fields_disjoint_fields_test() ->
+    Fields1 = [<<"field1">>, <<"field2">>, <<"field3">>],
+    Selector1 = to_selector(#{}),
+    Cursor1 = #cursor{fields = Fields1, selector = Selector1},
+    ?assertEqual([<<"field1">>, <<"field2">>, <<"field3">>], required_fields(Cursor1)),
+    Fields2 = [<<"field1">>, <<"field2">>],
+    Selector2 = to_selector(#{<<"field3">> => undefined, <<"field4">> => undefined}),
+    Cursor2 = #cursor{fields = Fields2, selector = to_selector(Selector2)},
+    ?assertEqual(
+        [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor2)
+    ).
+
+required_fields_overlapping_fields_test() ->
+    Fields1 = [<<"field1">>, <<"field2">>, <<"field3">>],
+    Selector1 = to_selector(#{<<"field3">> => undefined, <<"field4">> => undefined}),
+    Cursor1 = #cursor{fields = Fields1, selector = Selector1},
+    ?assertEqual(
+        [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor1)
+    ),
+    Fields2 = [<<"field3">>, <<"field1">>, <<"field2">>],
+    Selector2 = to_selector(#{<<"field4">> => undefined, <<"field1">> => undefined}),
+    Cursor2 = #cursor{fields = Fields2, selector = Selector2},
+    ?assertEqual(
+        [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor2)
+    ).
+
 explain_test() ->
     Cursor =
         #cursor{
diff --git a/src/mango/src/mango_selector.erl b/src/mango/src/mango_selector.erl
index 7de16bd51..59be7a6eb 100644
--- a/src/mango/src/mango_selector.erl
+++ b/src/mango/src/mango_selector.erl
@@ -16,7 +16,8 @@
     normalize/1,
     match/2,
     has_required_fields/2,
-    is_constant_field/2
+    is_constant_field/2,
+    fields/1
 ]).
 
 -include_lib("couch/include/couch_db.hrl").
@@ -638,6 +639,14 @@ is_constant_field([{[{Field, {[{Cond, _Val}]}}]} | _Rest], Field) ->
 is_constant_field([{[{_UnMatched, _}]} | Rest], Field) ->
     is_constant_field(Rest, Field).
 
+-spec fields(selector()) -> fields().
+fields({[{<<"$", _/binary>>, Args}]}) when is_list(Args) ->
+    lists:flatmap(fun fields/1, Args);
+fields({[{Field, _Cond}]}) ->
+    [Field];
+fields({[]}) ->
+    [].
+
 %%%%%%%% module tests below %%%%%%%%
 
 -ifdef(TEST).
@@ -1007,4 +1016,42 @@ match_demo_test_() ->
         ?_assertEqual(false, Check({[{<<"_id">>, <<"foo">>}, {<<"_rev">>, <<"quux">>}]}))
     ].
 
+fields_of(Selector) ->
+    fields(test_util:as_selector(Selector)).
+
+fields_empty_test() ->
+    ?assertEqual([], fields_of(#{})).
+
+fields_primitive_test() ->
+    Selector = #{<<"field">> => undefined},
+    ?assertEqual([<<"field">>], fields_of(Selector)).
+
+fields_nested_test() ->
+    Selector = #{<<"field1">> => #{<<"field2">> => undefined}},
+    ?assertEqual([<<"field1.field2">>], fields_of(Selector)).
+
+fields_and_test() ->
+    Selector1 = #{<<"$and">> => []},
+    ?assertEqual([], fields_of(Selector1)),
+    Selector2 = #{
+        <<"$and">> => [#{<<"field1">> => undefined}, #{<<"field2">> => undefined}]
+    },
+    ?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
+
+fields_or_test() ->
+    Selector1 = #{<<"$or">> => []},
+    ?assertEqual([], fields_of(Selector1)),
+    Selector2 = #{
+        <<"$or">> => [#{<<"field1">> => undefined}, #{<<"field2">> => undefined}]
+    },
+    ?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
+
+fields_nor_test() ->
+    Selector1 = #{<<"$nor">> => []},
+    ?assertEqual([], fields_of(Selector1)),
+    Selector2 = #{
+        <<"$nor">> => [#{<<"field1">> => undefined}, #{<<"field2">> => undefined}]
+    },
+    ?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
+
 -endif.
diff --git a/src/mango/test/22-covering-index-test.py b/src/mango/test/22-covering-index-test.py
index b2f0202ed..52a7f3612 100644
--- a/src/mango/test/22-covering-index-test.py
+++ b/src/mango/test/22-covering-index-test.py
@@ -21,8 +21,8 @@ class CoveringIndexTests(mango.UserDocsTests):
         self.assertEqual(resp["mrargs"]["include_docs"], False)
         self.assertEqual(resp["covered"], True)
 
-    def is_not_covered(self, selector, fields):
-        resp = self.db.find(selector, fields=fields, explain=True)
+    def is_not_covered(self, selector, fields, use_index=None):
+        resp = self.db.find(selector, fields=fields, use_index=use_index, explain=True)
         self.assertEqual(resp["mrargs"]["include_docs"], True)
         self.assertEqual(resp["covered"], False)
 
@@ -74,6 +74,20 @@ class CoveringIndexTests(mango.UserDocsTests):
     def test_index_does_not_cover_query_partial_selector(self):
         self.is_not_covered({"name.last": "Hernandez"}, ["name.first"])
 
+    def test_index_does_not_cover_selector_with_more_fields(self):
+        self.is_not_covered(
+            {
+                "$and": [
+                    {"age": {"$ne": 23}},
+                    {"twitter": {"$not": {"$regex": "^@.*[0-9]+$"}}},
+                    {"location.address.number": {"$gt": 4288}},
+                    {"location.city": {"$ne": "Pico Rivera"}},
+                ]
+            },
+            ["twitter"],
+            use_index="twitter",
+        )
+
     def test_covering_index_provides_correct_answer_id(self):
         docs = self.db.find({"age": {"$gte": 32}}, fields=["_id"])
         expected = [