Fix mango json index selection

src/mango/src/mango_idx_view.erl

@@ -114,11 +114,12 @@ columns(Idx) ->
 
 
 is_usable(Idx, 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).
+    % 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).
 
 
 is_text_search({[]}) ->

src/mango/src/mango_selector.erl

@@ -15,7 +15,8 @@
 
 -export([
     normalize/1,
-    match/2
+    match/2,
+    has_required_fields/2
 ]).
 
 
@@ -566,3 +567,110 @@ 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.

src/mango/test/03-operator-test.py

@@ -26,8 +26,8 @@ def test_all(self):
                 "manager": True,
                 "favorites": {"$all": ["Lisp", "Python"]}
             })
-        self.assertEqual(len(docs), 4)
-        user_ids = [2,12,9,14]
+        self.assertEqual(len(docs), 3)
+        user_ids = [2,12,9]
         self.assertUserIds(user_ids, docs)
 
     def test_all_non_array(self):
@@ -124,7 +124,7 @@ def test_in_operator_array(self):
                 "manager": True,
                 "favorites": {"$in": ["Ruby", "Python"]}
             })
-        self.assertUserIds([2,6,7,9,11,12,14], docs)
+        self.assertUserIds([2,6,7,9,11,12], docs)
 
     def test_nin_operator_array(self):
         docs = self.db.find({

src/mango/test/05-index-selection-test.py

@@ -23,7 +23,7 @@ def setUpClass(klass):
             user_docs.add_text_indexes(klass.db, {})
 
     def test_basic(self):
-        resp = self.db.find({"name.last": "A last name"}, explain=True)
+        resp = self.db.find({"age": 123}, explain=True)
         self.assertEqual(resp["index"]["type"], "json")
 
     def test_with_and(self):
@@ -92,6 +92,78 @@ def test_invalid_use_index(self):
         else:
             raise AssertionError("bad find")
 
+    def test_uses_all_docs_when_fields_do_not_match_selector(self):
+        # index exists on ["company", "manager"] but not ["company"]
+        # so we should fall back to all docs (so we include docs
+        # with no "manager" field)
+        selector = {
+            "company": "Pharmex"
+        }
+        docs = self.db.find(selector)
+        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")
+
+    def test_uses_all_docs_when_selector_doesnt_require_fields_to_exist(self):
+        # as in test above, use a selector that doesn't overlap with the index
+        # due to an explicit exists clause
+        selector = {
+            "company": "Pharmex",
+            "manager": {"$exists": False}
+        }
+        docs = self.db.find(selector)
+        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")
+
+    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, 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, 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):

src/mango/test/user_docs.py

@@ -493,7 +493,6 @@ def add_text_indexes(db, kwargs):
         },
         "company": "Pharmex",
         "email": "[email protected]",
-        "manager": True,
         "favorites": [
             "Erlang",
             "Python",