id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
36415	Add a public API to unregister model field lookups	Tim Graham	Tim Graham	"Motivated by the ongoing work to build a MongoDB backend, I'm interested in writing code like this:
`EmbeddedModelField._unregister_lookup(GreaterThanOrEqual)`
(a lot of the built-in lookups aren't applicable to `EmbeddedModelField`... I think we only want `exact`.)

As far as I can tell, `Field._unregister_lookup()` isn't a public API [https://github.com/django/django/blob/e44e8327d3d88d86895735c0e427102063ff5b55/django/db/models/query_utils.py#L339-L361due to concerns about thread-safety].

Simon Charette provided this guidance:

As far as I know we don't run tests in a multi-threaded environment (it's multi-process based) so I'm not sure what this comment is referring to.

If you implement this logic I suspect you'll need to use a notion of sentinel / tombstone to denote unregistration to work like you intend as `GreaterThanOrEqual` is registered for `Field` as well.

The following code might explain why the current situation is broken:

{{{#!python
from django.db.models import Field
from django.db.models.lookups import GreaterThanOrEqual

class EmbeddedModelField(Field):
   ...

>>> EmbeddedModelField._unregister_lookup(GreaterThanOrEqual)
>>> assert ""gte"" not in EmbeddedModelField.get_lookups()
>>> assert ""gte"" in Field.get_lookups()
}}}
I think that's what the ""thread-safe"" mention was referring to, it was more along the lines that `_unregister_lookup()` is broken if it doesn't have a corresponding `register_lookup()` call to restore its state because the unregistration logic doesn't ensure to create a per-class override prior to deletion.

A potential solution to this problem would be to avoid any form of deletion of registered lookups and use a `UnregisteredLookup` sentinel (or simply `None`) instead:
{{{#!diff
diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py
index f0ae810f47..4df1324825 100644
--- a/django/db/models/query_utils.py
+++ b/django/db/models/query_utils.py
@@ -337,13 +337,11 @@ def register_instance_lookup(self, lookup, lookup_name=None):
     register_class_lookup = classmethod(register_class_lookup)

     def _unregister_class_lookup(cls, lookup, lookup_name=None):
-        """"""
-        Remove given lookup from cls lookups. For use in tests only as it's
-        not thread-safe.
-        """"""
         if lookup_name is None:
             lookup_name = lookup.lookup_name
-        del cls.class_lookups[lookup_name]
+        if ""class_lookups"" not in cls.__dict__:
+            cls.class_lookups = {}
+        cls.class_lookups[lookup_name] = None
         cls._clear_cached_class_lookups()

     def _unregister_instance_lookup(self, lookup, lookup_name=None):
@@ -353,7 +351,9 @@ def _unregister_instance_lookup(self, lookup, lookup_name=None):
         """"""
         if lookup_name is None:
             lookup_name = lookup.lookup_name
-        del self.instance_lookups[lookup_name]
+        if ""instance_lookups"" not in self.__dict__:
+            self.instance_lookups = {}
+        self.instance_lookups[lookup_name] = None

     _unregister_lookup = class_or_instance_method(
         _unregister_class_lookup, _unregister_instance_lookup
}}}
None is a suitable sentinel as both `get_lookup` and `get_transform` treat it as a missing entry."	New feature	assigned	Database layer (models, ORM)	dev	Normal				Accepted	0	0	0	0	0	0