Merge pull request #4792 from pgj/feat/mango/find/use_index_allow_fallback

rnewson · web-flow · commit 43882834197a · 2024-10-18T15:18:25.000Z
feat(`mango`): add `allow_fallback` to control falling back to other indexes on `_find`
diff --git a/src/docs/src/api/database/find.rst b/src/docs/src/api/database/find.rst
@@ -19,11 +19,11 @@
 .. http:post:: /{db}/_find
     :synopsis: Find documents within a given database.
 
-    Find documents using a declarative JSON querying syntax.
-    Queries will use custom indexes, specified using the :ref:`_index <api/db/find/index>`
-    endpoint, if available.
-    Otherwise, they use the built-in :ref:`_all_docs <api/db/all_docs>` index, which
-    can be arbitrarily slow.
+    Find documents using a declarative JSON querying syntax.  Queries
+    will use custom indexes, specified using the :ref:`_index
+    <api/db/find/index>` endpoint, if available.  Otherwise, when
+    allowed, they use the built-in :ref:`_all_docs <api/db/all_docs>`
+    index, which can be arbitrarily slow.
 
     :param db: Database name
 
@@ -50,6 +50,14 @@
         is attempted. Therefore that is more like a hint. When
         fallback occurs, the details are given in the ``warning``
         field of the response. *Optional*
+    :<json boolean allow_fallback: Tell if it is allowed to fall back
+        to another valid index.  This can happen on running a query
+        with an index specified by ``use_index`` which is not deemed
+        usable, or when only the built-in :ref:`_all_docs
+        <api/db/all_docs>` index would be picked in lack of indexes
+        available to support the query.  Disabling this fallback logic
+        causes the endpoint immediately return an error in such cases.
+        Default is ``true``. *Optional*
     :<json boolean conflicts: Include conflicted documents if ``true``.
         Intended use is to easily find conflicted documents, without an
         index or view. Default is ``false``. *Optional*
@@ -1498,7 +1506,8 @@ it easier to take advantage of future improvements to query planning
                 "stale": false,
                 "update": true,
                 "stable": false,
-                "execution_stats": false
+                "execution_stats": false,
+                "allow_fallback": true
             },
             "limit": 2,
             "skip": 0,
diff --git a/src/mango/src/mango_cursor.erl b/src/mango/src/mango_cursor.erl
@@ -51,10 +51,25 @@
 create(Db, Selector0, Opts, Kind) ->
     Selector = mango_selector:normalize(Selector0),
     {UsableIndexes, Trace} = mango_idx:get_usable_indexes(Db, Selector, Opts, Kind),
+    UseIndex = use_index(Opts),
     case 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, Trace}, Selector, Opts);
+            % use_index doesn't match a valid index - determine how
+            % this shall be handled by the further settings
+            case allow_fallback(Opts) orelse (UseIndex == [] andalso length(UsableIndexes) > 1) of
+                true ->
+                    % fall back to a valid index
+                    create_cursor(Db, {UsableIndexes, Trace}, Selector, Opts);
+                false ->
+                    % no usable index but all_docs, no fallback allowed: return an error
+                    Details =
+                        case UseIndex of
+                            [] -> [];
+                            [DesignId] -> [ddoc_name(DesignId)];
+                            [DesignId, ViewName] -> [ddoc_name(DesignId), ViewName]
+                        end,
+                    ?MANGO_ERROR({invalid_index, Details})
+            end;
         UserSpecifiedIndex ->
             create_cursor(Db, {UserSpecifiedIndex, Trace}, Selector, Opts)
     end.
@@ -366,13 +381,21 @@ execute(#cursor{index = Idx} = Cursor, UserFun, UserAcc) ->
     Mod = mango_idx:cursor_mod(Idx),
     Mod:execute(Cursor, UserFun, UserAcc).
 
+use_index(Opts) ->
+    {use_index, UseIndex} = lists:keyfind(use_index, 1, Opts),
+    UseIndex.
+
+allow_fallback(Opts) ->
+    {allow_fallback, AllowFallback} = lists:keyfind(allow_fallback, 1, Opts),
+    AllowFallback.
+
 maybe_filter_indexes_by_ddoc(Indexes, Opts) ->
-    case lists:keyfind(use_index, 1, Opts) of
-        {use_index, []} ->
+    case use_index(Opts) of
+        [] ->
             [];
-        {use_index, [DesignId]} ->
+        [DesignId] ->
             filter_indexes(Indexes, DesignId);
-        {use_index, [DesignId, ViewName]} ->
+        [DesignId, ViewName] ->
             filter_indexes(Indexes, DesignId, ViewName)
     end.
 
@@ -575,7 +598,9 @@ create_test_() ->
         [
             ?TDEF_FE(t_create_regular, 10),
             ?TDEF_FE(t_create_user_specified_index, 10),
-            ?TDEF_FE(t_create_invalid_user_specified_index, 10)
+            ?TDEF_FE(t_create_invalid_user_specified_index, 10),
+            ?TDEF_FE(t_create_invalid_user_specified_index_no_fallback, 10),
+            ?TDEF_FE(t_create_no_suitable_index_no_fallback, 10)
         ]
     }.
 
