#35603 closed Bug (fixed)
"string" in F("...") hangs
Reported by: | Tim Graham | Owned by: | Tim Graham |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.1 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Prior to 94b6f101f7dc363a8e71593570b17527dbb9f77f, this test passes:
-
tests/expressions/tests.py
diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index cbb441601c..0600e04e06 100644
a b class FTests(SimpleTestCase): 1292 1292 self.assertNotEqual(f, value) 1293 1293 self.assertNotEqual(value, f) 1294 1294 1295 def test_in(self): 1296 with self.assertRaisesMessage(TypeError, "argument of type 'F' is not iterable"): 1297 "foo" in F("name") 1298 1295 1299 1296 1300 class ExpressionsTests(TestCase): 1297 1301 def test_F_reuse(self):
Afterward, it hangs indefinitely.
Change History (8)
comment:1 by , 4 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 4 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:3 by , 4 months ago
Simon, do you think it's better to return False than the old behavior of raising an exception saying that F
isn't iterable? The code where I encountered this is:
if LOOKUP_SEP in self.query.order_by: raise NotSupportedError("Ordering can't span tables.")
This naive implementation doesn't account for F
objects in ordering. If we change the behavior to return False
, my code will crash a bit later. That doesn't matter since this problem needs to be fixed eventually, but I tend to think continuing to raise an exception in F.__contains__()
would be fine since it doesn't have any meaning.
comment:4 by , 4 months ago
Tim, given we don't actually support it I think raising a TypeError
might effectively be more appropriate. I tried skimming the Python docs to understand if doing so would be against any established protocol and I couldn't find anything we'd be in violation of.
To make sure we're aligned, you were thinking of something like
-
django/db/models/expressions.py
diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index dcba973ff4..1d16781959 100644
a b def __repr__(self): 884 884 def __getitem__(self, subscript): 885 885 return Sliced(self, subscript) 886 886 887 def __contains__(self, other): 888 # Disable old-style iteration protocol inherited from implementing 889 # `__getitem__` for slicing to prevent containment checks from hanging. 890 raise TypeError(f"argument of type '{self.__class__.__name__}' is not iterable") 891 887 892 def resolve_expression( 888 893 self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False 889 894 ):
comment:6 by , 4 months ago
Owner: | changed from | to
---|
Lufafa, we appreciate your interesting in contributing code to Django but please ask before jumping on assigning tickets to yourself when an active discussion is taking place.
The issue lies in implementing
__getitem__
without implementing__contains__
, per the Python docsBasically what
"foo" in F("name")
translates to is the following old-style iteration protocolwhich explains why it hangs indefinitely.
Implementing
__contains__
resolves the issuedjango/db/models/expressions.py