Allow delete where clause to refer multiple tables.

Implemented "DELETE..FROM" syntax for Postgresql, MySQL, MS SQL Server
(as well as within the unsupported Sybase dialect) in a manner similar
to how "UPDATE..FROM" works.  A DELETE statement that refers to more than
one table will switch into "multi-table" mode and render the appropriate
"USING" or multi-table "FROM" clause as understood by the database.
Pull request courtesy Pieter Mulder.

For SQL syntaxes see:

Postgresql: https://www.postgresql.org/docs/current/static/sql-delete.html
MySQL: https://dev.mysql.com/doc/refman/5.7/en/delete.html#multiple-table_syntax
MSSQL: https://docs.microsoft.com/en-us/sql/t-sql/statements/delete-transact-sql
Sybase: http://infocenter.sybase.com/help/index.jsp?topic=/com.sybase.infocenter.dc00801.1510/html/iqrefso/X315721.htm

Co-authored by: Mike Bayer <mike_mp@zzzcomputing.com>
Change-Id: I6dfd57b49e44a095d076dc493cd2360bb5d920d3
Pull-request: https://github.com/zzzeek/sqlalchemy/pull/392
Fixes: #959

7 files changed, 166 insertions, 9 deletions

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index 0c3688c9b..9f4e7a9c4 100644
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -1510,6 +1510,28 @@ class MSSQLCompiler(compiler.SQLCompiler):
                                  fromhints=from_hints, **kw)
             for t in [from_table] + extra_froms)
 
+    def delete_table_clause(self, delete_stmt, from_table,
+                            extra_froms):
+        """If we have extra froms make sure we render any alias as hint."""
+        ashint = False
+        if extra_froms:
+            ashint = True
+        return from_table._compiler_dispatch(
+            self, asfrom=True, iscrud=True, ashint=ashint
+        )
+
+    def delete_extra_from_clause(self, delete_stmt, from_table,
+                                 extra_froms, from_hints, **kw):
+        """Render the DELETE .. FROM clause specific to MSSQL.
+
+        Yes, it has the FROM keyword twice.
+
+        """
+        return "FROM " + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in [from_table] + extra_froms)
+
 
 class MSSQLStrictCompiler(MSSQLCompiler):
 
diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py
index bee62b76f..09f8a2a0e 100644
--- a/lib/sqlalchemy/dialects/mysql/base.py
+++ b/lib/sqlalchemy/dialects/mysql/base.py
@@ -1102,6 +1102,24 @@ class MySQLCompiler(compiler.SQLCompiler):
                            extra_froms, from_hints, **kw):
         return None
 
+    def delete_table_clause(self, delete_stmt, from_table,
+                            extra_froms):
+        """If we have extra froms make sure we render any alias as hint."""
+        ashint = False
+        if extra_froms:
+            ashint = True
+        return from_table._compiler_dispatch(
+            self, asfrom=True, iscrud=True, ashint=ashint
+        )
+
+    def delete_extra_from_clause(self, delete_stmt, from_table,
+                           extra_froms, from_hints, **kw):
+        """Render the DELETE .. USING clause specific to MySQL."""
+        return "USING " + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in [from_table] + extra_froms)
+
 
 class MySQLDDLCompiler(compiler.DDLCompiler):
     def get_column_specification(self, column, **kw):
diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py
index 32de7861a..043efd6df 100644
--- a/lib/sqlalchemy/dialects/postgresql/base.py
+++ b/lib/sqlalchemy/dialects/postgresql/base.py
@@ -1647,6 +1647,23 @@ class PGCompiler(compiler.SQLCompiler):
 
         return 'ON CONFLICT %s DO UPDATE SET %s' % (target_text, action_text)
 
+    def update_from_clause(self, update_stmt,
+                           from_table, extra_froms,
+                           from_hints,
+                           **kw):
+        return "FROM " + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in extra_froms)
+
+    def delete_extra_from_clause(self, delete_stmt, from_table,
+                           extra_froms, from_hints, **kw):
+        """Render the DELETE .. USING clause specific to PostgresSQL."""
+        return "USING " + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in extra_froms)
+
 
 class PGDDLCompiler(compiler.DDLCompiler):
 