@@ -591,7 +616,7 @@ t_create_regular(_) ->
             filtered_indexes => sets:from_list(FilteredIndexes),
             indexes_of_type => sets:from_list(IndexesOfType)
         },
-    Options = [{use_index, []}],
+    Options = [{use_index, []}, {allow_fallback, true}],
     meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
     meck:expect(
         mango_idx,
@@ -650,7 +675,7 @@ t_create_invalid_user_specified_index(_) ->
             filtered_indexes => sets:from_list(UsableIndexes),
             indexes_of_type => sets:from_list(IndexesOfType)
         },
-    Options = [{use_index, [<<"foobar">>]}],
+    Options = [{use_index, [<<"foobar">>]}, {allow_fallback, true}],
     meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
     meck:expect(
         mango_idx,
@@ -666,6 +691,64 @@ t_create_invalid_user_specified_index(_) ->
     ),
     ?assertEqual(view_cursor, create(db, selector, Options, target)).
 
+t_create_invalid_user_specified_index_no_fallback(_) ->
+    IndexSpecial = #idx{type = <<"special">>, def = all_docs},
+    IndexView1 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx1">>},
+    IndexView2 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx2">>},
+    IndexView3 = #idx{type = <<"json">>, ddoc = <<"_design/view_idx3">>},
+    UsableIndexes = [IndexSpecial, IndexView1, IndexView2, IndexView3],
+    IndexesOfType = [IndexView1, IndexView2, IndexView3],
+    Trace1 = #{},
+    Trace2 =
+        #{
+            filtered_indexes => sets:from_list(UsableIndexes),
+            indexes_of_type => sets:from_list(IndexesOfType)
+        },
+    UseIndex = [<<"design">>, <<"foobar">>],
+    Options = [{use_index, UseIndex}, {allow_fallback, false}],
+    meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
+    meck:expect(
+        mango_idx,
+        get_usable_indexes,
+        [db, normalized_selector, Options, target],
+        meck:val({UsableIndexes, Trace1})
+    ),
+    meck:expect(
+        mango_cursor_view,
+        create,
+        [db, {IndexesOfType, Trace2}, normalized_selector, Options],
+        meck:val(view_cursor)
+    ),
+    Exception = {mango_error, mango_cursor, {invalid_index, UseIndex}},
+    ?assertThrow(Exception, create(db, selector, Options, target)).
+
+t_create_no_suitable_index_no_fallback(_) ->
+    IndexSpecial = #idx{type = <<"special">>, def = all_docs},
+    UsableIndexes = [IndexSpecial],
+    IndexesOfType = [],
+    Trace1 = #{},
+    Trace2 =
+        #{
+            filtered_indexes => sets:from_list(UsableIndexes),
+            indexes_of_type => sets:from_list(IndexesOfType)
+        },
+    Options = [{use_index, []}, {allow_fallback, false}],
+    meck:expect(mango_selector, normalize, [selector], meck:val(normalized_selector)),
+    meck:expect(
+        mango_idx,
+        get_usable_indexes,
+        [db, normalized_selector, Options, target],
+        meck:val({UsableIndexes, Trace1})
+    ),
+    meck:expect(
+        mango_cursor_view,
+        create,
+        [db, {IndexesOfType, Trace2}, normalized_selector, Options],
+        meck:val(view_cursor)
+    ),
+    Exception = {mango_error, mango_cursor, {invalid_index, []}},
+    ?assertThrow(Exception, create(db, selector, Options, target)).
+
 enhance_candidates_test() ->
     Candidates1 = #{index => #{reason => [], usable => true}},
     Candidates2 = #{index => #{reason => [reason1], usable => true}},
diff --git a/src/mango/src/mango_error.erl b/src/mango/src/mango_error.erl
@@ -48,6 +48,28 @@ info(mango_json_bookmark, {invalid_bookmark, BadBookmark}) ->
         <<"invalid_bookmark">>,
         fmt("Invalid bookmark value: ~s", [?JSON_ENCODE(BadBookmark)])
     };
+info(mango_cursor, {invalid_index, []}) ->
+    {
+        400,
+        <<"invalid_index">>,
+        <<"You must specify an index with the `use_index` parameter.">>
+    };
+info(mango_cursor, {invalid_index, [DDocName]}) ->
+    {
+        400,
+        <<"invalid_index">>,
+        fmt("_design/~s specified by `use_index` could not be found or it is not suitable.", [
+            DDocName
+        ])
+    };
+info(mango_cursor, {invalid_index, [DDocName, ViewName]}) ->
+    {
+        400,
+        <<"invalid_index">>,
+        fmt("_design/~s, ~s specified by `use_index` could not be found or it is not suitable.", [
+            DDocName, ViewName
+        ])
+    };
 info(mango_cursor_text, {invalid_bookmark, BadBookmark}) ->
     {
         400,
@@ -231,6 +253,12 @@ info(mango_opts, {invalid_bulk_docs, Val}) ->
             [Val]
         )
     };
