Opened 4 days ago
Last modified 2 days ago
#36580 new Bug
Model validation of constraints fails if condition's Q object references ForeignObject
Reported by: | Jacob Walls | Owned by: | JaeHyuckSa |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Similar to #36433, just for ForeignObject
instead of ForeignKey
.
With this adjusted test model and corresponding adjustment to unrelated test, test_full_clean_update
passes on stable/5.2.x (the constraint is purposefully not very imaginative, can polish in PR review):
-
tests/composite_pk/models/tenant.py
diff --git a/tests/composite_pk/models/tenant.py b/tests/composite_pk/models/tenant.py index 65eb0feae8..954a5519f8 100644
a b class Comment(models.Model): 48 48 text = models.TextField(default="", blank=True) 49 49 integer = models.IntegerField(default=0) 50 50 51 class Meta: 52 constraints = [ 53 models.CheckConstraint( 54 condition=models.Q(user__isnull=False), 55 name="user_not_null", 56 ), 57 ] 58 51 59 52 60 class Post(models.Model): 53 61 pk = models.CompositePrimaryKey("tenant_id", "id") -
tests/composite_pk/tests.py
diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py index c4a8e6ca8c..cade405dee 100644
a b class CompositePKTests(TestCase): 205 205 self.assertEqual(user.email, self.user.email) 206 206 207 207 def test_select_related(self): 208 Comment.objects.create(tenant=self.tenant, id=2) 208 user2 = User.objects.create( 209 tenant=self.tenant, 210 id=2, 211 email="user0002@example.com", 212 ) 213 Comment.objects.create(tenant=self.tenant, id=2, user=user2) 209 214 with self.assertNumQueries(1): 210 215 comments = list(Comment.objects.select_related("user").order_by("pk")) 211 216 self.assertEqual(len(comments), 2) 212 217 self.assertEqual(comments[0].user, self.user) 213 self.assert IsNone(comments[1].user)218 self.assertEqual(comments[1].user, user2) 214 219 215 220 def test_model_forms(self): 216 221 fields = ["tenant", "id", "user_id", "text", "integer"]
... but fails on main:
====================================================================== ERROR: test_full_clean_update (composite_pk.test_models.CompositePKModelsTests.test_full_clean_update) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/unittest/case.py", line 58, in testPartExecutor yield File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/unittest/case.py", line 651, in run self._callTestMethod(testMethod) File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/unittest/case.py", line 606, in _callTestMethod if method() is not None: ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/tests/composite_pk/test_models.py", line 123, in test_full_clean_update self.comment_1.full_clean() ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/base.py", line 1638, in full_clean self.validate_constraints(exclude=exclude) ^^^^^^^ File "/Users/jwalls/django/django/db/models/base.py", line 1586, in validate_constraints constraint.validate(model_class, self, exclude=exclude, using=using) ^^^ File "/Users/jwalls/django/django/db/models/constraints.py", line 212, in validate if not Q(self.condition).check(against, using=using): ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/query_utils.py", line 176, in check query.add_q(Q(Coalesce(self, True, output_field=BooleanField()))) ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1670, in add_q clause, _ = self._add_q(q_object, can_reuse) ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1702, in _add_q child_clause, needed_inner = self.build_filter( ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1541, in build_filter condition = filter_expr.resolve_expression( ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/expressions.py", line 301, in resolve_expression expr.resolve_expression(query, allow_joins, reuse, summarize, for_save) ^^^^^^^ File "/Users/jwalls/django/django/db/models/query_utils.py", line 91, in resolve_expression clause, joins = query._add_q( ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1702, in _add_q child_clause, needed_inner = self.build_filter( ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1527, in build_filter return self._add_q( ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1702, in _add_q child_clause, needed_inner = self.build_filter( ^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1550, in build_filter lookups, parts, reffed_expression = self.solve_lookup_type(arg, summarize) ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1357, in solve_lookup_type _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta()) ^^^^^^^^^^^^^^^ File "/Users/jwalls/django/django/db/models/sql/query.py", line 1830, in names_to_path raise FieldError( ^^^ django.core.exceptions.FieldError: Cannot resolve keyword 'user' into field. Choices are: _check ---------------------------------------------------------------------- Ran 178 tests in 1.224s
---
There is a prior comment in ticket:36433#comment:1 about improving the error message.
Change History (7)
comment:1 by , 3 days ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:2 by , 3 days ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 2 days ago
Has patch: | set |
---|
follow-up: 5 comment:4 by , 2 days ago
Resolution: | → needsinfo |
---|---|
Status: | assigned → closed |
comment:5 by , 2 days ago
Replying to Jacob Walls:
I neglected to check a test project, there's a system check preventing you from getting into this situation that errors on migrate:
ERRORS: models.OrderLineItem: (models.E012) 'constraints' refers to the nonexistent field 'user'.So this needs a more realistic test case to find out if there's a usage of
Q()
supported on 5.2 that is actually breaking here.
Hi Jacob
Is there a way to run the system checks inside the Django source repository rather than in my own project?
follow-up: 7 comment:6 by , 2 days ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
Reopening, my constraint didn't match my models. Sorry for the noise!
Is there a way to run the system checks inside the Django source repository rather than in my own project?
I'm not sure we run system checks on test models. Not super relevant to this ticket, as I was just totally wrong in this case, but I have noticed it when working in the test suite that test models might not be 100% valid.
comment:7 by , 2 days ago
Replying to Jacob Walls:
Reopening, my constraint didn't match my models. Sorry for the noise!
Is there a way to run the system checks inside the Django source repository rather than in my own project?
I'm not sure we run system checks on test models. Not super relevant to this ticket, as I was just totally wrong in this case, but I have noticed it when working in the test suite that test models might not be 100% valid.
I wasn’t aware of that. I’ll keep it in mind in future tests. Thank you for your response.
I neglected to check a test project, there's a system check preventing you from getting into this situation that errors on migrate:
So this needs a more realistic test case to find out if there's a usage of
Q()
supported on 5.2 that is actually breaking here.