Skip to content

Commit

Permalink
[3.1.x] Fixed #31965 -- Adjusted multi-table fast-deletion on MySQL/M…
Browse files Browse the repository at this point in the history
…ariaDB.

The optimization introduced in 7acef09 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 5b83bae.

Thanks Anton Plotkin for the report.

Refs #23576.

Backport of f6405c0 from master
  • Loading branch information
charettes authored and felixxm committed Aug 31, 2020
1 parent 655e1ce commit 2986ec0
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 11 deletions.
22 changes: 12 additions & 10 deletions django/db/backends/mysql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,26 @@ 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()
)
]
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):
Expand Down
5 changes: 5 additions & 0 deletions django/db/models/sql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,11 @@ def as_sql(self):
]
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)

Expand Down
4 changes: 4 additions & 0 deletions docs/releases/3.1.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
2 changes: 1 addition & 1 deletion tests/delete/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
13 changes: 13 additions & 0 deletions tests/delete/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -709,3 +709,16 @@ def test_fast_delete_combined_relationships(self):
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)

0 comments on commit 2986ec0

Please sign in to comment.