diff --git a/django/db/backends/base/features.py b/django/db/backends/base/features.py index ecc283ff6ba5..1e957d105ed3 100644 --- a/django/db/backends/base/features.py +++ b/django/db/backends/base/features.py @@ -393,8 +393,10 @@ class BaseDatabaseFeatures: # subqueries? supports_tuple_comparison_against_subquery = True - # Does the backend support DEFAULT as delete option? + # Does the backend support CASCADE, DEFAULT, NULL as delete options? + supports_on_delete_db_cascade = True supports_on_delete_db_default = True + supports_on_delete_db_null = True # Collation names for use by the Django test suite. test_collations = { diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 0d47366d2c03..6b90a42cf1d2 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -426,7 +426,7 @@ def replace_expressions(self, replacements): clone = self.copy() clone.set_source_expressions( [ - expr.replace_expressions(replacements) if expr else None + None if expr is None else expr.replace_expressions(replacements) for expr in source_expressions ] ) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 0293c7890991..f1bc664007dd 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -12,6 +12,7 @@ from django.db.models.constants import LOOKUP_SEP from django.db.models.deletion import ( CASCADE, + DB_CASCADE, DB_SET_DEFAULT, DB_SET_NULL, DO_NOTHING, @@ -1056,9 +1057,38 @@ def check(self, **kwargs): *self._check_unique(), ] + def _check_on_delete_db_support(self, on_delete, feature_flag, databases): + for db in databases: + if not router.allow_migrate_model(db, self.model): + continue + connection = connections[db] + if feature_flag in self.model._meta.required_db_features or getattr( + connection.features, feature_flag + ): + continue + no_db_option_name = on_delete.__name__.removeprefix("DB_") + yield checks.Error( + f"{connection.display_name} does not support a {on_delete.__name__}.", + hint=f"Change the on_delete rule to {no_db_option_name}.", + obj=self, + id="fields.E324", + ) + def _check_on_delete(self, databases): on_delete = getattr(self.remote_field, "on_delete", None) errors = [] + if on_delete == DB_CASCADE: + errors.extend( + self._check_on_delete_db_support( + on_delete, "supports_on_delete_db_cascade", databases + ) + ) + if on_delete == DB_SET_NULL: + errors.extend( + self._check_on_delete_db_support( + on_delete, "supports_on_delete_db_null", databases + ) + ) if on_delete in [DB_SET_NULL, SET_NULL] and not self.null: errors.append( checks.Error( @@ -1092,25 +1122,12 @@ def _check_on_delete(self, databases): id="fields.E322", ) ) - for db in databases: - if not router.allow_migrate_model(db, self.model): - continue - connection = connections[db] - if not ( - "supports_on_delete_db_default" - in self.model._meta.required_db_features - or connection.features.supports_on_delete_db_default - ): - errors.append( - checks.Error( - f"{connection.display_name} does not support a " - "DB_SET_DEFAULT.", - hint="Change the on_delete rule to SET_DEFAULT.", - obj=self, - id="fields.E324", - ), - ) - elif not isinstance(self.remote_field.model, str) and on_delete != DO_NOTHING: + errors.extend( + self._check_on_delete_db_support( + on_delete, "supports_on_delete_db_default", databases + ) + ) + if not isinstance(self.remote_field.model, str) and on_delete != DO_NOTHING: # Database and Python variants cannot be mixed in a chain of # model references. is_db_on_delete = isinstance(on_delete, DatabaseOnDelete) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 22ebfed2ca4e..7dfa7c61d24b 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -309,7 +309,8 @@ Related fields ``db_default`` value. * **fields.E323**: Field specifies database/Python-level on_delete variant, but referenced model uses python/database-level variant. -* **fields.E324**: ```` does not support ``DB_SET_DEFAULT``. +* **fields.E324**: ```` does not support + ````. * **fields.E330**: ``ManyToManyField``\s cannot be unique. * **fields.E331**: Field specifies a many-to-many relation through model ````, which has not been installed. diff --git a/docs/releases/5.2.9.txt b/docs/releases/5.2.9.txt index 588c278be5e9..515b233b8a99 100644 --- a/docs/releases/5.2.9.txt +++ b/docs/releases/5.2.9.txt @@ -16,3 +16,7 @@ Bugfixes * Fixed a bug in Django 5.2 on PostgreSQL where ``bulk_create()`` did not apply a field's custom query placeholders (:ticket:`36748`). + +* Fixed a regression in Django 5.2.2 that caused a crash when using aggregate + functions with an empty ``Q`` filter over a queryset with annotations + (:ticket:`36751`). diff --git a/tests/admin_utils/models.py b/tests/admin_utils/models.py index e5d2b6788722..55810f0b50a8 100644 --- a/tests/admin_utils/models.py +++ b/tests/admin_utils/models.py @@ -55,6 +55,9 @@ class DBCascade(models.Model): def __str__(self): return str(self.num) + class Meta: + required_db_features = {"supports_on_delete_db_cascade"} + class Event(models.Model): date = models.DateTimeField(auto_now_add=True) diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index ce32535c52f0..81c6f495f8ef 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -21,7 +21,7 @@ from django.contrib.auth.templatetags.auth import render_password_as_hash from django.core.validators import EMPTY_VALUES from django.db import DEFAULT_DB_ALIAS, models -from django.test import SimpleTestCase, TestCase, override_settings +from django.test import SimpleTestCase, TestCase, override_settings, skipUnlessDBFeature from django.test.utils import isolate_apps from django.utils.formats import localize from django.utils.safestring import mark_safe @@ -115,6 +115,7 @@ def test_relation_on_abstract(self): n.collect([Vehicle.objects.first()]) +@skipUnlessDBFeature("supports_on_delete_db_cascade") class DBNestedObjectsTests(NestedObjectsTests): """ Exercise NestedObjectsTests but with a model that makes use of DB_CASCADE diff --git a/tests/aggregation/test_filter_argument.py b/tests/aggregation/test_filter_argument.py index 1a17703a8640..59588d30d05e 100644 --- a/tests/aggregation/test_filter_argument.py +++ b/tests/aggregation/test_filter_argument.py @@ -88,6 +88,17 @@ def test_empty_filtered_aggregates(self): agg = Count("pk", filter=Q()) self.assertEqual(Author.objects.aggregate(count=agg)["count"], 3) + def test_empty_filtered_aggregates_with_annotation(self): + agg = Count("pk", filter=Q()) + self.assertEqual( + Author.objects.annotate( + age_annotation=F("age"), + ).aggregate( + count=agg + )["count"], + 3, + ) + def test_double_filtered_aggregates(self): agg = Sum("age", filter=Q(Q(name="test2") & ~Q(name="test"))) self.assertEqual(Author.objects.aggregate(age=agg)["age"], 60) diff --git a/tests/delete/models.py b/tests/delete/models.py index bd9caf42a7ae..d8f81827be66 100644 --- a/tests/delete/models.py +++ b/tests/delete/models.py @@ -49,17 +49,26 @@ class RelatedDbOptionParent(models.Model): p = models.ForeignKey(RelatedDbOptionGrandParent, models.DB_CASCADE, null=True) -class RelatedDbOption(models.Model): +class CascadeDbModel(models.Model): name = models.CharField(max_length=30) + db_cascade = models.ForeignKey( + RelatedDbOptionParent, models.DB_CASCADE, related_name="db_cascade_set" + ) + + class Meta: + required_db_features = {"supports_on_delete_db_cascade"} + + +class SetNullDbModel(models.Model): db_setnull = models.ForeignKey( RelatedDbOptionParent, models.DB_SET_NULL, null=True, related_name="db_setnull_set", ) - db_cascade = models.ForeignKey( - RelatedDbOptionParent, models.DB_CASCADE, related_name="db_cascade_set" - ) + + class Meta: + required_db_features = {"supports_on_delete_db_null"} class SetDefaultDbModel(models.Model): @@ -159,15 +168,6 @@ def create_a(name): return a -def create_related_db_option(name): - a = RelatedDbOption(name=name) - for name in ["db_setnull", "db_cascade"]: - r = RelatedDbOptionParent.objects.create() - setattr(a, name, r) - a.save() - return a - - class M(models.Model): m2m = models.ManyToManyField(R, related_name="m_set") m2m_through = models.ManyToManyField(R, through="MR", related_name="m_through_set") diff --git a/tests/delete/tests.py b/tests/delete/tests.py index 8d525d1e5ff3..664cdfb440f2 100644 --- a/tests/delete/tests.py +++ b/tests/delete/tests.py @@ -15,6 +15,7 @@ Avatar, B, Base, + CascadeDbModel, Child, DeleteBottom, DeleteTop, @@ -34,16 +35,15 @@ RChild, RChildChild, Referrer, - RelatedDbOption, RelatedDbOptionGrandParent, RelatedDbOptionParent, RProxy, S, SetDefaultDbModel, + SetNullDbModel, T, User, create_a, - create_related_db_option, get_default_r, ) @@ -81,10 +81,13 @@ def test_setnull(self): a = A.objects.get(pk=a.pk) self.assertIsNone(a.setnull) + @skipUnlessDBFeature("supports_on_delete_db_null") def test_db_setnull(self): - a = create_related_db_option("db_setnull") + a = SetNullDbModel.objects.create( + db_setnull=RelatedDbOptionParent.objects.create() + ) a.db_setnull.delete() - a = RelatedDbOption.objects.get(pk=a.pk) + a = SetNullDbModel.objects.get(pk=a.pk) self.assertIsNone(a.db_setnull) def test_setdefault(self): @@ -394,20 +397,21 @@ def test_bulk(self): self.assertNumQueries(5, s.delete) self.assertFalse(S.objects.exists()) + @skipUnlessDBFeature("supports_on_delete_db_cascade") def test_db_cascade(self): related_db_op = RelatedDbOptionParent.objects.create( p=RelatedDbOptionGrandParent.objects.create() ) - RelatedDbOption.objects.bulk_create( + CascadeDbModel.objects.bulk_create( [ - RelatedDbOption(db_cascade=related_db_op) + CascadeDbModel(db_cascade=related_db_op) for _ in range(2 * GET_ITERATOR_CHUNK_SIZE) ] ) with self.assertNumQueries(1): results = related_db_op.delete() self.assertEqual(results, (1, {"delete.RelatedDbOptionParent": 1})) - self.assertFalse(RelatedDbOption.objects.exists()) + self.assertFalse(CascadeDbModel.objects.exists()) self.assertFalse(RelatedDbOptionParent.objects.exists()) def test_instance_update(self): diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index 6f18321aa7b3..981d84e9e8ec 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -1571,6 +1571,24 @@ def test_get_expression_for_validation_only_one_source_expression(self): with self.assertRaisesMessage(ValueError, msg): expression.get_expression_for_validation() + def test_replace_expressions_falsey(self): + class AssignableExpression(Expression): + def __init__(self, *source_expressions): + super().__init__() + self.set_source_expressions(list(source_expressions)) + + def get_source_expressions(self): + return self.source_expressions + + def set_source_expressions(self, exprs): + self.source_expressions = exprs + + expression = AssignableExpression() + falsey = Q() + expression.set_source_expressions([falsey]) + replaced = expression.replace_expressions({"replacement": Expression()}) + self.assertEqual(replaced.get_source_expressions(), [falsey]) + class ExpressionsNumericTests(TestCase): @classmethod diff --git a/tests/inspectdb/models.py b/tests/inspectdb/models.py index fbe1df8a9531..489704879f35 100644 --- a/tests/inspectdb/models.py +++ b/tests/inspectdb/models.py @@ -178,3 +178,9 @@ class DbOnDeleteModel(models.Model): fk_set_null = models.ForeignKey( DigitsInColumnName, on_delete=models.DB_SET_NULL, null=True ) + + class Meta: + required_db_features = { + "supports_on_delete_db_cascade", + "supports_on_delete_db_null", + } diff --git a/tests/inspectdb/tests.py b/tests/inspectdb/tests.py index c16258b0eb67..8175c52e4ec9 100644 --- a/tests/inspectdb/tests.py +++ b/tests/inspectdb/tests.py @@ -301,7 +301,11 @@ def test_foreign_key_to_field(self): out.getvalue(), ) - @skipUnlessDBFeature("can_introspect_foreign_keys") + @skipUnlessDBFeature( + "can_introspect_foreign_keys", + "supports_on_delete_db_cascade", + "supports_on_delete_db_null", + ) def test_foreign_key_db_on_delete(self): out = StringIO() call_command("inspectdb", "inspectdb_dbondeletemodel", stdout=out) diff --git a/tests/introspection/models.py b/tests/introspection/models.py index 6c94f5212c9d..8ced619b08b2 100644 --- a/tests/introspection/models.py +++ b/tests/introspection/models.py @@ -112,11 +112,20 @@ class Meta: required_db_features = {"supports_comments"} -class DbOnDeleteModel(models.Model): +class DbOnDeleteCascadeModel(models.Model): fk_do_nothing = models.ForeignKey(Country, on_delete=models.DO_NOTHING) fk_db_cascade = models.ForeignKey(City, on_delete=models.DB_CASCADE) + + class Meta: + required_db_features = {"supports_on_delete_db_cascade"} + + +class DbOnDeleteSetNullModel(models.Model): fk_set_null = models.ForeignKey(Reporter, on_delete=models.DB_SET_NULL, null=True) + class Meta: + required_db_features = {"supports_on_delete_db_null"} + class DbOnDeleteSetDefaultModel(models.Model): fk_db_set_default = models.ForeignKey( diff --git a/tests/introspection/tests.py b/tests/introspection/tests.py index 1f7f22b2dc16..3a2c6112c874 100644 --- a/tests/introspection/tests.py +++ b/tests/introspection/tests.py @@ -10,8 +10,9 @@ Comment, Country, DbCommentModel, - DbOnDeleteModel, + DbOnDeleteCascadeModel, DbOnDeleteSetDefaultModel, + DbOnDeleteSetNullModel, District, Reporter, UniqueConstraintConditionModel, @@ -244,11 +245,11 @@ def test_get_relations(self): editor.add_field(Article, body) self.assertEqual(relations, expected_relations) - @skipUnlessDBFeature("can_introspect_foreign_keys") - def test_get_relations_db_on_delete(self): + @skipUnlessDBFeature("can_introspect_foreign_keys", "supports_on_delete_db_cascade") + def test_get_relations_db_on_delete_cascade(self): with connection.cursor() as cursor: relations = connection.introspection.get_relations( - cursor, DbOnDeleteModel._meta.db_table + cursor, DbOnDeleteCascadeModel._meta.db_table ) if connection.vendor == "mysql" and connection.mysql_is_mariadb: @@ -259,6 +260,18 @@ def test_get_relations_db_on_delete(self): expected_relations = { "fk_db_cascade_id": ("id", City._meta.db_table, DB_CASCADE), "fk_do_nothing_id": ("id", Country._meta.db_table, no_db_on_delete), + } + self.assertEqual(relations, expected_relations) + + @skipUnlessDBFeature("can_introspect_foreign_keys", "supports_on_delete_db_null") + def test_get_relations_db_on_delete_null(self): + with connection.cursor() as cursor: + relations = connection.introspection.get_relations( + cursor, DbOnDeleteSetNullModel._meta.db_table + ) + + # {field_name: (field_name_other_table, other_table, db_on_delete)} + expected_relations = { "fk_set_null_id": ("id", Reporter._meta.db_table, DB_SET_NULL), } self.assertEqual(relations, expected_relations) diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py index e73f22ab41fe..51b1de149427 100644 --- a/tests/invalid_models_tests/test_relative_fields.py +++ b/tests/invalid_models_tests/test_relative_fields.py @@ -2288,6 +2288,41 @@ class FooBar(models.Model): @isolate_apps("invalid_models_tests") class DatabaseLevelOnDeleteTests(TestCase): + def test_db_cascade_support(self): + class Parent(models.Model): + pass + + class Child(models.Model): + parent = models.ForeignKey(Parent, models.DB_CASCADE) + + field = Child._meta.get_field("parent") + expected = ( + [] + if connection.features.supports_on_delete_db_cascade + else [ + Error( + f"{connection.display_name} does not support a DB_CASCADE.", + hint="Change the on_delete rule to CASCADE.", + obj=field, + id="fields.E324", + ) + ] + ) + self.assertEqual(field.check(databases=self.databases), expected) + + def test_db_cascade_required_db_features(self): + class Parent(models.Model): + pass + + class Child(models.Model): + parent = models.ForeignKey(Parent, models.DB_CASCADE) + + class Meta: + required_db_features = {"supports_on_delete_db_cascade"} + + field = Child._meta.get_field("parent") + self.assertEqual(field.check(databases=self.databases), []) + def test_db_set_default_support(self): class Parent(models.Model): pass @@ -2349,6 +2384,41 @@ class Child(models.Model): ], ) + def test_db_set_null_support(self): + class Parent(models.Model): + pass + + class Child(models.Model): + parent = models.ForeignKey(Parent, models.DB_SET_NULL, null=True) + + field = Child._meta.get_field("parent") + expected = ( + [] + if connection.features.supports_on_delete_db_null + else [ + Error( + f"{connection.display_name} does not support a DB_SET_NULL.", + hint="Change the on_delete rule to SET_NULL.", + obj=field, + id="fields.E324", + ) + ] + ) + self.assertEqual(field.check(databases=self.databases), expected) + + def test_db_set_null_required_db_features(self): + class Parent(models.Model): + pass + + class Child(models.Model): + parent = models.ForeignKey(Parent, models.DB_SET_NULL, null=True) + + class Meta: + required_db_features = {"supports_on_delete_db_null"} + + field = Child._meta.get_field("parent") + self.assertEqual(field.check(databases=self.databases), []) + def test_python_db_chain(self): class GrandParent(models.Model): pass @@ -2376,6 +2446,7 @@ class Child(models.Model): ], ) + @skipUnlessDBFeature("supports_on_delete_db_null") def test_db_python_chain(self): class GrandParent(models.Model): pass @@ -2403,6 +2474,7 @@ class Child(models.Model): ], ) + @skipUnlessDBFeature("supports_on_delete_db_cascade") def test_db_python_chain_auto_created(self): class GrandParent(models.Model): pass @@ -2430,6 +2502,7 @@ class Child(models.Model): ], ) + @skipUnlessDBFeature("supports_on_delete_db_null") def test_db_do_nothing_chain(self): class GrandParent(models.Model): pass