Skip to content

Commit

Permalink
Revert "Fix json index selection (apache#816)"
Browse files Browse the repository at this point in the history
This reverts commit dae81be.

Conflicts:
	src/mango/test/05-index-selection-test.py
  • Loading branch information
iilyak committed Nov 13, 2017
1 parent dc89343 commit beb4f63
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 163 deletions.
11 changes: 5 additions & 6 deletions src/mango/src/mango_idx_view.erl
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,11 @@ columns(Idx) ->


is_usable(Idx, Selector) ->
% This index is usable if all of the columns are
% restricted by the selector such that they are required to exist
% and the selector is not a text search (so requires a text index)
RequiredFields = columns(Idx),
mango_selector:has_required_fields(Selector, RequiredFields)
andalso not is_text_search(Selector).
% This index is usable if at least the first column is
% a member of the indexable fields of the selector.
Columns = columns(Idx),
Fields = indexable_fields(Selector),
lists:member(hd(Columns), Fields) andalso not is_text_search(Selector).


is_text_search({[]}) ->
Expand Down
110 changes: 1 addition & 109 deletions src/mango/src/mango_selector.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@

-export([
normalize/1,
match/2,
has_required_fields/2
match/2
]).


Expand Down Expand Up @@ -567,110 +566,3 @@ match({[{Field, Cond}]}, Value, Cmp) ->

match({[_, _ | _] = _Props} = Sel, _Value, _Cmp) ->
erlang:error({unnormalized_selector, Sel}).


% Returns true if Selector requires all
% fields in RequiredFields to exist in any matching documents.

% For each condition in the selector, check
% whether the field is in RequiredFields.
% If it is, remove it from RequiredFields and continue
% until we match then all or run out of selector to
% match against.

% Empty selector
has_required_fields({[]}, _) ->
false;

% No more required fields
has_required_fields(_, []) ->
true;

% No more selector
has_required_fields([], _) ->
false;

has_required_fields(Selector, RequiredFields) when not is_list(Selector) ->
has_required_fields([Selector], RequiredFields);

% We can "see" through $and operator. We ignore other
% combination operators because they can't be used to restrict
% an index.
has_required_fields([{[{<<"$and">>, Args}]}], RequiredFields)
when is_list(Args) ->
has_required_fields(Args, RequiredFields);

has_required_fields([{[{Field, Cond}]} | Rest], RequiredFields) ->
case Cond of
% $exists:false is a special case - this is the only operator
% that explicitly does not require a field to exist
{[{<<"$exists">>, false}]} ->
has_required_fields(Rest, RequiredFields);
_ ->
has_required_fields(Rest, lists:delete(Field, RequiredFields))
end.


%%%%%%%% module tests below %%%%%%%%

-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").

has_required_fields_basic_test() ->
RequiredFields = [<<"A">>],
Selector = {[{<<"A">>, <<"foo">>}]},
Normalized = normalize(Selector),
?assertEqual(true, has_required_fields(Normalized, RequiredFields)).

has_required_fields_basic_failure_test() ->
RequiredFields = [<<"B">>],
Selector = {[{<<"A">>, <<"foo">>}]},
Normalized = normalize(Selector),
?assertEqual(false, has_required_fields(Normalized, RequiredFields)).

has_required_fields_empty_selector_test() ->
RequiredFields = [<<"A">>],
Selector = {[]},
Normalized = normalize(Selector),
?assertEqual(false, has_required_fields(Normalized, RequiredFields)).

has_required_fields_exists_false_test() ->
RequiredFields = [<<"A">>],
Selector = {[{<<"A">>,{[{<<"$exists">>, false}]}}]},
Normalized = normalize(Selector),
?assertEqual(false, has_required_fields(Normalized, RequiredFields)).

has_required_fields_and_true_test() ->
RequiredFields = [<<"A">>],
Selector = {[{<<"$and">>,
[
{[{<<"A">>, <<"foo">>}]},
{[{<<"B">>, <<"foo">>}]}
]
}]},
Normalized = normalize(Selector),
?assertEqual(true, has_required_fields(Normalized, RequiredFields)).

