Using subquery to filter a model delete generates a sub-optimal SQL

[Christofer Bertonha]

Taking in consideration the app of tutorial for demonstrate the issue.
using postgresql as db.

Executing this following code:

subquery = Comment.objects.filter(created_at__lt=datetime(2022, 6, 1))[:1000]
Comment.objects.filter(id__in=subquery).delete()

Generates the following SQL:

DELETE FROM "comment" WHERE "comment"."id" IN (SELECT V0."id" FROM "comment" V0 WHERE V0."id" IN (SELECT U0."id" FROM "comment" U0 WHERE U0."created_at" < '2022-06-01T00:00:00+00:00'::timestamptz LIMIT 1000))

The inner select V0 is completely unnecessary

If a simple select with the same subquery is executed. It generates expected SQL query.

subquery = Comment.objects.filter(created_at__lt=datetime(2022, 6, 1))[:1000]
comments = list( Comment.objects..filter(id__in=subquery) )

SELECT * FROM "comment" WHERE "comment"."id" IN (SELECT U0."id" FROM "comment" U0 WHERE U0."created_at" < '2022-06-01T00:00:00+00:00'::timestamptz LIMIT 1000),

[Mariusz Felisiak]

This behavior was changed in 4074f38e1dcc93b859bbbfd6abd8441c3bca36b3 to prevent:

django.db.utils.NotSupportedError: (1235, "This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'")

on MySQL. I have no idea for an alternative fix.

[Mariusz Felisiak]

We could add a new feature flag to add an extra subquery only on MySQL:

django/db/backends/base/features.py

@@ -12 +12 @@
     allows_group_by_selected_pks = False
     empty_fetchmany_value = []
     update_can_self_select = True
+    # Does the backend support self-reference subqueries in the DELETE
+    # statement?
+    delete_can_self_reference_subquery = True
 
     # Does the backend distinguish between '' and None?
     interprets_empty_strings_as_nulls = False

django/db/backends/mysql/features.py

@@ -25 +25 @@
     supports_slicing_ordering_in_compound = True
     supports_index_on_text_field = False
     supports_update_conflicts = True
+    delete_can_self_reference_subquery = False
     create_test_procedure_without_params_sql = """
         CREATE PROCEDURE test_procedure ()
         BEGIN

django/db/models/sql/compiler.py

@@ -1721 +1721 @@
         Create the SQL for this query. Return the SQL string and list of
         parameters.
         """
-        if self.single_alias and not self.contains_self_reference_subquery:
+        if self.single_alias and (
+            self.connection.features.delete_can_self_reference_subquery or
+            not self.contains_self_reference_subquery
+        ):
             return self._as_sql(self.query)
         innerq = self.query.clone()
         innerq.__class__ = Query

Another issue is that QuerySet.delete() from the ticket description:

subquery = Comment.objects.filter(created_at__lt=datetime(2022, 6, 1))[:1000]
Comment.objects.filter(id__in=subquery).delete()
...
  File "site-packages/MySQLdb/connections.py", line 254, in query
    _mysql.connection.query(self, query)
django.db.utils.NotSupportedError: (1235, "This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'")

crashes on MySQL even with 4074f38e1dcc93b859bbbfd6abd8441c3bca36b3.

[Simon Charette]

Mariusz I think that it makes sense to only enable the query wrapping when necessary through a feature flag as you've suggested.

As for the MySQL limited subquery issue that's a different can of worms. The unnecessary query wrapping should be testable on backends with the feature disabled by counting the number of SELECT.

Thanks for the ping and sorry for my late answer!

[Aman Pandey]

Draft PR up ​https://github.com/django/django/pull/16809

[Aman Pandey]

PR ready.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 0b0998dc:

Fixed #33759 -- Avoided unnecessary subquery in QuerySet.delete() with self-referential subqueries if supported.