willholley closed pull request #962: Mango: generate a warning instead of an
error when an user-specified index cannot be used
URL: https://github.com/apache/couchdb/pull/962
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/src/mango/src/mango_cursor.erl b/src/mango/src/mango_cursor.erl
index 7997d9ada3..5108d36b27 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,53 @@ 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) ->
+ NoIndexWarning = 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
+ ok
+ end,
+
+ UseIndexInvalidWarning = case lists:keyfind(use_index, 1, Opts) of
+ {use_index, []} ->
+ NoIndexWarning;
+ {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)]);
+ _ ->
+ NoIndexWarning
+ 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]);
+ _ ->
+ NoIndexWarning
+ end
+ end,
+
+ maybe_add_warning_int(UseIndexInvalidWarning, UserFun, UserAcc).
+
+
+maybe_add_warning_int(ok, _, UserAcc) ->
+ UserAcc;
+
+maybe_add_warning_int(Warning, UserFun, UserAcc) ->
+ Arg = {add_key, warning, Warning},
+ {_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 88abfc00a7..3883bc8f2b 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 7c57b14142..1e2108b7d7 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 4c55ef3f62..ad665e2f34 100644
--- a/src/mango/src/mango_error.erl
+++ b/src/mango/src/mango_error.erl
@@ -21,31 +21,12 @@
]).
-info(mango_idx, {no_usable_index, no_indexes_defined}) ->
- {
- 400,
- <<"no_usable_index">>,
- <<"There are no indexes defined in this database.">>
- };
-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 8e19ebff84..ea5949c022 100644
--- a/src/mango/src/mango_idx.erl
+++ b/src/mango/src/mango_idx.erl
@@ -59,20 +59,16 @@ list(Db) ->
get_usable_indexes(Db, Selector, Opts) ->
ExistingIndexes = mango_idx:list(Db),
- if ExistingIndexes /= [] -> ok; true ->
- ?MANGO_ERROR({no_usable_index, no_indexes_defined})
- end,
- 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 ef662a918b..eec0bd9a60 100644
--- a/src/mango/test/05-index-selection-test.py
+++ b/src/mango/test/05-index-selection-test.py
@@ -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,58 @@ 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))
+
+ # should still return a correct result
+ for d in resp["docs"]:
+ self.assertEqual(d["company"], "Pharmex")
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))
+ self.assertEqual(len(resp["docs"]), 0)
+
+ 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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
