Better management of the automatic "_cls" field (#2112)

* Rename BaseQuerySet._initial_query to BaseQuerySet._cls_query

This new name more accurately reflects the purpose of the dict. It is either
empty for documents that don't use inheritance or it contains a `{"_cls": ...}`
shape query. There was nothing "initial" about it per se.

* Drop read_preference as a kwarg on BaseQuerySet.__call__/filter

It was a poor design choice to offer two different ways to do the same thing:
1. `SomeDoc.objects(foo=bar, bar=baz).read_preference(...)`
2. `SomeDoc.objects(foo=bar, bar=baz, read_preference=...)`

Option 1 is good because it's immediately obvious which part defines the query
to be used and which part defines the read preference.

Option 2 is bad because you don't immediately know whether `read_preference`
is a query kwarg or a reserved keyword with some special behavior. If you
wanted to be particularly cruel, you could even write
`SomeDoc.objects(foo=bar, read_preference=..., bar=baz)`.

THIS IS A BREAKING CHANGE. From now on you have to use the
`BaseQuerySet.read_preference(...)` method.

* Add a BaseQuerySet.clear_cls_query method + get rid of the class_check kwarg

This is similar to what the previous commit did to read preference except that
in this case we were still missing a `BaseQuerySet` method for clearing the
`_cls` query.

Now, instead of the undocumented, untested, and confusing interface:
    `ParentDoc.objects(foo=bar, bar=baz, class_check=False)`
We do:
    `ParentDoc.objects(foo=bar, bar=baz).clear_cls_query()`

docs/changelog.rst

@@ -6,6 +6,10 @@ Changelog
 Development
 ===========
 - (Fill this out as you fix issues and develop your features).
+- BREAKING CHANGE: `class_check` and `read_preference` keyword arguments are no longer available when filtering a `QuerySet`. #2112
+  - Instead of `Doc.objects(foo=bar, read_preference=...)` use `Doc.objects(foo=bar).read_preference(...)`.
+  - Instead of `Doc.objects(foo=bar, class_check=False)` use `Doc.objects(foo=bar).clear_cls_query(...)`.
+  - This change also renames the private `QuerySet._initial_query` attribute to `_cls_query`.
 - BREAKING CHANGE: Removed the deprecated `format` param from `QuerySet.explain` #2113
 - BREAKING CHANGE: Renamed `MongoEngineConnectionError` to `ConnectionFailure` #2111
   - If you catch/use `MongoEngineConnectionError` in your code, you'll have to rename it.

mongoengine/queryset/base.py

@@ -53,13 +53,12 @@ class BaseQuerySet(object):
         self._collection_obj = collection
         self._mongo_query = None
         self._query_obj = Q()
-        self._initial_query = {}
+        self._cls_query = {}
         self._where_clause = None
         self._loaded_fields = QueryFieldList()
         self._ordering = None
         self._snapshot = False
         self._timeout = True
-        self._class_check = True
         self._slave_okay = False
         self._read_preference = None
         self._iter = False
@@ -72,9 +71,9 @@ class BaseQuerySet(object):
         # subclasses of the class being used
         if document._meta.get("allow_inheritance") is True:
             if len(self._document._subclasses) == 1:
-                self._initial_query = {"_cls": self._document._subclasses[0]}
+                self._cls_query = {"_cls": self._document._subclasses[0]}
             else:
-                self._initial_query = {"_cls": {"$in": self._document._subclasses}}
+                self._cls_query = {"_cls": {"$in": self._document._subclasses}}
             self._loaded_fields = QueryFieldList(always_include=["_cls"])
 
         self._cursor_obj = None
@@ -86,23 +85,19 @@ class BaseQuerySet(object):
         self._max_time_ms = None
         self._comment = None
 
-    def __call__(self, q_obj=None, class_check=True, read_preference=None, **query):
+    def __call__(self, q_obj=None, **query):
         """Filter the selected documents by calling the
         :class:`~mongoengine.queryset.QuerySet` with a query.
 
         :param q_obj: a :class:`~mongoengine.queryset.Q` object to be used in
             the query; the :class:`~mongoengine.queryset.QuerySet` is filtered
             multiple times with different :class:`~mongoengine.queryset.Q`
-            objects, only the last one will be used
+            objects, only the last one will be used.
-        :param class_check: If set to False bypass class name check when
+        :param query: Django-style query keyword arguments.
-            querying collection
-        :param read_preference: if set, overrides connection-level
-            read_preference from `ReplicaSetConnection`.
-        :param query: Django-style query keyword arguments
         """
         query = Q(**query)
         if q_obj:
-            # make sure proper query object is passed
+            # Make sure proper query object is passed.
             if not isinstance(q_obj, QNode):
                 msg = (
                     "Not a query object: %s. "
@@ -111,16 +106,10 @@ class BaseQuerySet(object):
                 raise InvalidQueryError(msg)
             query &= q_obj
 
-        if read_preference is None:
+        queryset = self.clone()
-            queryset = self.clone()
-        else:
-            # Use the clone provided when setting read_preference
-            queryset = self.read_preference(read_preference)
-
         queryset._query_obj &= query
         queryset._mongo_query = None
         queryset._cursor_obj = None
-        queryset._class_check = class_check
 
         return queryset
 
@@ -222,8 +211,7 @@ class BaseQuerySet(object):
         return self.__call__()
 
     def filter(self, *q_objs, **query):
-        """An alias of :meth:`~mongoengine.queryset.QuerySet.__call__`
+        """An alias of :meth:`~mongoengine.queryset.QuerySet.__call__`"""
-        """
         return self.__call__(*q_objs, **query)
 
     def search_text(self, text, language=None):
@@ -743,7 +731,7 @@ class BaseQuerySet(object):
         Do NOT return any inherited documents.
         """
         if self._document._meta.get("allow_inheritance") is True:
-            self._initial_query = {"_cls": self._document._class_name}
+            self._cls_query = {"_cls": self._document._class_name}
 
         return self
 
@@ -777,7 +765,7 @@ class BaseQuerySet(object):
 
         copy_props = (
             "_mongo_query",
-            "_initial_query",
+            "_cls_query",
             "_none",
             "_query_obj",
             "_where_clause",
@@ -785,7 +773,6 @@ class BaseQuerySet(object):
             "_ordering",
             "_snapshot",
             "_timeout",
-            "_class_check",
             "_slave_okay",
             "_read_preference",
             "_iter",
@@ -1100,6 +1087,20 @@ class BaseQuerySet(object):
 
         return queryset
 
+    def clear_cls_query(self):
+        """Clear the default "_cls" query.
+
+        By default, all queries generated for documents that allow inheritance
+        include an extra "_cls" clause. In most cases this is desirable, but
+        sometimes you might achieve better performance if you clear that
+        default query.
+
+        Scan the code for `_cls_query` to get more details.
+        """
+        queryset = self.clone()
+        queryset._cls_query = {}
+        return queryset
+
     def comment(self, text):
         """Add a comment to the query.
 
@@ -1651,13 +1652,11 @@ class BaseQuerySet(object):
     def _query(self):
         if self._mongo_query is None:
             self._mongo_query = self._query_obj.to_query(self._document)
-            if self._class_check and self._initial_query:
+            if self._cls_query:
                 if "_cls" in self._mongo_query:
-                    self._mongo_query = {
+                    self._mongo_query = {"$and": [self._cls_query, self._mongo_query]}
-                        "$and": [self._initial_query, self._mongo_query]
-                    }
                 else:
-                    self._mongo_query.update(self._initial_query)
+                    self._mongo_query.update(self._cls_query)
         return self._mongo_query
 
     @property

tests/queryset/queryset.py

@@ -4588,6 +4588,44 @@ class QuerySetTest(unittest.TestCase):
         doc.save()
         self.assertEqual(MyDoc.objects.only("test__47").get().test["47"], 1)
 
+    def test_clear_cls_query(self):
+        class Parent(Document):
+            name = StringField()
+            meta = {"allow_inheritance": True}
+
+        class Child(Parent):
+            age = IntField()
+
+        Parent.drop_collection()
+
+        # Default query includes the "_cls" check.
+        self.assertEqual(
+            Parent.objects._query, {"_cls": {"$in": ("Parent", "Parent.Child")}}
+        )
+
+        # Clearing the "_cls" query should work.
+        self.assertEqual(Parent.objects.clear_cls_query()._query, {})
+
+        # Clearing the "_cls" query should not persist across queryset instances.
+        self.assertEqual(
+            Parent.objects._query, {"_cls": {"$in": ("Parent", "Parent.Child")}}
+        )
+
+        # The rest of the query should not be cleared.
+        self.assertEqual(
+            Parent.objects.filter(name="xyz").clear_cls_query()._query, {"name": "xyz"}
+        )
+
+        Parent.objects.create(name="foo")
+        Child.objects.create(name="bar", age=1)
+        self.assertEqual(Parent.objects.clear_cls_query().count(), 2)
+        self.assertEqual(Parent.objects.count(), 2)
+        self.assertEqual(Child.objects().count(), 1)
+
+        # XXX This isn't really how you'd want to use `clear_cls_query()`, but
+        # it's a decent test to validate its behavior nonetheless.
+        self.assertEqual(Child.objects.clear_cls_query().count(), 2)
+
     def test_read_preference(self):
         class Bar(Document):
             txt = StringField()
@@ -4595,40 +4633,35 @@ class QuerySetTest(unittest.TestCase):
             meta = {"indexes": ["txt"]}
 
         Bar.drop_collection()
-        bars = list(Bar.objects(read_preference=ReadPreference.PRIMARY))
+        bar = Bar.objects.create(txt="xyz")
-        self.assertEqual([], bars)
 
-        self.assertRaises(TypeError, Bar.objects, read_preference="Primary")
+        bars = list(Bar.objects.read_preference(ReadPreference.PRIMARY))
+        self.assertEqual(bars, [bar])
 
-        # read_preference as a kwarg
-        bars = Bar.objects(read_preference=ReadPreference.SECONDARY_PREFERRED)
-        self.assertEqual(bars._read_preference, ReadPreference.SECONDARY_PREFERRED)
-        self.assertEqual(
-            bars._cursor._Cursor__read_preference, ReadPreference.SECONDARY_PREFERRED
-        )
-
-        # read_preference as a query set method
         bars = Bar.objects.read_preference(ReadPreference.SECONDARY_PREFERRED)
         self.assertEqual(bars._read_preference, ReadPreference.SECONDARY_PREFERRED)
         self.assertEqual(
             bars._cursor._Cursor__read_preference, ReadPreference.SECONDARY_PREFERRED
         )
 
-        # read_preference after skip
+        # Make sure that `.read_preference(...)` does accept string values.
+        self.assertRaises(TypeError, Bar.objects.read_preference, "Primary")
+
+        # Make sure read preference is respected after a `.skip(...)`.
         bars = Bar.objects.skip(1).read_preference(ReadPreference.SECONDARY_PREFERRED)
         self.assertEqual(bars._read_preference, ReadPreference.SECONDARY_PREFERRED)
         self.assertEqual(
             bars._cursor._Cursor__read_preference, ReadPreference.SECONDARY_PREFERRED
         )
 
-        # read_preference after limit
+        # Make sure read preference is respected after a `.limit(...)`.
         bars = Bar.objects.limit(1).read_preference(ReadPreference.SECONDARY_PREFERRED)
         self.assertEqual(bars._read_preference, ReadPreference.SECONDARY_PREFERRED)
         self.assertEqual(
             bars._cursor._Cursor__read_preference, ReadPreference.SECONDARY_PREFERRED
         )
 
-        # read_preference after order_by
+        # Make sure read preference is respected after an `.order_by(...)`.
         bars = Bar.objects.order_by("txt").read_preference(
             ReadPreference.SECONDARY_PREFERRED
         )
@@ -4637,7 +4670,7 @@ class QuerySetTest(unittest.TestCase):
             bars._cursor._Cursor__read_preference, ReadPreference.SECONDARY_PREFERRED
         )
 
-        # read_preference after hint
+        # Make sure read preference is respected after a `.hint(...)`.
         bars = Bar.objects.hint([("txt", 1)]).read_preference(
             ReadPreference.SECONDARY_PREFERRED
         )