mango: add allow_fallback for user-specified indexes on _find

pgj · pgj · commit e21d0e983001 · 2024-10-18T15:53:13.000+02:00
It is not always beneficial for the performance if the Mango query planner tries to assign an index to the selector. User-specified indexes may save the day, but since they are only hints for the planner, fallbacks may still happen. Introduce the `allow_fallback` flag which can be used to tell if falling back to other indexes is acceptable when an index is explicitly specified by the user. When set to `false`, give up on planning and return an HTTP 400 response right away. This way the user has the chance to learn about the requested but missing index, optionally create it and try again. By default, fallbacks are allowed to maintain backwards compatibility. It is possible to set `allow_fallback` to `true` but currently it coincides with the default behavior hence becomes a no-op in practice. Fixes #4511
diff --git a/src/docs/src/api/database/find.rst b/src/docs/src/api/database/find.rst
@@ -50,6 +50,12 @@
         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 a valid index when requesting a query to use a specific
+        index that is not deemed usable.  Default is ``true``.  This
+        is meant to be used in combination with ``use_index`` and
+        setting ``allow_fallback`` to ``false`` can make the query
+        fail if the user-specified index is not suitable.  *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 +1504,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
@@ -53,8 +53,22 @@ create(Db, Selector0, Opts, Kind) ->
     {UsableIndexes, Trace} = mango_idx:get_usable_indexes(Db, Selector, Opts, Kind),
     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) of
+                true ->
+                    % fall back to a valid index
+                    create_cursor(Db, {UsableIndexes, Trace}, Selector, Opts);
+                false ->
+                    % return an error
+                    Details =
+                        case use_index(Opts) 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 +380,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 +597,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_1, 10),
+            ?TDEF_FE(t_create_invalid_user_specified_index_no_fallback_2, 10)
         ]
     }.
 
@@ -591,7 +615,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 +674,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 +690,68 @@ t_create_invalid_user_specified_index(_) ->
     ),
     ?assertEqual(view_cursor, create(db, selector, Options, target)).
 
+t_create_invalid_user_specified_index_no_fallback_1(_) ->
+    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_invalid_user_specified_index_no_fallback_2(_) ->
+    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 = [],
+    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)).
+
 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,
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).
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,31 @@ def test_explain_sort_reverse(self):
         )
         self.assertEqual(resp_explain["index"]["type"], "json")
 
+    def test_use_index_without_fallback(self):
+        with self.subTest(use_index="valid"):
+            docs = self.db.find(
+                {"manager": True}, use_index="manager", allow_fallback=False
+            )
+            assert len(docs) > 0
+
+        with self.subTest(use_index="invalid"):
+            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")
+
+        with self.subTest(use_index="empty"):
+            try:
+                self.db.find({"manager": True}, 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")
+
 
 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)
