Fallback to valid index when invalid use_index specified

If a user specifies a value for use_index that is not
valid for the selector - i.e. it does not meet the coverage
requirements of the selector or sort fields - attempt
to fall back to a valid index (or database scan) rather
than returning a 400 error.

When a fallback occurs, populate the "warning" field
in the response (as we already do when a full database
scan takes place) with details of the fallback.

This change is partially as mitigation for apache#816, which may
lead to some previously valid indexes being deemed invalid,
and also to make use_index less brittle in general. If
an index that is used explicitly by active queries is removed,
Couch will now generate warnings and there may be a performance
impact, but the client will still get correct results.

src/mango/src/mango_cursor.erl

@@ -18,6 +18,7 @@
     explain/1,
     execute/3,
     maybe_filter_indexes_by_ddoc/2,
+    remove_indexes_with_partial_filter_selector/1,
     maybe_add_warning/3
 ]).
 
@@ -47,16 +48,18 @@
 create(Db, Selector0, Opts) ->
     Selector = mango_selector:normalize(Selector0),
     UsableIndexes = mango_idx:get_usable_indexes(Db, Selector, Opts),
-
-    {use_index, IndexSpecified} = proplists:lookup(use_index, Opts),
-    case {length(UsableIndexes), length(IndexSpecified)} of
-        {0, 0} ->
+    case length(UsableIndexes) of
+        0 ->
             AllDocs = mango_idx:special(Db),
             create_cursor(Db, AllDocs, Selector, Opts);
-        {0, _} ->
-            ?MANGO_ERROR({no_usable_index, selector_unsupported});
         _ ->
-            create_cursor(Db, UsableIndexes, Selector, Opts)
+            case mango_cursor:maybe_filter_indexes_by_ddoc(UsableIndexes, Opts) of
+                [] ->
+                    % use_index doesn't match a valid index - fall back to a valid one
+                    create_cursor(Db, UsableIndexes, Selector, Opts);
+                UserSpecifiedIndex ->
+                    create_cursor(Db, UserSpecifiedIndex, Selector, Opts)
+            end
     end.
 
 
@@ -90,9 +93,7 @@ execute(#cursor{index=Idx}=Cursor, UserFun, UserAcc) ->
 maybe_filter_indexes_by_ddoc(Indexes, Opts) ->
     case lists:keyfind(use_index, 1, Opts) of
         {use_index, []} ->
-            % We remove any indexes that have a selector 
-            % since they are only used when specified via use_index
-            remove_indexes_with_partial_filter_selector(Indexes);
+            [];
         {use_index, [DesignId]} ->
             filter_indexes(Indexes, DesignId);
         {use_index, [DesignId, ViewName]} ->
@@ -150,12 +151,54 @@ group_indexes_by_type(Indexes) ->
     end, ?CURSOR_MODULES).
 
 
