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):  
    4848    text = models.TextField(default="", blank=True)
    4949    integer = models.IntegerField(default=0)
    5050
     51    class Meta:
     52        constraints = [
     53            models.CheckConstraint(
     54                condition=models.Q(user__isnull=False),
     55                name="user_not_null",
     56            ),
     57        ]
     58
    5159
    5260class Post(models.Model):
    5361    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):  
    205205            self.assertEqual(user.email, self.user.email)
    206206
    207207    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)
    209214        with self.assertNumQueries(1):
    210215            comments = list(Comment.objects.select_related("user").order_by("pk"))
    211216            self.assertEqual(len(comments), 2)
    212217            self.assertEqual(comments[0].user, self.user)
    213             self.assertIsNone(comments[1].user)
     218            self.assertEqual(comments[1].user, user2)
    214219
    215220    def test_model_forms(self):
    216221        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 JaeHyuckSa, 3 days ago

Owner: set to JaeHyuckSa
Status: newassigned

comment:2 by Sarah Boyce, 3 days ago

Triage Stage: UnreviewedAccepted

comment:3 by JaeHyuckSa, 2 days ago

Has patch: set

comment:4 by Jacob Walls, 2 days ago

Resolution: needsinfo
Status: assignedclosed

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.

Last edited 2 days ago by Jacob Walls (previous) (diff)

in reply to:  4 comment:5 by JaeHyuckSa, 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?

comment:6 by Jacob Walls, 2 days ago

Resolution: needsinfo
Status: closednew

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.

in reply to:  6 comment:7 by JaeHyuckSa, 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.

Note: See TracTickets for help on using tickets.
Back to Top