django.db.models.query_utils.RegisterLookupMixin._unregister_lookup method also should clear the cache stored for django.db.models.query_utils.RegisterLookupMixin.get_lookups

[Himanshu Balasamanta]

In current source code, in the _unregister_lookup method,
​https://github.com/django/django/blame/main/django/db/models/query_utils.py#L212,
the cache is not cleared, which should be done, as it is done in
register_lookup,
​https://github.com/django/django/blame/main/django/db/models/query_utils.py#L202.
Corresponding to this change, minor changes need to be brought in the
schema.tests.SchemaTests.test_func_unique_constraint_lookups test.

The PR generated is ​https://github.com/django/django/pull/15569

[Carlton Gibson]

Hi Himanshu. This may be right, yes — I need to have a sit-down and play with it.

Main question: Are you able to put together an example case where the wrong result arrises?

• I was looking at ​PR #6906 which added the cache clearing.
• Also noting the For use in tests only... in the _unregister_lookup docstring. So this would show up in a test inter-dependency...? 🤔

[Himanshu Balasamanta]

Replying to Carlton Gibson:

Hi Himanshu. This may be right, yes — I need to have a sit-down and play with it.

Main question: Are you able to put together an example case where the wrong result arrises?

Hi Carlton.
I have opened the PR ​​https://github.com/django/django/pull/15569, and have also modified the test that was supposed to throw error( schema.tests.SchemaTests.test_func_unique_constraint_lookups ).
There is no test that checks if the lookup stays in cache after unregistering it. In my PR, I have added an assert statement to check it in custom_lookups.tests.LookupTests.test_lookups_caching test. Running the test without clearing cache from _unregister_lookup will fail.

• I was looking at ​PR #6906 which added the cache clearing.
• Also noting the For use in tests only... in the _unregister_lookup docstring. So this would show up in a test inter-dependency...? 🤔

The cache stays between tests, so you are likely to get different results running tests independently and running all tests in a module. (PS: I faced this problem earlier running tests individually and together gave different results.)

[Himanshu Balasamanta]

Replying to Carlton Gibson:

Hi Himanshu. This may be right, yes — I need to have a sit-down and play with it.

Main question: Are you able to put together an example case where the wrong result arrises?

• I was looking at ​PR #6906 which added the cache clearing.
• Also noting the For use in tests only... in the _unregister_lookup docstring. So this would show up in a test inter-dependency...? 🤔