-maybe_add_warning(UserFun, #idx{type = IndexType}, UserAcc) ->
-    case IndexType of
+maybe_add_warning(UserFun, #cursor{index = Index, opts = Opts}, UserAcc) ->
+    UseIndexInvalid = case lists:keyfind(use_index, 1, Opts) of
+        {use_index, []} ->
+            [];
+        {use_index, [DesignId]} ->
+            case filter_indexes([Index], DesignId) of
+                [] ->
+                    [fmt("_design/~s was not used because it does not contain a valid index for this query.", 
+                        [ddoc_name(DesignId)])];
+                _ ->
+                    []
+            end;
+        {use_index, [DesignId, ViewName]} ->
+            case filter_indexes([Index], DesignId, ViewName) of
+                [] ->
+                    [fmt("_design/~s, ~s was not used because it is not a valid index for this query.", 
+                        [ddoc_name(DesignId), ViewName])];
+                _ ->
+                    []
+            end
+    end,
+
+    NoIndex = case Index#idx.type of
         <<"special">> ->
-            Arg = {add_key, warning, <<"no matching index found, create an index to optimize query time">>},
-            {_Go, UserAcc0} = UserFun(Arg, UserAcc),
-            UserAcc0;
+            [<<"no matching index found, create an index to optimize query time">>];
         _ ->
-            UserAcc
-    end.
+            []
+    end,
+
+    maybe_add_warning_int(UseIndexInvalid ++ NoIndex, UserFun, UserAcc).
+
+
+maybe_add_warning_int([], _, UserAcc) ->
+   UserAcc;
+
+maybe_add_warning_int(Warnings, UserFun, UserAcc) ->
+    % only include the first warning
+    Arg = {add_key, warning, hd(Warnings)},
+    {_Go, UserAcc0} = UserFun(Arg, UserAcc),
+    UserAcc0.
+
+
+fmt(Format, Args) ->
+    iolist_to_binary(io_lib:format(Format, Args)).
+
+
+ddoc_name(<<"_design/", Name/binary>>) ->
+    Name;
+
+ddoc_name(Name) ->
+    Name.

src/mango/src/mango_cursor_text.erl

@@ -124,7 +124,7 @@ execute(Cursor, UserFun, UserAcc) ->
             Arg = {add_key, bookmark, JsonBM},
             {_Go, FinalUserAcc} = UserFun(Arg, LastUserAcc),
             FinalUserAcc0 = mango_execution_stats:maybe_add_stats(Opts, UserFun, Stats0, FinalUserAcc),
-            FinalUserAcc1 = mango_cursor:maybe_add_warning(UserFun, Idx, FinalUserAcc0),
+            FinalUserAcc1 = mango_cursor:maybe_add_warning(UserFun, Cursor, FinalUserAcc0),
             {ok, FinalUserAcc1}
     end.
 

src/mango/src/mango_cursor_view.erl

@@ -137,7 +137,7 @@ execute(#cursor{db = Db, index = Idx, execution_stats = Stats} = Cursor0, UserFu
                     {_Go, FinalUserAcc} = UserFun(Arg, LastCursor#cursor.user_acc),
                     Stats0 = LastCursor#cursor.execution_stats,
                     FinalUserAcc0 = mango_execution_stats:maybe_add_stats(Opts, UserFun, Stats0, FinalUserAcc),
-                    FinalUserAcc1 = mango_cursor:maybe_add_warning(UserFun, Idx, FinalUserAcc0),
+                    FinalUserAcc1 = mango_cursor:maybe_add_warning(UserFun, Cursor, FinalUserAcc0),
                     {ok, FinalUserAcc1};
                 {error, Reason} ->
                     {error, Reason}

src/mango/src/mango_error.erl

@@ -21,25 +21,12 @@
 ]).
 
 
-info(mango_idx, {no_usable_index, no_index_matching_name}) ->
-    {
-        400,
-        <<"no_usable_index">>,
-        <<"No index matches the index specified with \"use_index\"">>
-    };
 info(mango_idx, {no_usable_index, missing_sort_index}) ->
     {
         400,
         <<"no_usable_index">>,
         <<"No index exists for this sort, try indexing by the sort fields.">>
     };
-info(mango_cursor, {no_usable_index, selector_unsupported}) ->
-    {
-        400,
-        <<"no_usable_index">>,
-        <<"The index specified with \"use_index\" is not usable for the query.">>
-    };
-
 info(mango_json_bookmark, {invalid_bookmark, BadBookmark}) ->
     {
         400,

src/mango/src/mango_idx.erl

@@ -60,16 +60,15 @@ list(Db) ->
 get_usable_indexes(Db, Selector, Opts) ->
     ExistingIndexes = mango_idx:list(Db),
 
-    FilteredIndexes = mango_cursor:maybe_filter_indexes_by_ddoc(ExistingIndexes, Opts),
-    if FilteredIndexes /= [] -> ok; true ->
-        ?MANGO_ERROR({no_usable_index, no_index_matching_name})
-    end,
+    GlobalIndexes = mango_cursor:remove_indexes_with_partial_filter_selector(ExistingIndexes),
+    UserSpecifiedIndex = mango_cursor:maybe_filter_indexes_by_ddoc(ExistingIndexes, Opts),
+    UsableIndexes0 = lists:usort(GlobalIndexes ++ UserSpecifiedIndex),
 
     SortFields = get_sort_fields(Opts),
     UsableFilter = fun(I) -> is_usable(I, Selector, SortFields) end,
-    UsableIndexes0 = lists:filter(UsableFilter, FilteredIndexes),
+    UsableIndexes1 = lists:filter(UsableFilter, UsableIndexes0),
 
-    case maybe_filter_by_sort_fields(UsableIndexes0, SortFields) of
+    case maybe_filter_by_sort_fields(UsableIndexes1, SortFields) of
         {ok, SortIndexes} -> 
             SortIndexes;
         {error, no_usable_index} -> 

src/mango/test/05-index-selection-test.py

@@ -46,7 +46,7 @@ def test_use_most_columns(self):
                 "name.last": "Something or other",
                 "age": {"$gt": 1}
             }, explain=True)
-        self.assertNotEqual(resp["index"]["ddoc"], "_design/" + ddocid)
+        self.assertNotEqual(resp["index"]["ddoc"], ddocid)
 
         resp = self.db.find({
                 "name.first": "Stephanie",
@@ -66,12 +66,8 @@ def test_no_valid_sort_index(self):
     def test_invalid_use_index(self):
         # ddoc id for the age index
         ddocid = "_design/ad3d537c03cd7c6a43cf8dff66ef70ea54c2b40f"
-        try:
-            self.db.find({}, use_index=ddocid)
-        except Exception as e:
-            self.assertEqual(e.response.status_code, 400)
-        else:
-            raise AssertionError("bad find")
+        r = self.db.find({}, use_index=ddocid, return_raw=True)
+        self.assertEqual(r["warning"], '{0} was not used because it does not contain a valid index for this query.'.format(ddocid))
 
     def test_uses_index_when_no_range_or_equals(self):
         # index on ["manager"] should be valid because
@@ -87,19 +83,18 @@ def test_uses_index_when_no_range_or_equals(self):
         resp_explain = self.db.find(selector, explain=True)
         self.assertEqual(resp_explain["index"]["type"], "json")
 
-
     def test_reject_use_index_invalid_fields(self):
         # index on ["company","manager"] which should not be valid
         ddocid = "_design/a0c425a60cf3c3c09e3c537c9ef20059dcef9198"
         selector = {
             "company": "Pharmex"
         }
-        try:
-            self.db.find(selector, use_index=ddocid)
-        except Exception as e:
-            self.assertEqual(e.response.status_code, 400)
-        else:
-            raise AssertionError("did not reject bad use_index")
+        r = self.db.find(selector, use_index=ddocid, return_raw=True)
+        self.assertEqual(r["warning"], '{0} was not used because it does not contain a valid index for this query.'.format(ddocid))
+
+        # should still return a correct result
+        for d in r["docs"]:
+            self.assertEqual(d["company"], "Pharmex")
 
     def test_reject_use_index_ddoc_and_name_invalid_fields(self):
         # index on ["company","manager"] which should not be valid
@@ -108,41 +103,53 @@ def test_reject_use_index_ddoc_and_name_invalid_fields(self):
         selector = {
             "company": "Pharmex"
         }
-        try:
-            self.db.find(selector, use_index=[ddocid,name])
-        except Exception as e:
-            self.assertEqual(e.response.status_code, 400)
-        else:
-            raise AssertionError("did not reject bad use_index")
+
+        resp = self.db.find(selector, use_index=[ddocid,name], return_raw=True)
+        self.assertEqual(resp["warning"], "{0}, {1} was not used because it is not a valid index for this query.".format(ddocid, name))
 
     def test_reject_use_index_sort_order(self):
         # index on ["company","manager"] which should not be valid
+        # and there is no valid fallback (i.e. an index on ["company"])
         ddocid = "_design/a0c425a60cf3c3c09e3c537c9ef20059dcef9198"
         selector = {
-            "company": {"$gt": None},
-            "manager": {"$gt": None}
+            "company": {"$gt": None}
         }
         try:
-            self.db.find(selector, use_index=ddocid, sort=[{"manager":"desc"}])
+            self.db.find(selector, use_index=ddocid, sort=[{"company":"desc"}])
         except Exception as e:
             self.assertEqual(e.response.status_code, 400)
         else:
             raise AssertionError("did not reject bad use_index")
 
-    def test_reject_use_index_ddoc_and_name_sort_order(self):
-        # index on ["company","manager"] which should not be valid
-        ddocid = "_design/a0c425a60cf3c3c09e3c537c9ef20059dcef9198"
-        name = "a0c425a60cf3c3c09e3c537c9ef20059dcef9198"
+    def test_use_index_fallback_if_valid_sort(self):
+        ddocid_valid = "_design/fallbackfoo"
+        ddocid_invalid = "_design/fallbackfoobar"
+        self.db.create_index(fields=["foo"], ddoc=ddocid_invalid)
+        self.db.create_index(fields=["foo", "bar"], ddoc=ddocid_valid)
         selector = {
-            "company": {"$gt": None},
-            "manager": {"$gt": None}
+            "foo": {"$gt": None}
         }
-        try:
-            self.db.find(selector, use_index=[ddocid,name], sort=[{"manager":"desc"}])
-        except Exception as e:
-            self.assertEqual(e.response.status_code, 400)
-        else:
-            raise AssertionError("did not reject bad use_index")
+
+        resp_explain = self.db.find(selector, sort=["foo", "bar"], use_index=ddocid_invalid, explain=True)
+        self.assertEqual(resp_explain["index"]["ddoc"], ddocid_valid)    
+
+        resp = self.db.find(selector, sort=["foo", "bar"], use_index=ddocid_invalid, return_raw=True)
+        self.assertEqual(resp["warning"], '{0} was not used because it does not contain a valid index for this query.'.format(ddocid_invalid))    
+
+    def test_prefer_use_index_over_optimal_index(self):
+        # index on ["company"] even though index on ["company", "manager"] is better
+        ddocid_preferred = "_design/testsuboptimal"
+        self.db.create_index(fields=["baz"], ddoc=ddocid_preferred)
+        self.db.create_index(fields=["baz", "bar"])
+        selector = {
+            "baz": {"$gt": None},
+            "bar": {"$gt": None}
+        }
+        resp = self.db.find(selector, use_index=ddocid_preferred, return_raw=True)
+        self.assertTrue("warning" not in resp)
+
+        resp_explain = self.db.find(selector, use_index=ddocid_preferred, explain=True)
+        self.assertEqual(resp_explain["index"]["ddoc"], ddocid_preferred)
 
     # This doc will not be saved given the new ddoc validation code
     # in couch_mrview