+info(mango_opts, {invalid_index_name, Val}) ->
+    {
+        400,
+        <<"invalid_index_name">>,
+        fmt("Invalid index name: ~w", [Val])
+    };
 info(mango_opts, {invalid_ejson, Val}) ->
     {
         400,
diff --git a/src/mango/src/mango_opts.erl b/src/mango/src/mango_opts.erl
@@ -162,6 +162,12 @@ validate_find({Props}) ->
             {optional, true},
             {default, false},
             {validator, fun mango_opts:is_boolean/1}
+        ]},
+        {<<"allow_fallback">>, [
+            {tag, allow_fallback},
+            {optional, true},
+            {default, true},
+            {validator, fun mango_opts:is_boolean/1}
         ]}
     ],
     validate(Props, Opts).
@@ -256,7 +262,7 @@ validate_bulk_docs(Else) ->
     ?MANGO_ERROR({invalid_bulk_docs, Else}).
 
 validate_use_index(IndexName) when is_binary(IndexName) ->
-    case binary:split(IndexName, <<"/">>) of
+    case binary:split(IndexName, <<"/">>, [global]) of
         [DesignId] ->
             {ok, [DesignId]};
         [<<"_design">>, DesignId] ->
diff --git a/src/mango/test/02-basic-find-test.py b/src/mango/test/02-basic-find-test.py
@@ -311,6 +311,7 @@ def test_explain_options(self):
         assert opts["stale"] == False
         assert opts["update"] == True
         assert opts["use_index"] == []
+        assert opts["allow_fallback"] == True
 
     def test_sort_with_all_docs(self):
         explain = self.db.find(
diff --git a/src/mango/test/05-index-selection-test.py b/src/mango/test/05-index-selection-test.py
@@ -211,6 +211,79 @@ def test_explain_sort_reverse(self):
         )
         self.assertEqual(resp_explain["index"]["type"], "json")
 
+    def test_use_index_with_invalid_name(self):
+        for index in ["foo/bar/baz", ["foo", "bar", "baz"]]:
+            with self.subTest(index=index):
+                try:
+                    self.db.find({"manager": True}, use_index=index)
+                except Exception as e:
+                    self.assertEqual(e.response.status_code, 400)
+                else:
+                    raise AssertionError("did not fail on invalid index name")
+
+    def test_use_index_without_fallback(self):
+        with self.subTest(use_index="valid", fallback_available=None):
+            docs = self.db.find(
+                {"manager": True}, use_index="manager", allow_fallback=False
+            )
+            assert len(docs) > 0
+
+        with self.subTest(use_index="invalid", fallback_available=True):
+            try:
+                self.db.find(
+                    {"manager": True}, use_index="invalid", allow_fallback=False
+                )
+            except Exception as e:
+                self.assertEqual(e.response.status_code, 400)
+            else:
+                raise AssertionError("did not fail on invalid index for use_index")
+
+        with self.subTest(use_index="empty", fallback_available=True):
+            try:
+                docs = self.db.find(
+                    {"manager": True}, use_index=[], allow_fallback=False
+                )
+                assert len(docs) > 0
+            except Exception as e:
+                raise AssertionError(
+                    "fail due to missing use_index with suitable indexes"
+                )
+
+        with self.subTest(use_index="empty", fallback_available=False):
+            try:
+                self.db.find({"company": "foobar"}, use_index=[], allow_fallback=False)
+            except Exception as e:
+                self.assertEqual(e.response.status_code, 400)
+            else:
+                raise AssertionError(
+                    "did not fail due to missing use_index without suitable indexes"
+                )
+
+        with self.subTest(use_index="invalid", fallback_available=False):
+            try:
+                self.db.find(
+                    {"company": "foobar"}, use_index="invalid", allow_fallback=False
+                )
+            except Exception as e:
+                self.assertEqual(e.response.status_code, 400)
+            else:
+                raise AssertionError("did not fail on invalid index for use_index")
+
+    def test_index_without_fallback(self):
+        try:
+            docs = self.db.find({"manager": True}, allow_fallback=False)
+            assert len(docs) > 0
+        except Exception as e:
+            raise AssertionError("fail on usable indexes")
+
+    def test_no_index_without_fallback(self):
+        try:
+            self.db.find({"company": "foobar"}, allow_fallback=False)
+        except Exception as e:
+            self.assertEqual(e.response.status_code, 400)
+        else:
+            raise AssertionError("did not fail on no usable indexes")
+
 
 class JSONIndexSelectionTests(mango.UserDocsTests, IndexSelectionTests):
     @classmethod
diff --git a/src/mango/test/mango.py b/src/mango/test/mango.py
@@ -282,6 +282,7 @@ def find(
         update=True,
         executionStats=False,
         partition=None,
+        allow_fallback=None,
     ):
         body = {
             "selector": selector,
@@ -301,6 +302,8 @@ def find(
             body["update"] = False
         if executionStats == True:
             body["execution_stats"] = True
+        if allow_fallback is not None:
+            body["allow_fallback"] = allow_fallback
         body = json.dumps(body)
         if partition:
             ppath = "_partition/{}/".format(partition)
