Skip to content

Commit

Permalink
Fix incorrect index selection when sort specified
Browse files Browse the repository at this point in the history
Mango has an apparently long-standing bug whereby if an
index is deemed valid for sorting a query but is not valid
according to mango_idx:is_usable, the query planner would
fall back to _all_docs silently, therefore ignoring the
sort order specified in the query.

This reverses the index selection logic so that the
list of usable indexes is generated prior to filtering
them based on the sort order specified in the query.

Similar logic is applied to use_index, allowing us
to generate a more specific error message when a user
specifies an index which isn't valid for the current
selector.
  • Loading branch information
willholley committed Oct 5, 2017
1 parent a84f41a commit 3b5bda4
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 11 deletions.
6 changes: 6 additions & 0 deletions src/mango/src/mango_error.erl
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ info(mango_idx, {no_usable_index, no_index_matching_name}) ->
<<"no_usable_index">>,
<<"No index matches the index specified with \"use_index\"">>
};
info(mango_idx, {no_usable_index, no_usable_index_matching_name}) ->
{
400,
<<"no_usable_index">>,
<<"The index specified with \"use_index\" is not usable for the query.">>
};
info(mango_idx, {no_usable_index, missing_sort_index}) ->
{
400,
Expand Down
56 changes: 46 additions & 10 deletions src/mango/src/mango_idx.erl
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,33 @@ get_usable_indexes(Db, Selector0, Opts) ->
?MANGO_ERROR({no_usable_index, no_index_matching_name})
end,

SortIndexes = mango_idx:for_sort(FilteredIndexes, Opts),
if SortIndexes /= [] -> ok; true ->
?MANGO_ERROR({no_usable_index, missing_sort_index})
UsableFilter = fun(I) -> mango_idx:is_usable(I, Selector) end,
UsableIndexes0 = lists:filter(UsableFilter, FilteredIndexes),

UsableIndexes1 = case has_use_index(Opts) of
true ->
UsableFilteredIndexes = mango_cursor:maybe_filter_indexes_by_ddoc(UsableIndexes0, Opts),
if UsableFilteredIndexes /= [] -> ok; true ->
?MANGO_ERROR({no_usable_index, no_usable_index_matching_name})
end,
UsableFilteredIndexes;
false ->
UsableIndexes0
end,

UsableFilter = fun(I) -> mango_idx:is_usable(I, Selector) end,
lists:filter(UsableFilter, SortIndexes).
% If a sort was specified we have to find an index that
% can satisfy the request.
case has_sort(Opts) of
true ->
SortIndexes = mango_idx:for_sort(UsableIndexes1, Opts),
if SortIndexes /= [] -> ok; true ->
?MANGO_ERROR({no_usable_index, missing_sort_index})
end,
SortIndexes;
false ->
UsableIndexes1
end.


recover(Db) ->
{ok, DDocs0} = mango_util:open_ddocs(Db),
Expand All @@ -93,17 +113,33 @@ recover(Db) ->
end, DDocs)}.


for_sort(Indexes, Opts) ->
% If a sort was specified we have to find an index that
% can satisfy the request.
has_use_index(Opts) ->
erlang:display(<<"sort opts">>),
erlang:display(Opts),
case lists:keyfind(sort, 1, Opts) of
{use_index, _} ->
true;
_ ->
false
end.


has_sort(Opts) ->
case lists:keyfind(sort, 1, Opts) of
{sort, {[]}} ->
false;
{sort, {SProps}} when is_list(SProps) ->
for_sort_int(Indexes, {SProps});
true;
_ ->
Indexes
false
end.


for_sort(Indexes, Opts) ->
{sort, {SProps}} = lists:keyfind(sort, 1, Opts),
for_sort_int(Indexes, {SProps}).


for_sort_int(Indexes, Sort) ->
Fields = mango_sort:fields(Sort),
FilterFun = fun(Idx) ->
Expand Down
14 changes: 14 additions & 0 deletions src/mango/test/02-basic-find-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,20 @@ def test_sort(self):
docs2 = list(reversed(sorted(docs1, key=lambda d: d["age"])))
assert docs1 is not docs2 and docs1 == docs2

def test_sort_desc_complex(self):
docs = self.db.find({
"company": {"$lt": "M"},
"manager": {"$exists": True},
"$or": [
{"company": "Dreamia"},
{"manager": True}
]
}, sort=[{"company":"desc"}])

companies_returned = list(d["company"] for d in docs)
desc_companies = sorted(companies_returned, reverse=True)
self.assertEqual(desc_companies, companies_returned)

def test_fields(self):
selector = {"age": {"$gt": 0}}
docs = self.db.find(selector, fields=["user_id", "location.address"])
Expand Down
2 changes: 1 addition & 1 deletion src/mango/test/13-users-db-find-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def test_multi_cond_or(self):

def test_sort(self):
self.db.create_index(["order", "name"])
selector = {"name": {"$gt": "demo01"}}
selector = {"name": {"$gt": "demo01"}, "order": {"$exists": True}}
docs1 = self.db.find(selector, sort=[{"order": "asc"}])
docs2 = list(sorted(docs1, key=lambda d: d["order"]))
assert docs1 is not docs2 and docs1 == docs2
Expand Down

0 comments on commit 3b5bda4

Please sign in to comment.