PYTHON-2875 Require hint with min/max queries (mongodb#712)

Author: ShaneHarvey

doc/changelog.rst

@@ -100,6 +100,8 @@ Breaking Changes in 4.0
 - Removed :class:`bson.binary.UUIDLegacy`.
 - The "tls" install extra is no longer necessary or supported and will be
  ignored by pip.
+- The ``hint`` option is now required when using ``min`` or ``max`` queries
+ with :meth:`~pymongo.collection.Collection.find`.
 - ``name`` is now a required argument for the :class:`pymongo.driver_info.DriverInfo` class.
 
 Notable improvements

doc/migrate-to-pymongo4.rst

@@ -591,6 +591,19 @@ can be changed to this::
  show_record_id=False,
  )
 
+The hint parameter is required with min/max
+...........................................
+
+The ``hint`` option is now required when using ``min`` or ``max`` queries
+with :meth:`~pymongo.collection.Collection.find` to ensure the query utilizes
+the correct index. For example, code like this::
+
+ cursor = coll.find({}, min={'x', min_value})
+
+can be changed to this::
+
+ cursor = coll.find({}, min={'x', min_value}, hint=[('x', ASCENDING)])
+
 SONManipulator is removed
 -------------------------
 

pymongo/cursor.py

@@ -1060,11 +1060,9 @@ def _refresh(self):
 
  if self.__id is None: # Query
  if (self.__min or self.__max) and not self.__hint:
- warnings.warn("using a min/max query operator without "
- "specifying a Cursor.hint is deprecated. A "
- "hint will be required when using min/max in "
- "PyMongo 4.0",
- DeprecationWarning, stacklevel=3)
+ raise InvalidOperation(
+ "Passing a 'hint' is required when using the min/max query"
+ " option to ensure the query utilizes the correct index")
  q = self._query_class(self.__query_flags,
  self.__collection.database.name,
  self.__collection.name,

test/test_collection.py

@@ -1932,9 +1932,8 @@ def test_min_query(self):
  self.db.test.insert_many([{"x": 1}, {"x": 2}])
  self.db.test.create_index("x")
 
- cursor = self.db.test.find({"$min": {"x": 2}, "$query": {}})
- if client_context.requires_hint_with_min_max_queries:
- cursor = cursor.hint("x_1")
+ cursor = self.db.test.find({"$min": {"x": 2}, "$query": {}},
+ hint="x_1")
 
  docs = list(cursor)
  self.assertEqual(1, len(docs))

test/test_cursor.py

@@ -21,7 +21,6 @@
 import sys
 import time
 import threading
-import warnings
 
 sys.path[0:0] = [""]
 
@@ -450,7 +449,6 @@ def test_limit(self):
  break
  self.assertRaises(InvalidOperation, a.limit, 5)
 
- @ignore_deprecations # Ignore max without hint.
  def test_max(self):
  db = self.db
  db.test.drop()
@@ -460,10 +458,7 @@ def test_max(self):
  db.test.insert_many([{"j": j, "k": j} for j in range(10)])
 
  def find(max_spec, expected_index):
- cursor = db.test.find().max(max_spec)
- if client_context.requires_hint_with_min_max_queries:
- cursor = cursor.hint(expected_index)
- return cursor
+ return db.test.find().max(max_spec).hint(expected_index)
 
  cursor = find([("j", 3)], j_index)
  self.assertEqual(len(list(cursor)), 3)
@@ -489,7 +484,6 @@ def find(max_spec, expected_index):
  self.assertRaises(TypeError, db.test.find().max, 10)
  self.assertRaises(TypeError, db.test.find().max, {"j": 10})
 
- @ignore_deprecations # Ignore min without hint.
  def test_min(self):
  db = self.db
  db.test.drop()
@@ -499,10 +493,7 @@ def test_min(self):
  db.test.insert_many([{"j": j, "k": j} for j in range(10)])
 
  def find(min_spec, expected_index):
- cursor = db.test.find().min(min_spec)
- if client_context.requires_hint_with_min_max_queries:
- cursor = cursor.hint(expected_index)
- return cursor
+ return db.test.find().min(min_spec).hint(expected_index)
 
  cursor = find([("j", 3)], j_index)
  self.assertEqual(len(list(cursor)), 7)
@@ -528,23 +519,15 @@ def find(min_spec, expected_index):
  self.assertRaises(TypeError, db.test.find().min, 10)
  self.assertRaises(TypeError, db.test.find().min, {"j": 10})
 
- @client_context.require_version_max(4, 1, -1)
  def test_min_max_without_hint(self):
  coll = self.db.test
  j_index = [("j", ASCENDING)]
  coll.create_index(j_index)
 
- with warnings.catch_warnings(record=True) as warns:
- warnings.simplefilter("default", DeprecationWarning)
- list(coll.find().min([("j", 3)]))
- self.assertIn('using a min/max query operator', str(warns[0]))
- # Ensure the warning is raised with the proper stack level.
- del warns[:]
+ with self.assertRaises(InvalidOperation):
  list(coll.find().min([("j", 3)]))
- self.assertIn('using a min/max query operator', str(warns[0]))
- del warns[:]
+ with self.assertRaises(InvalidOperation):
  list(coll.find().max([("j", 3)]))
- self.assertIn('using a min/max query operator', str(warns[0]))
 
  def test_batch_size(self):
  db = self.db