Generalize RETURNING and suppor for MariaDB / SQLite

As almost every dialect supports RETURNING now, RETURNING
is also made more of a default assumption.

* the default compiler generates a RETURNING clause now
  when specified; CompileError is no longer raised.
* The dialect-level implicit_returning parameter now has
  no effect.   It's not fully clear if there are real world
  cases relying on the dialect-level parameter, so we will see
  once 2.0 is released.   ORM-level RETURNING can be disabled
  at the table level, and perhaps "implicit returning" should
  become an ORM-level option at some point as that's where
  it applies.
* Altered ORM update() / delete() to respect table-level
  implicit returning for fetch.
* Since MariaDB doesnt support UPDATE returning, "full_returning"
  is now split into insert_returning, update_returning, delete_returning
* Crazy new thing.  Dialects that have *both* cursor.lastrowid
  *and* returning.   so now we can pick between them for SQLite
  and mariadb.  so, we are trying to keep it on .lastrowid for
  simple inserts with an autoincrement column, this helps with
  some edge case test scenarios and i bet .lastrowid is faster
  anyway.  any return_defaults() / multiparams etc then we
  use returning
* SQLite decided they dont want to return rows that match in
  ON CONFLICT.  this is flat out wrong, but for now we need to
  work with it.

Fixes: #6195
Fixes: #7011
Closes: #7047
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/7047
Pull-request-sha: d25d5ea3abe094f282c53c7dd87f5f53a9e85248

Co-authored-by: Mike Bayer <mike_mp@zzzcomputing.com>
Change-Id: I9908ce0ff7bdc50bd5b27722081767c31c19a950

5 files changed, 103 insertions, 25 deletions

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 3685751b0..78c6af38b 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -3482,7 +3482,7 @@ class SQLCompiler(Compiled):
         )
 
     def _label_returning_column(
-        self, stmt, column, populate_result_map, column_clause_args=None
+        self, stmt, column, populate_result_map, column_clause_args=None, **kw
     ):
         """Render a column with necessary labels inside of a RETURNING clause.
 
@@ -3499,6 +3499,7 @@ class SQLCompiler(Compiled):
             populate_result_map,
             False,
             {} if column_clause_args is None else column_clause_args,
+            **kw,
         )
 
     def _label_select_column(
@@ -3514,6 +3515,7 @@ class SQLCompiler(Compiled):
         within_columns_clause=True,
         column_is_repeated=False,
         need_column_expressions=False,
+        include_table=True,
     ):
         """produce labeled columns present in a select()."""
         impl = column.type.dialect_impl(self.dialect)
@@ -3661,6 +3663,7 @@ class SQLCompiler(Compiled):
         column_clause_args.update(
             within_columns_clause=within_columns_clause,
             add_to_result_map=add_to_result_map,
+            include_table=include_table,
         )
         return result_expr._compiler_dispatch(self, **column_clause_args)
 
@@ -4218,10 +4221,12 @@ class SQLCompiler(Compiled):
         populate_result_map: bool,
         **kw: Any,
     ) -> str:
-        raise exc.CompileError(
-            "RETURNING is not supported by this "
-            "dialect's statement compiler."
-        )
+        columns = [
+            self._label_returning_column(stmt, c, populate_result_map, **kw)
+            for c in base._select_iterables(returning_cols)
+        ]
+
+        return "RETURNING " + ", ".join(columns)
 
     def limit_clause(self, select, **kw):
         text = ""
diff --git a/lib/sqlalchemy/sql/crud.py b/lib/sqlalchemy/sql/crud.py
index 913e4d433..81151a26b 100644
--- a/lib/sqlalchemy/sql/crud.py
+++ b/lib/sqlalchemy/sql/crud.py
@@ -568,6 +568,7 @@ def _scan_cols(
                 _col_bind_name,
                 implicit_returning,
                 implicit_return_defaults,
+                postfetch_lastrowid,
                 values,
                 autoincrement_col,
                 insert_null_pk_still_autoincrements,
@@ -649,6 +650,7 @@ def _append_param_parameter(
     _col_bind_name,
     implicit_returning,
     implicit_return_defaults,
+    postfetch_lastrowid,
     values,
     autoincrement_col,
     insert_null_pk_still_autoincrements,
@@ -668,11 +670,12 @@ def _append_param_parameter(
             and c is autoincrement_col
         ):
             # support use case for #7998, fetch autoincrement cols
-            # even if value was given
-            if implicit_returning:
-                compiler.implicit_returning.append(c)
-            elif compiler.dialect.postfetch_lastrowid:
+            # even if value was given.
+
+            if postfetch_lastrowid:
                 compiler.postfetch_lastrowid = True
+            elif implicit_returning:
+                compiler.implicit_returning.append(c)
 
         value = _create_bind_param(
             compiler,
@@ -1281,7 +1284,12 @@ def _get_stmt_parameter_tuples_params(
 
 
 def _get_returning_modifiers(compiler, stmt, compile_state, toplevel):
+    """determines RETURNING strategy, if any, for the statement.
+
+    This is where it's determined what we need to fetch from the
+    INSERT or UPDATE statement after it's invoked.
 
