full_clean crashes on model with both a CheckConstraint and a GeneratedField with a Case expression

[Olivier Dalang]

Hello !

I think I ran accross a regression when upgrading a model containing a CheckConstraint and a GeneratedField (that are unrelated) from Django 5.1 to 5.2.

Below is a simplified case that reproduces the issue.

I found is a similar ticket: https://code.djangoproject.com/ticket/34871 (affecting 4.2 already, and thus not flagged as regression, for a slightly simpler use case).

That ticket links to recent PR which could quite likely has introduced this regression: ​https://github.com/django/django/pull/19218

I'm not yet familiar with contributing to Django, and fear this is (way) above my level to fix, but please let me know if I can do anything to help fixing this, as it currently prevents us from upgrading.

Cheers !!

Olivier

# models.py

class MyModel(models.Model):
    class Meta:
        constraints = [
            models.CheckConstraint(name="age_valid", check=Q(age__lt=100)),
        ]

    age = models.IntegerField()
    is_old_enough = models.GeneratedField(
        expression=Case(
            When(
                age__gte=18,
                then=Value(True),
            ),
            default=Value(False),
        ),
        db_persist=True,
        output_field=models.BooleanField(),
    )

# tests.py

class MyTests(TestCase):
    def test_fullclean(self):
        bob = MyModel.objects.create(age=17)
        bob.full_clean()

The test succeeds on Django 5.1, but fails on 5.2.

> uv run --with django==5.2.4 manage.py test
Found 1 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
E
======================================================================
ERROR: test_fullclean (myproj.myapp.tests.MyTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/app/myproj/myapp/tests.py", line 8, in test_fullclean
    bob.full_clean()
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/base.py", line 1674, in full_clean
    self.validate_constraints(exclude=exclude)
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/base.py", line 1622, in validate_constraints
    constraint.validate(model_class, self, exclude=exclude, using=using)
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/constraints.py", line 261, in validate
    against = instance._get_field_expression_map(meta=model._meta, exclude=exclude)
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/base.py", line 1372, in _get_field_expression_map
    generated_field.expression.replace_expressions(replacements),
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/expressions.py", line 427, in replace_expressions
    [
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/expressions.py", line 428, in <listcomp>
    expr.replace_expressions(replacements) if expr else None
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/expressions.py", line 427, in replace_expressions
    [
  File "/home/cache/archive-v0/kkaSi9So5gXBgF1hYbu9X/lib/site-packages/django/db/models/expressions.py", line 428, in <listcomp>
    expr.replace_expressions(replacements) if expr else None
AttributeError: 'Q' object has no attribute 'replace_expressions'
----------------------------------------------------------------------
Ran 1 test in 0.003s

FAILED (errors=1)
Destroying test database for alias 'default'...

> uv run --with django==5.1.11 manage.py test
Found 1 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.
----------------------------------------------------------------------
Ran 1 test in 0.002s

OK
Destroying test database for alias 'default'...

[Sarah Boyce]

Thank you for the report!
Replicated, git bisected to 228128618bd895ecad235d2215f4ad4e3232595d (#35575)

[Sarah Boyce]

Tried to look into this today a bit, note that the following is a possible regression test:

tests/constraints/models.py

@@ -69 +69 @@
         required_db_features = {"supports_virtual_generated_columns"}
 
 
+class GeneratedFieldCaseWhenConstraint(models.Model):
+    age = models.IntegerField()
+    is_old_enough = models.GeneratedField(
+        expression=models.Case(
+            models.When(age__gte=18, then=models.Value(True)),
+            default=models.Value(False),
+        ),
+        db_persist=True,
+        output_field=models.BooleanField(),
+    )
+
+    class Meta:
+        required_db_features = {
+            "supports_stored_generated_columns",
+            "supports_table_check_constraints"
+        }
+        constraints = [
+            models.CheckConstraint(
+                name="age_valid", condition=models.Q(age__lt=100)
+            ),
+        ]
+
+
 class UniqueConstraintProduct(models.Model):
     name = models.CharField(max_length=255)
     color = models.CharField(max_length=32, null=True)

tests/constraints/tests.py

@@ -12 +12 @@
 from .models import (
     ChildModel,
     ChildUniqueConstraintProduct,
+    GeneratedFieldCaseWhenConstraint,
     GeneratedFieldStoredProduct,
     GeneratedFieldVirtualProduct,
     JSONFieldModel,
@@ -414 +415 @@
         constraint.validate(model, invalid_product, exclude={"price"})
         constraint.validate(model, invalid_product, exclude={"rebate"})
 
+    @skipUnlessDBFeature(
+        "supports_stored_generated_columns", "supports_table_check_constraints"
+    )
+    def test_full_clean_generated_field_case_when(self):
+        bob = GeneratedFieldCaseWhenConstraint(age=17)
+        bob.full_clean()
+        bob.save()
+        self.assertEqual(bob.is_old_enough, False)
+
     def test_database_default(self):
         models.CheckConstraint(

This test fails on main but passes once the current fix for #34871 (current PR ​https://github.com/django/django/pull/19190) is applied

[Simon Charette]

Thanks for the ping and test Sarah, here's my analysis of the situation.

We've known that validation of constraints including unresolved lookups (Q, Case) is since broken since 4.2 (#34871).

By fixing #35575 and requiring that GeneratedField expressions are replaced with the validated against instance values we triggered the same problem for GeneratedField making use of unresolved lookups hence why the patch for #34871 happens to resolve this issue.

The interesting part is that since we have constraint asks for all fields replacements via _get_field_expression_map (except for the ones part of excludes) even if only a few ones are required by the constraint itself the problem can now be triggered even if a generated field making use of unresolved lookup is not referenced by the constraint. For example, in the reported case the age_valid constraint doesn't refer to the is_old_enough field.

Looking back at ​the proposed patch it seems like it would be adequate to address this issue even if it duplicates sql.Query.build_filter logic as we likely want to avoid also backporting a refactor. It would keep the unnecessary work in performed by _get_field_expression_map though.

The only alternative I see is likely more invasive as it would require us to flip the logic around by having constraint introspect their respective expressions (e.g. condition, constraint) and only request that fields of interests are considered by _get_field_expression_map. This seems like a worthy optimization that would address this issue but it wouldn't address the underlying cause of #34871 and is likely risky as a backport.

[Sarah Boyce]

Looking back at ​the proposed patch it seems like it would be adequate to address this issue even if it duplicates sql.Query.build_filter logic as we likely want to avoid also backporting a refactor. It would keep the unnecessary work in performed by _get_field_expression_map though.

I agree the solution for #34871 is backportable and likely preferable to a deeper refactor, would you mind adding a test to that patch relating to this ticket and a release note?

[Simon Charette]

would you mind adding a test to that patch relating to this ticket and a release note?

yeah will do!

[Sarah Boyce <42296566+sarahboyce@…>]

In 079d31e6:

Fixed #34871, #36518 -- Implemented unresolved lookups expression replacement.

This allows the proper resolving of lookups when performing constraint
validation involving Q and Case objects.

Thanks Andrew Roberts for the report and Sarah for the tests and review.

[Sarah Boyce <42296566+sarahboyce@…>]

In b3bb723:

[5.2.x] Fixed #34871, #36518 -- Implemented unresolved lookups expression replacement.

This allows the proper resolving of lookups when performing constraint
validation involving Q and Case objects.

Thanks Andrew Roberts for the report and Sarah for the tests and review.

Backport of 079d31e698fa08dd92e2bc4f3fe9b4817a214419 from main.