From f6405c0b8ef7aff513b105c1da68407a881a3671 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 30 Aug 2020 03:00:15 -0400 Subject: [PATCH] Fixed #31965 -- Adjusted multi-table fast-deletion on MySQL/MariaDB. The optimization introduced in 7acef095d73 did not properly handle deletion involving filters against aggregate annotations. It initially was surfaced by a MariaDB test failure but misattributed to an undocumented change in behavior that resulted in the systemic generation of poorly performing database queries in 5b83bae031. Thanks Anton Plotkin for the report. Refs #23576. --- django/db/backends/mysql/compiler.py | 22 ++++++++++++---------- django/db/models/sql/compiler.py | 5 +++++ docs/releases/3.1.1.txt | 4 ++++ tests/delete/models.py | 2 +- tests/delete/tests.py | 13 +++++++++++++ 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/django/db/backends/mysql/compiler.py b/django/db/backends/mysql/compiler.py index 1b4d583fb0..da02c1fd73 100644 --- a/django/db/backends/mysql/compiler.py +++ b/django/db/backends/mysql/compiler.py @@ -16,13 +16,15 @@ class SQLInsertCompiler(compiler.SQLInsertCompiler, SQLCompiler): class SQLDeleteCompiler(compiler.SQLDeleteCompiler, SQLCompiler): def as_sql(self): - if self.connection.features.update_can_self_select or self.single_alias: + # Prefer the non-standard DELETE FROM syntax over the SQL generated by + # the SQLDeleteCompiler's default implementation when multiple tables + # are involved since MySQL/MariaDB will generate a more efficient query + # plan than when using a subquery. + where, having = self.query.where.split_having() + if self.single_alias or having: + # DELETE FROM cannot be used when filtering against aggregates + # since it doesn't allow for GROUP BY and HAVING clauses. return super().as_sql() - # MySQL and MariaDB < 10.3.2 doesn't support deletion with a subquery - # which is what the default implementation of SQLDeleteCompiler uses - # when multiple tables are involved. Use the MySQL/MariaDB specific - # DELETE table FROM table syntax instead to avoid performing the - # operation in two queries. result = [ 'DELETE %s FROM' % self.quote_name_unless_alias( self.query.get_initial_alias() @@ -30,10 +32,10 @@ class SQLDeleteCompiler(compiler.SQLDeleteCompiler, SQLCompiler): ] from_sql, from_params = self.get_from_clause() result.extend(from_sql) - where, params = self.compile(self.query.where) - if where: - result.append('WHERE %s' % where) - return ' '.join(result), tuple(from_params) + tuple(params) + where_sql, where_params = self.compile(where) + if where_sql: + result.append('WHERE %s' % where_sql) + return ' '.join(result), tuple(from_params) + tuple(where_params) class SQLUpdateCompiler(compiler.SQLUpdateCompiler, SQLCompiler): diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index b321762303..208f0ddf73 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1443,6 +1443,11 @@ class SQLDeleteCompiler(SQLCompiler): ] outerq = Query(self.query.model) outerq.where = self.query.where_class() + if not self.connection.features.update_can_self_select: + # Force the materialization of the inner query to allow reference + # to the target table on MySQL. + sql, params = innerq.get_compiler(connection=self.connection).as_sql() + innerq = RawSQL('SELECT * FROM (%s) subquery' % sql, params) outerq.add_q(Q(pk__in=innerq)) return self._as_sql(outerq) diff --git a/docs/releases/3.1.1.txt b/docs/releases/3.1.1.txt index 452a922608..b1d598bf3c 100644 --- a/docs/releases/3.1.1.txt +++ b/docs/releases/3.1.1.txt @@ -55,3 +55,7 @@ Bugfixes * Fixed a ``QuerySet.order_by()`` crash on PostgreSQL when ordering and grouping by :class:`~django.db.models.JSONField` with a custom :attr:`~django.db.models.JSONField.decoder` (:ticket:`31956`). + +* Fixed a ``QuerySet.delete()`` crash on MySQL, following a performance + regression in Django 3.1 on MariaDB 10.3.2+, when filtering against an + aggregate function (:ticket:`31965`). diff --git a/tests/delete/models.py b/tests/delete/models.py index 7f6e6a89ce..96ef65c766 100644 --- a/tests/delete/models.py +++ b/tests/delete/models.py @@ -141,7 +141,7 @@ class Base(models.Model): class RelToBase(models.Model): - base = models.ForeignKey(Base, models.DO_NOTHING) + base = models.ForeignKey(Base, models.DO_NOTHING, related_name='rels') class Origin(models.Model): diff --git a/tests/delete/tests.py b/tests/delete/tests.py index e1ec26bc1a..485ae1aaf5 100644 --- a/tests/delete/tests.py +++ b/tests/delete/tests.py @@ -709,3 +709,16 @@ class FastDeleteTests(TestCase): referer = Referrer.objects.create(origin=origin, unique_field=42) with self.assertNumQueries(2): referer.delete() + + def test_fast_delete_aggregation(self): + # Fast-deleting when filtering against an aggregation result in + # a single query containing a subquery. + Base.objects.create() + with self.assertNumQueries(1): + self.assertEqual( + Base.objects.annotate( + rels_count=models.Count('rels'), + ).filter(rels_count=0).delete(), + (1, {'delete.Base': 1}), + ) + self.assertIs(Base.objects.exists(), False)