mango: enhance compositionality of `consider_index_coverage/3`

Ideally, the effect of this function should be applied at a single
spot of the code.  When building the base options, covering index
information should be left blank to make it consistent with the
rest of the parameters.

2 files changed, 46 insertions, 34 deletions

diff --git a/src/couch/src/couch_util.erl b/src/couch/src/couch_util.erl
index dc58e2bf6..739df28e5 100644
--- a/src/couch/src/couch_util.erl
+++ b/src/couch/src/couch_util.erl
@@ -23,7 +23,7 @@
 -export([to_binary/1, to_integer/1, to_list/1, url_encode/1]).
 -export([json_encode/1, json_decode/1, json_decode/2]).
 -export([verify/2, simple_call/2, shutdown_sync/1]).
--export([get_value/2, get_value/3]).
+-export([get_value/2, get_value/3, set_value/3]).
 -export([reorder_results/2, reorder_results/3]).
 -export([url_strip_password/1]).
 -export([encode_doc_id/1]).
@@ -263,6 +263,9 @@ get_value(Key, List, Default) ->
             Default
     end.
 
+set_value(Key, List, Value) ->
+    lists:keyreplace(Key, 1, List, {Key, Value}).
+
 get_nested_json_value({Props}, [Key | Keys]) ->
     case couch_util:get_value(Key, Props, nil) of
         nil -> throw({not_found, <<"missing json key: ", Key/binary>>});
diff --git a/src/mango/src/mango_cursor_view.erl b/src/mango/src/mango_cursor_view.erl
index 325727180..474d3bfe6 100644
--- a/src/mango/src/mango_cursor_view.erl
+++ b/src/mango/src/mango_cursor_view.erl
@@ -161,13 +161,6 @@ base_args(#cursor{index = Idx, selector = Selector, fields = Fields} = Cursor) -
                     mango_idx:end_key(Idx, Cursor#cursor.ranges)
                 }
         end,
-    CoveringIndex =
-        case mango_idx_view:covers(Idx, Fields) of
-            true ->
-                Idx;
-            false ->
-                undefined
-        end,
     #mrargs{
         view_type = map,
         reduce = false,
@@ -180,7 +173,7 @@ base_args(#cursor{index = Idx, selector = Selector, fields = Fields} = Cursor) -
             {callback, {?MODULE, view_cb}},
             % TODO remove selector. It supports older nodes during version upgrades.
             {selector, Selector},
-            {callback_args, viewcbargs_new(Selector, Fields, CoveringIndex)},
+            {callback_args, viewcbargs_new(Selector, Fields, undefined)},
 
             {ignore_partition_query_limit, true}
         ]
@@ -572,9 +565,25 @@ apply_opts([{_, _} | Rest], Args) ->
     apply_opts(Rest, Args).
 
 -spec consider_index_coverage(#idx{}, fields(), #mrargs{}) -> #mrargs{}.
-consider_index_coverage(Index, Fields, #mrargs{include_docs = IncludeDocs0} = Args) ->
-    IncludeDocs = IncludeDocs0 andalso (not mango_idx_view:covers(Index, Fields)),
-    Args#mrargs{include_docs = IncludeDocs}.
+consider_index_coverage(Index, Fields, #mrargs{include_docs = IncludeDocs} = Args) ->
+    Covering = mango_idx_view:covers(Index, Fields),
+    Args0 = Args#mrargs{include_docs = IncludeDocs andalso (not Covering)},
+    case
+        {
+            Args0#mrargs.include_docs,
+            Covering,
+            couch_util:get_value(callback_args, Args#mrargs.extra)
+        }
+    of
+        {false, true, ViewCBArgs} when ViewCBArgs =/= undefined ->
+            VCBSelector = viewcbargs_get(selector, ViewCBArgs),
+            VCBFields = viewcbargs_get(fields, ViewCBArgs),
+            ViewCBArgs0 = viewcbargs_new(VCBSelector, VCBFields, Index),
+            Extra = couch_util:set_value(callback_args, Args#mrargs.extra, ViewCBArgs0),
+            Args0#mrargs{extra = Extra};
+        _ ->
+            Args0
+    end.
 
 -spec doc_member_and_extract(#cursor{}, row_properties()) -> Result when
     Result ::
@@ -678,7 +687,7 @@ base_opts_test() ->
             {callback_args, #{
                 selector => selector,
                 fields => Fields,
-                covering_index => Index
+                covering_index => undefined
             }},
             {ignore_partition_query_limit, true}
         ],
@@ -691,23 +700,7 @@ base_opts_test() ->
             include_docs = true,
             extra = Extra
         },
-    ?assertEqual(MRArgs, base_args(Cursor)),
-
-    % non-covering index
-    Cursor1 = Cursor#cursor{fields = all_fields},
-    Extra1 =
-        [
-            {callback, {mango_cursor_view, view_cb}},
-            {selector, selector},
-            {callback_args, #{
-                selector => selector,
-                fields => all_fields,
-                covering_index => undefined
-            }},
-            {ignore_partition_query_limit, true}
-        ],
-    MRArgs1 = MRArgs#mrargs{extra = Extra1},
-    ?assertEqual(MRArgs1, base_args(Cursor1)).
+    ?assertEqual(MRArgs, base_args(Cursor)).
 
 apply_opts_empty_test() ->
     ?assertEqual(args, apply_opts([], args)).
@@ -780,10 +773,18 @@ consider_index_coverage_positive_test() ->
             def = {[{<<"fields">>, {[]}}]}
         },
     Fields = [<<"_id">>],
-    MRArgs = #mrargs{include_docs = true},
-    MRArgsRef = MRArgs#mrargs{include_docs = false},
+    MRArgs =
+        #mrargs{
+            include_docs = true,
+            extra = [{callback_args, viewcbargs_new(selector, fields, undefined)}]
+        },
+    MRArgsRef =
+        MRArgs#mrargs{
+            include_docs = false,
+            extra = [{callback_args, viewcbargs_new(selector, fields, Index)}]
+        },
     ?assertEqual(MRArgsRef, consider_index_coverage(Index, Fields, MRArgs)),
-    MRArgs1 = #mrargs{include_docs = false},
+    MRArgs1 = MRArgs#mrargs{include_docs = false},
     ?assertEqual(MRArgsRef, consider_index_coverage(Index, Fields, MRArgs1)).
 
 consider_index_coverage_negative_test() ->
@@ -792,7 +793,15 @@ consider_index_coverage_negative_test() ->
     MRArgs = #mrargs{include_docs = true},
     ?assertEqual(MRArgs, consider_index_coverage(Index, Fields, MRArgs)),
     MRArgs1 = #mrargs{include_docs = false},
-    ?assertEqual(MRArgs1, consider_index_coverage(Index, Fields, MRArgs1)).
+    ?assertEqual(MRArgs1, consider_index_coverage(Index, Fields, MRArgs1)),
+    % no extra attributes hence no effect
+    Index1 =
+        #idx{
+            type = <<"json">>,
+            def = {[{<<"fields">>, {[]}}]}
+        },
+    MRArgs2 = #mrargs{include_docs = false},
+    ?assertEqual(MRArgs1, consider_index_coverage(Index1, [<<"_id">>], MRArgs2)).
 
 derive_doc_from_index_test() ->
     Index =