From 079d31e698fa08dd92e2bc4f3fe9b4817a214419 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Wed, 19 Feb 2025 03:04:16 -0500 Subject: [PATCH] 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. --- django/db/models/query_utils.py | 41 ++++++++++++++++++++- docs/releases/5.2.5.txt | 4 +++ tests/constraints/tests.py | 19 +++++++++- tests/queries/test_q.py | 64 +++++++++++++++++++++++++++++++-- 4 files changed, 124 insertions(+), 4 deletions(-) diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index 5da3d81672..c383b80640 100644 --- a/django/db/models/query_utils.py +++ b/django/db/models/query_utils.py @@ -13,7 +13,7 @@ from collections import namedtuple from contextlib import nullcontext from django.core.exceptions import FieldError -from django.db import DEFAULT_DB_ALIAS, DatabaseError, connections, transaction +from django.db import DEFAULT_DB_ALIAS, DatabaseError, connections, models, transaction from django.db.models.constants import LOOKUP_SEP from django.utils import tree from django.utils.functional import cached_property @@ -99,6 +99,45 @@ class Q(tree.Node): query.promote_joins(joins) return clause + def replace_expressions(self, replacements): + if not replacements: + return self + clone = self.create(connector=self.connector, negated=self.negated) + for child in self.children: + child_replacement = child + if isinstance(child, tuple): + lhs, rhs = child + if LOOKUP_SEP in lhs: + path, lookup = lhs.rsplit(LOOKUP_SEP, 1) + else: + path = lhs + lookup = None + field = models.F(path) + if ( + field_replacement := field.replace_expressions(replacements) + ) is not field: + # Handle the implicit __exact case by falling back to an + # extra transform when get_lookup returns no match for the + # last component of the path. + if lookup is None: + lookup = "exact" + if (lookup_class := field_replacement.get_lookup(lookup)) is None: + if ( + transform_class := field_replacement.get_transform(lookup) + ) is not None: + field_replacement = transform_class(field_replacement) + lookup = "exact" + lookup_class = field_replacement.get_lookup(lookup) + if rhs is None and lookup == "exact": + lookup_class = field_replacement.get_lookup("isnull") + rhs = True + if lookup_class is not None: + child_replacement = lookup_class(field_replacement, rhs) + else: + child_replacement = child.replace_expressions(replacements) + clone.children.append(child_replacement) + return clone + def flatten(self): """ Recursively yield this Q object and all subexpressions, in depth-first diff --git a/docs/releases/5.2.5.txt b/docs/releases/5.2.5.txt index 7708563857..bdb78d0c63 100644 --- a/docs/releases/5.2.5.txt +++ b/docs/releases/5.2.5.txt @@ -15,3 +15,7 @@ Bugfixes * Fixed a crash in Django 5.2 when filtering against a composite primary key using a tuple containing expressions (:ticket:`36522`). + +* Fixed a crash in Django 5.2 when validating a model that uses + ``GeneratedField`` or constraints composed of ``Q`` and ``Case`` lookups + (:ticket:`36518`). diff --git a/tests/constraints/tests.py b/tests/constraints/tests.py index bff8de8566..f988121528 100644 --- a/tests/constraints/tests.py +++ b/tests/constraints/tests.py @@ -3,7 +3,7 @@ from unittest import mock from django.core.exceptions import ValidationError from django.db import IntegrityError, connection, models -from django.db.models import F +from django.db.models import Case, F, When from django.db.models.constraints import BaseConstraint, UniqueConstraint from django.db.models.functions import Abs, Lower, Sqrt, Upper from django.db.transaction import atomic @@ -1064,6 +1064,23 @@ class UniqueConstraintTests(TestCase): UniqueConstraintProduct(updated=updated_date + timedelta(days=1)), ) + def test_validate_case_when(self): + UniqueConstraintProduct.objects.create(name="p1") + constraint = models.UniqueConstraint( + Case(When(color__isnull=True, then=F("name"))), + name="name_without_color_uniq", + ) + msg = "Constraint “name_without_color_uniq” is violated." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name="p1"), + ) + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name="p1", color="green"), + ) + def test_validate_ordered_expression(self): constraint = models.UniqueConstraint( Lower("name").desc(), name="name_lower_uniq_desc" diff --git a/tests/queries/test_q.py b/tests/queries/test_q.py index f37d7becac..1a62aca061 100644 --- a/tests/queries/test_q.py +++ b/tests/queries/test_q.py @@ -1,3 +1,5 @@ +from datetime import datetime + from django.core.exceptions import FieldError from django.db import connection from django.db.models import ( @@ -10,8 +12,13 @@ from django.db.models import ( Value, ) from django.db.models.expressions import NegatedExpression, RawSQL -from django.db.models.functions import Lower -from django.db.models.lookups import Exact, IsNull +from django.db.models.functions import ExtractDay, Lower, TruncDate +from django.db.models.lookups import ( + Exact, + IntegerFieldExact, + IntegerLessThanOrEqual, + IsNull, +) from django.db.models.sql.where import NothingNode from django.test import SimpleTestCase, TestCase @@ -292,6 +299,59 @@ class QTests(SimpleTestCase): expected_base_fields, ) + def test_replace_expressions(self): + replacements = {F("timestamp"): Value(None)} + self.assertEqual( + Q(timestamp__date__day=25).replace_expressions(replacements), + Q(timestamp__date__day=25), + ) + replacements = {F("timestamp"): Value(datetime(2025, 10, 23))} + self.assertEqual( + Q(timestamp__date__day=13).replace_expressions(replacements), + Q( + IntegerFieldExact( + ExtractDay(TruncDate(Value(datetime(2025, 10, 23)))), + 13, + ) + ), + ) + self.assertEqual( + Q(timestamp__date__day__lte=25).replace_expressions(replacements), + Q( + IntegerLessThanOrEqual( + ExtractDay(TruncDate(Value(datetime(2025, 10, 23)))), + 25, + ) + ), + ) + self.assertEqual( + ( + Q(Q(timestamp__date__day__lte=25), timestamp__date__day=13) + ).replace_expressions(replacements), + ( + Q( + Q( + IntegerLessThanOrEqual( + ExtractDay(TruncDate(Value(datetime(2025, 10, 23)))), + 25, + ) + ), + IntegerFieldExact( + ExtractDay(TruncDate(Value(datetime(2025, 10, 23)))), + 13, + ), + ) + ), + ) + self.assertEqual( + Q(timestamp=None).replace_expressions(replacements), + Q(IsNull(Value(datetime(2025, 10, 23)), True)), + ) + self.assertEqual( + Q(timestamp__date__day__invalid=25).replace_expressions(replacements), + Q(timestamp__date__day__invalid=25), + ) + class QCheckTests(TestCase): def test_basic(self):