+    """
     need_pks = (
         toplevel
         and compile_state.isinsert
@@ -1296,19 +1304,58 @@ def _get_returning_modifiers(compiler, stmt, compile_state, toplevel):
         and not stmt._returning
         and not compile_state._has_multi_parameters
     )
+
+    # check if we have access to simple cursor.lastrowid.  we can use that
+    # after the INSERT if that's all we need.
+    postfetch_lastrowid = (
+        need_pks
+        and compiler.dialect.postfetch_lastrowid
+        and stmt.table._autoincrement_column is not None
+    )
+
+    # see if we want to add RETURNING to an INSERT in order to get
+    # primary key columns back.  This would be instead of postfetch_lastrowid
+    # if that's set.
     implicit_returning = (
+        # statement itself can veto it
         need_pks
-        and compiler.dialect.implicit_returning
-        and stmt.table.implicit_returning
+        # the dialect can veto it if it just doesnt support RETURNING
+        # with INSERT
+        and compiler.dialect.insert_returning
+        # user-defined implicit_returning on Table can veto it
+        and compile_state._primary_table.implicit_returning
+        # the compile_state can veto it (SQlite uses this to disable
+        # RETURNING for an ON CONFLICT insert, as SQLite does not return
+        # for rows that were updated, which is wrong)
+        and compile_state._supports_implicit_returning
+        and (
+            # since we support MariaDB and SQLite which also support lastrowid,
+            # decide if we should use lastrowid or RETURNING.  for insert
+            # that didnt call return_defaults() and has just one set of
+            # parameters, we can use lastrowid.   this is more "traditional"
+            # and a lot of weird use cases are supported by it.
+            # SQLite lastrowid times 3x faster than returning,
+            # Mariadb lastrowid 2x faster than returning
+            (
+                not postfetch_lastrowid
+                or compiler.dialect.favor_returning_over_lastrowid
+            )
+            or compile_state._has_multi_parameters
+            or stmt._return_defaults
+        )
     )
 
+    if implicit_returning:
+        postfetch_lastrowid = False
+
     if compile_state.isinsert:
         implicit_return_defaults = implicit_returning and stmt._return_defaults
     elif compile_state.isupdate:
         implicit_return_defaults = (
-            compiler.dialect.implicit_returning
-            and stmt.table.implicit_returning
-            and stmt._return_defaults
+            stmt._return_defaults
+            and compile_state._primary_table.implicit_returning
+            and compile_state._supports_implicit_returning
+            and compiler.dialect.update_returning
         )
     else:
         # this line is unused, currently we are always
@@ -1321,7 +1368,6 @@ def _get_returning_modifiers(compiler, stmt, compile_state, toplevel):
         else:
             implicit_return_defaults = set(stmt._return_defaults_columns)
 
-    postfetch_lastrowid = need_pks and compiler.dialect.postfetch_lastrowid
     return (
         need_pks,
         implicit_returning,
diff --git a/lib/sqlalchemy/sql/dml.py b/lib/sqlalchemy/sql/dml.py
index e63a34454..28ea512a7 100644
--- a/lib/sqlalchemy/sql/dml.py
+++ b/lib/sqlalchemy/sql/dml.py
@@ -119,6 +119,8 @@ class DMLState(CompileState):
     _ordered_values: Optional[List[Tuple[_DMLColumnElement, Any]]] = None
     _parameter_ordering: Optional[List[_DMLColumnElement]] = None
     _has_multi_parameters = False
+    _primary_table: FromClause
+    _supports_implicit_returning = True
 
     isupdate = False
     isdelete = False
@@ -182,11 +184,14 @@ class DMLState(CompileState):
             for k, v in kv_iterator
         ]
 
-    def _make_extra_froms(self, statement: DMLWhereBase) -> List[FromClause]:
+    def _make_extra_froms(
+        self, statement: DMLWhereBase
+    ) -> Tuple[FromClause, List[FromClause]]:
         froms: List[FromClause] = []
 
         all_tables = list(sql_util.tables_from_leftmost(statement.table))
-        seen = {all_tables[0]}
+        primary_table = all_tables[0]
+        seen = {primary_table}
 
         for crit in statement._where_criteria:
             for item in _from_objects(crit):
@@ -195,7 +200,7 @@ class DMLState(CompileState):
                 seen.update(item._cloned_set)
 
         froms.extend(all_tables[1:])
-        return froms
+        return primary_table, froms
 
     def _process_multi_values(self, statement: ValuesBase) -> None:
         if not statement._supports_multi_parameters:
@@ -286,8 +291,18 @@ class InsertDMLState(DMLState):
 
     include_table_with_column_exprs = False
 
-    def __init__(self, statement: Insert, compiler: SQLCompiler, **kw: Any):
+    def __init__(
+        self,
+        statement: Insert,
+        compiler: SQLCompiler,
+        disable_implicit_returning: bool = False,
+        **kw: Any,
+    ):
         self.statement = statement
+        self._primary_table = statement.table
+
+        if disable_implicit_returning:
+            self._supports_implicit_returning = False
 
         self.isinsert = True
         if statement._select_names:
@@ -306,6 +321,7 @@ class UpdateDMLState(DMLState):
 
     def __init__(self, statement: Update, compiler: SQLCompiler, **kw: Any):
         self.statement = statement
+
         self.isupdate = True
         if statement._ordered_values is not None:
             self._process_ordered_values(statement)
@@ -313,7 +329,9 @@ class UpdateDMLState(DMLState):
             self._process_values(statement)
         elif statement._multi_values:
             self._process_multi_values(statement)
-        self._extra_froms = ef = self._make_extra_froms(statement)
+        t, ef = self._make_extra_froms(statement)
+        self._primary_table = t
+        self._extra_froms = ef
 
         self.is_multitable = mt = ef
 
@@ -330,7 +348,9 @@ class DeleteDMLState(DMLState):
         self.statement = statement
 
         self.isdelete = True
-        self._extra_froms = self._make_extra_froms(statement)
+        t, ef = self._make_extra_froms(statement)
+        self._primary_table = t
+        self._extra_froms = ef
 
 
 SelfUpdateBase = typing.TypeVar("SelfUpdateBase", bound="UpdateBase")
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 598bacc59..447e102ed 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -639,10 +639,13 @@ class Table(
 
 
         :param implicit_returning: True by default - indicates that
-            RETURNING can be used by default to fetch newly inserted primary key
-            values, for backends which support this.  Note that
-            :func:`_sa.create_engine` also provides an ``implicit_returning``
-            flag.
+            RETURNING can be used, typically by the ORM, in order to fetch
+            server-generated values such as primary key values and
+            server side defaults, on those backends which support RETURNING.
+
+            In modern SQLAlchemy there is generally no reason to alter this
+            setting, except in the case of some backends such as SQL Server
+            when INSERT triggers are used for that table.
 
         :param include_columns: A list of strings indicating a subset of
             columns to be loaded via the ``autoload`` operation; table columns who
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 53dcf51c7..eebefb877 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -1635,6 +1635,10 @@ class AliasedReturnsRows(NoInit, NamedFromClause):
 
         return name
 
+    @util.ro_non_memoized_property
+    def implicit_returning(self):
+        return self.element.implicit_returning  # type: ignore
+
     @property
     def original(self):
         """Legacy for dialects that are referring to Alias.original."""