diff --git a/lib/sqlalchemy/dialects/sybase/base.py b/lib/sqlalchemy/dialects/sybase/base.py
index 5d2f0f70c..0d7d03e78 100644
--- a/lib/sqlalchemy/dialects/sybase/base.py
+++ b/lib/sqlalchemy/dialects/sybase/base.py
@@ -369,6 +369,24 @@ class SybaseSQLCompiler(compiler.SQLCompiler):
         else:
             return ""
 
+    def delete_table_clause(self, delete_stmt, from_table,
+                            extra_froms):
+        """If we have extra froms make sure we render any alias as hint."""
+        ashint = False
+        if extra_froms:
+            ashint = True
+        return from_table._compiler_dispatch(
+            self, asfrom=True, iscrud=True, ashint=ashint
+        )
+
+    def delete_extra_from_clause(self, delete_stmt, from_table,
+                                 extra_froms, from_hints, **kw):
+        """Render the DELETE .. FROM clause specific to Sybase."""
+        return "FROM " + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in [from_table] + extra_froms)
+
 
 class SybaseDDLCompiler(compiler.DDLCompiler):
     def get_column_specification(self, column, **kwargs):
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index b0f0807d6..9ed75ca06 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -2146,10 +2146,9 @@ class SQLCompiler(Compiled):
         MySQL and MSSQL override this.
 
         """
-        return "FROM " + ', '.join(
-            t._compiler_dispatch(self, asfrom=True,
-                                 fromhints=from_hints, **kw)
-            for t in extra_froms)
+        raise NotImplementedError(
+            "This backend does not support multiple-table "
+            "criteria within UPDATE")
 
     def visit_update(self, update_stmt, asfrom=False, **kw):
         toplevel = not self.stack
@@ -2232,6 +2231,25 @@ class SQLCompiler(Compiled):
     def _key_getters_for_crud_column(self):
         return crud._key_getters_for_crud_column(self, self.statement)
 
+    def delete_extra_from_clause(self, update_stmt,
+                                 from_table, extra_froms,
+                                 from_hints, **kw):
+        """Provide a hook to override the generation of an
+        DELETE..FROM clause.
+
+        This can be used to implement DELETE..USING for example.
+
+        MySQL and MSSQL override this.
+
+        """
+        raise NotImplementedError(
+            "This backend does not support multiple-table "
+            "criteria within DELETE")
+
+    def delete_table_clause(self, delete_stmt, from_table,
+                            extra_froms):
+        return from_table._compiler_dispatch(self, asfrom=True, iscrud=True)
+
     def visit_delete(self, delete_stmt, asfrom=False, **kw):
         toplevel = not self.stack
 
@@ -2241,6 +2259,8 @@ class SQLCompiler(Compiled):
 
         crud._setup_crud_params(self, delete_stmt, crud.ISDELETE, **kw)
 
+        extra_froms = delete_stmt._extra_froms
+
         text = "DELETE "
 
         if delete_stmt._prefixes:
@@ -2248,12 +2268,14 @@ class SQLCompiler(Compiled):
                                             delete_stmt._prefixes, **kw)
 
         text += "FROM "
-        table_text = delete_stmt.table._compiler_dispatch(
-            self, asfrom=True, iscrud=True)
+        table_text = self.delete_table_clause(delete_stmt, delete_stmt.table,
+                                              extra_froms)
 
         if delete_stmt._hints:
             dialect_hints, table_text = self._setup_crud_hints(
                 delete_stmt, table_text)
+        else:
+            dialect_hints = None
 
         text += table_text
 
@@ -2262,6 +2284,15 @@ class SQLCompiler(Compiled):
                 text += " " + self.returning_clause(
                     delete_stmt, delete_stmt._returning)
 
+        if extra_froms:
+            extra_from_text = self.delete_extra_from_clause(
+                delete_stmt,
+                delete_stmt.table,
+                extra_froms,
+                dialect_hints, **kw)
+            if extra_from_text:
+                text += " " + extra_from_text
+
         if delete_stmt._whereclause is not None:
             t = delete_stmt._whereclause._compiler_dispatch(self, **kw)
             if t:
@@ -2324,6 +2355,24 @@ class StrSQLCompiler(SQLCompiler):
 
         return 'RETURNING ' + ', '.join(columns)
 
+    def update_from_clause(self, update_stmt,
+                           from_table, extra_froms,
+                           from_hints,
+                           **kw):
+        return "FROM " + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in extra_froms)
+
+    def delete_extra_from_clause(self, update_stmt,
+                           from_table, extra_froms,
+                           from_hints,
+                           **kw):
+        return ', ' + ', '.join(
+            t._compiler_dispatch(self, asfrom=True,
+                                 fromhints=from_hints, **kw)
+            for t in extra_froms)
+
 
 class DDLCompiler(Compiled):
 
diff --git a/lib/sqlalchemy/sql/dml.py b/lib/sqlalchemy/sql/dml.py
index 958e9bfb1..6c8c550b4 100644
--- a/lib/sqlalchemy/sql/dml.py
+++ b/lib/sqlalchemy/sql/dml.py
@@ -656,7 +656,7 @@ class Update(ValuesBase):
                     )
 
          .. versionchanged:: 0.7.4
-             The WHERE clause can refer to multiple tables.
+             The WHERE clause of UPDATE can refer to multiple tables.
 
         :param values:
           Optional dictionary which specifies the ``SET`` conditions of the
@@ -768,8 +768,6 @@ class Update(ValuesBase):
 
     @property
     def _extra_froms(self):
-        # TODO: this could be made memoized
-        # if the memoization is reset on each generative call.
         froms = []
         seen = {self.table}
 
@@ -811,6 +809,23 @@ class Delete(UpdateBase):
           condition of the ``DELETE`` statement. Note that the
           :meth:`~Delete.where()` generative method may be used instead.
 
+         The WHERE clause can refer to multiple tables.
+         For databases which support this, a ``DELETE..USING`` or similar
+         clause will be generated.  The statement
+         will fail on databases that don't have support for multi-table
+         delete statements.  A SQL-standard method of referring to
+         additional tables in the WHERE clause is to use a correlated
+         subquery::
+
+            users.delete().where(
+                    users.c.name==select([addresses.c.email_address]).\
+                                where(addresses.c.user_id==users.c.id).\
+                                as_scalar()
+                    )
+
+         .. versionchanged:: 1.2.0
+             The WHERE clause of DELETE can refer to multiple tables.
+
         .. seealso::
 
             :ref:`deletes` - SQL Expression Tutorial
@@ -846,6 +861,19 @@ class Delete(UpdateBase):
         else:
             self._whereclause = _literal_as_text(whereclause)
 
+    @property
+    def _extra_froms(self):
+        froms = []
+        seen = {self.table}
+
+        if self._whereclause is not None:
+            for item in _from_objects(self._whereclause):
+                if not seen.intersection(item._cloned_set):
+                    froms.append(item)
+                seen.update(item._cloned_set)
+
+        return froms
+
     def _copy_internals(self, clone=_clone, **kw):
         # TODO: coverage
         self._whereclause = clone(self._whereclause, **kw)
diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py
index a66b091aa..0b89a4506 100644
--- a/lib/sqlalchemy/testing/requirements.py
+++ b/lib/sqlalchemy/testing/requirements.py
@@ -693,6 +693,11 @@ class SuiteRequirements(Requirements):
         return exclusions.closed()
 
     @property
+    def delete_from(self):
+        """Target must support DELETE FROM..FROM or DELETE..USING syntax"""
+        return exclusions.closed()
+
+    @property
     def update_where_target_in_subquery(self):
         """Target must support UPDATE where the same table is present in a
         subquery in the WHERE clause.