diff --git a/src/mango/src/mango_cursor.erl b/src/mango/src/mango_cursor.erl index 7997d9ada3d..4cff1e14fb5 100644 --- a/src/mango/src/mango_cursor.erl +++ b/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. \ No newline at end of file + [] + 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. diff --git a/src/mango/src/mango_cursor_text.erl b/src/mango/src/mango_cursor_text.erl index 88abfc00a74..3883bc8f2bb 100644 --- a/src/mango/src/mango_cursor_text.erl +++ b/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. diff --git a/src/mango/src/mango_cursor_view.erl b/src/mango/src/mango_cursor_view.erl index 7c57b14142a..1e2108b7d70 100644 --- a/src/mango/src/mango_cursor_view.erl +++ b/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} diff --git a/src/mango/src/mango_error.erl b/src/mango/src/mango_error.erl index a2c771de1da..ad665e2f346 100644 --- a/src/mango/src/mango_error.erl +++ b/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, diff --git a/src/mango/src/mango_idx.erl b/src/mango/src/mango_idx.erl index 522077f4433..ea5949c0221 100644 --- a/src/mango/src/mango_idx.erl +++ b/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} -> diff --git a/src/mango/test/05-index-selection-test.py b/src/mango/test/05-index-selection-test.py index ef662a918b3..0a5607f3d75 100644 --- a/src/mango/test/05-index-selection-test.py +++ b/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