has_required_fields_and_false_test() ->
RequiredFields = [<<"A">>, <<"C">>],
Selector = {[{<<"$and">>,
[
{[{<<"A">>, <<"foo">>}]},
{[{<<"B">>, <<"foo">>}]}
]
}]},
Normalized = normalize(Selector),
?assertEqual(false, has_required_fields(Normalized, RequiredFields)).

has_required_fields_or_test() ->
RequiredFields = [<<"A">>],
Selector = {[{<<"$or">>,
[
{[{<<"A">>, <<"foo">>}]},
{[{<<"B">>, <<"foo">>}]}
]
}]},
Normalized = normalize(Selector),
?assertEqual(false, has_required_fields(Normalized, RequiredFields)).

-endif.
6 changes: 3 additions & 3 deletions src/mango/test/03-operator-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ def test_all(self):
"manager": True,
"favorites": {"$all": ["Lisp", "Python"]}
})
self.assertEqual(len(docs), 3)
user_ids = [2,12,9]
self.assertEqual(len(docs), 4)
user_ids = [2,12,9,14]
self.assertUserIds(user_ids, docs)

def test_all_non_array(self):
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_in_operator_array(self):
"manager": True,
"favorites": {"$in": ["Ruby", "Python"]}
})
self.assertUserIds([2,6,7,9,11,12], docs)
self.assertUserIds([2,6,7,9,11,12,14], docs)

def test_nin_operator_array(self):
docs = self.db.find({
Expand Down
48 changes: 3 additions & 45 deletions src/mango/test/05-index-selection-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
class IndexSelectionTests:

def test_basic(self):
resp = self.db.find({"age": 123}, explain=True)
resp = self.db.find({"name.last": "A last name"}, explain=True)
self.assertEqual(resp["index"]["type"], "json")

def test_with_and(self):
Expand Down Expand Up @@ -63,48 +63,6 @@ def test_invalid_use_index(self):
else:
raise AssertionError("bad find")

def test_uses_index_when_no_range_or_equals(self):
# index on ["manager"] should be valid because
# selector requires "manager" to exist. The
# selector doesn't narrow the keyrange so it's
# a full index scan
selector = {
"manager": {"$exists": True}
}
docs = self.db.find(selector)
self.assertEqual(len(docs), 14)

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")

def test_reject_use_index_sort_order(self):
# index on ["company","manager"] which should not be valid
ddocid = "_design/a0c425a60cf3c3c09e3c537c9ef20059dcef9198"
selector = {
"company": {"$gt": None},
"manager": {"$gt": None}
}
try:
self.db.find(selector, use_index=ddocid, sort=[{"manager":"desc"}])
except Exception as e:
self.assertEqual(e.response.status_code, 400)
else:
raise AssertionError("did not reject bad use_index")

# This doc will not be saved given the new ddoc validation code
# in couch_mrview
def test_manual_bad_view_idx01(self):
Expand Down Expand Up @@ -159,7 +117,7 @@ def test_uses_all_docs_when_fields_do_not_match_selector(self):
self.assertEqual(len(docs), 1)
self.assertEqual(docs[0]["company"], "Pharmex")
self.assertNotIn("manager", docs[0])

resp_explain = self.db.find(selector, explain=True)

self.assertEqual(resp_explain["index"]["type"], "special")
Expand Down Expand Up @@ -211,7 +169,7 @@ def test_with_or(self):
]
}, explain=True)
self.assertEqual(resp["index"]["type"], "text")

def test_manual_bad_text_idx(self):
design_doc = {
"_id": "_design/bad_text_index",
Expand Down
1 change: 1 addition & 0 deletions src/mango/test/user_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ def add_text_indexes(db, kwargs):
},
"company": "Pharmex",
"email": "[email protected]",
"manager": True,
"favorites": [
"Erlang",
"Python",
Expand Down

0 comments on commit beb4f63

Please sign in to comment.