summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWill Holley <willholley@gmail.com>2017-11-30 19:53:19 +0000
committerGitHub <noreply@github.com>2017-11-30 19:53:19 +0000
commit743bd8820cf612e830c1ff6dd0b9b07c12aab8fb (patch)
tree22f0761c9574690bbe010b354f2798d7cf8dbf03
parentccb657ea736d21c35ff967cabad149ee5bda4431 (diff)
downloadcouchdb-743bd8820cf612e830c1ff6dd0b9b07c12aab8fb.tar.gz
warn instead of error when use_index not valid (#962)
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 #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.
-rw-r--r--src/mango/src/mango_cursor.erl76
-rw-r--r--src/mango/src/mango_cursor_text.erl2
-rw-r--r--src/mango/src/mango_cursor_view.erl2
-rw-r--r--src/mango/src/mango_error.erl19
-rw-r--r--src/mango/src/mango_idx.erl14
-rw-r--r--src/mango/test/05-index-selection-test.py80
6 files changed, 112 insertions, 81 deletions
diff --git a/src/mango/src/mango_cursor.erl b/src/mango/src/mango_cursor.erl
index 7997d9ada..5108d36b2 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 88abfc00a..3883bc8f2 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 7c57b1414..1e2108b7d 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 4c55ef3f6..ad665e2f3 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 8e19ebff8..ea5949c02 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 ef662a918..eec0bd9a6 100644
--- a/src/mango/test/05-index-selection-test.py
+++ b/src/mango/test/05-index-selection-test.py
@@ -66,12 +66,8 @@ class IndexSelectionTests:
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 @@ class IndexSelectionTests:
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 @@ class IndexSelectionTests:
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