Updates for MariaDB sequences

MariaDB should not run a Sequence if it has optional=True.
Additionally, rework the rules in crud.py to accommodate the
new combination MariaDB brings us, which is a dialect
that supports both cursor.lastrowid, explicit sequences,
*and* no support for returning.

Co-authored-by: Mike Bayer <mike_mp@zzzcomputing.com>
Fixes: #5528
Change-Id: I9a8ea69a34983affa95dfd22186e2908fdf0d58c

1 files changed, 163 insertions, 114 deletions

diff --git a/lib/sqlalchemy/sql/crud.py b/lib/sqlalchemy/sql/crud.py
index 3bf8a7c62..986f63aad 100644
--- a/lib/sqlalchemy/sql/crud.py
+++ b/lib/sqlalchemy/sql/crud.py
@@ -335,7 +335,6 @@ def _scan_cols(
     values,
     kw,
 ):
-
     (
         need_pks,
         implicit_returning,
@@ -358,9 +357,12 @@ def _scan_cols(
         cols = stmt.table.columns
 
     for c in cols:
+        # scan through every column in the target table
+
         col_key = _getattr_col_key(c)
 
         if col_key in parameters and col_key not in check_columns:
+            # parameter is present for the column.  use that.
 
             _append_param_parameter(
                 compiler,
@@ -377,30 +379,43 @@ def _scan_cols(
             )
 
         elif compile_state.isinsert:
-            if (
-                c.primary_key
-                and need_pks
-                and (
-                    implicit_returning
-                    or not postfetch_lastrowid
-                    or c is not stmt.table._autoincrement_column
-                )
-            ):
+            # no parameter is present and it's an insert.
+
+            if c.primary_key and need_pks:
+                # it's a primary key column, it will need to be generated by a
+                # default generator of some kind, and the statement expects
+                # inserted_primary_key to be available.
 
                 if implicit_returning:
+                    # we can use RETURNING, find out how to invoke this
+                    # column and get the value where RETURNING is an option.
+                    # we can inline server-side functions in this case.
+
                     _append_param_insert_pk_returning(
                         compiler, stmt, c, values, kw
                     )
                 else:
-                    _append_param_insert_pk(compiler, stmt, c, values, kw)
+                    # otherwise, find out how to invoke this column
+                    # and get its value where RETURNING is not an option.
+                    # if we have to invoke a server-side function, we need
+                    # to pre-execute it.   or if this is a straight
+                    # autoincrement column and the dialect supports it
+                    # we can use curosr.lastrowid.
+
+                    _append_param_insert_pk_no_returning(
+                        compiler, stmt, c, values, kw
+                    )
 
             elif c.default is not None:
-
+                # column has a default, but it's not a pk column, or it is but
+                # we don't need to get the pk back.
                 _append_param_insert_hasdefault(
                     compiler, stmt, c, implicit_return_defaults, values, kw
                 )
 
             elif c.server_default is not None:
+                # column has a DDL-level default, and is either not a pk
+                # column or we don't need the pk.
                 if implicit_return_defaults and c in implicit_return_defaults:
                     compiler.returning.append(c)
                 elif not c.primary_key:
@@ -415,6 +430,8 @@ def _scan_cols(
                 _warn_pk_with_no_anticipated_value(c)
 
         elif compile_state.isupdate:
+            # no parameter is present and it's an insert.
+
             _append_param_update(
                 compiler,
                 compile_state,
@@ -468,38 +485,42 @@ def _append_param_parameter(
             **kw
         )
     else:
-        if c.primary_key and implicit_returning:
-            compiler.returning.append(c)
-            value = compiler.process(value.self_group(), **kw)
-        elif implicit_return_defaults and c in implicit_return_defaults:
-            compiler.returning.append(c)
-            value = compiler.process(value.self_group(), **kw)
+        # value is a SQL expression
+        value = compiler.process(value.self_group(), **kw)
+
+        if compile_state.isupdate:
+            if implicit_return_defaults and c in implicit_return_defaults:
+                compiler.returning.append(c)
+
+            else:
+                compiler.postfetch.append(c)
         else:
-            # postfetch specifically means, "we can SELECT the row we just
-            # inserted by primary key to get back the server generated
-            # defaults". so by definition this can't be used to get the primary
-            # key value back, because we need to have it ahead of time.
-            if not c.primary_key:
+            if c.primary_key:
+
+                if implicit_returning:
+                    compiler.returning.append(c)
+                elif compiler.dialect.postfetch_lastrowid:
+                    compiler.postfetch_lastrowid = True
+
+            elif implicit_return_defaults and c in implicit_return_defaults:
+                compiler.returning.append(c)
+
+            else:
+                # postfetch specifically means, "we can SELECT the row we just
+                # inserted by primary key to get back the server generated
+                # defaults". so by definition this can't be used to get the
+                # primary key value back, because we need to have it ahead of
+                # time.
+
                 compiler.postfetch.append(c)
-            value = compiler.process(value.self_group(), **kw)
+
     values.append((c, col_value, value))
 
 
 def _append_param_insert_pk_returning(compiler, stmt, c, values, kw):
-    """Create a primary key expression in the INSERT statement and
-    possibly a RETURNING clause for it.
-
-    If the column has a Python-side default, we will create a bound
-    parameter for it and "pre-execute" the Python function.  If
-    the column has a SQL expression default, or is a sequence,
-    we will add it directly into the INSERT statement and add a
-    RETURNING element to get the new value.  If the column has a
-    server side default or is marked as the "autoincrement" column,
-    we will add a RETRUNING element to get at the value.
-
-    If all the above tests fail, that indicates a primary key column with no
-    noted default generation capabilities that has no parameter passed;
-    raise an exception.
+    """Create a primary key expression in the INSERT statement where
+    we want to populate result.inserted_primary_key and RETURNING
+    is available.
 
     """
     if c.default is not None:
@@ -526,6 +547,9 @@ def _append_param_insert_pk_returning(compiler, stmt, c, values, kw):
             )
             compiler.returning.append(c)
         else:
+            # client side default.  OK we can't use RETURNING, need to
+            # do a "prefetch", which in fact fetches the default value
+            # on the Python side
             values.append(
                 (
                     c,
@@ -541,78 +565,15 @@ def _append_param_insert_pk_returning(compiler, stmt, c, values, kw):
         _warn_pk_with_no_anticipated_value(c)
 
 
-def _create_insert_prefetch_bind_param(
-    compiler, c, process=True, name=None, **kw
-):
-    param = _create_bind_param(
-        compiler, c, None, process=process, name=name, **kw
-    )
-    compiler.insert_prefetch.append(c)
-    return param
+def _append_param_insert_pk_no_returning(compiler, stmt, c, values, kw):
+    """Create a primary key expression in the INSERT statement where
+    we want to populate result.inserted_primary_key and we cannot use
+    RETURNING.
 
+    Depending on the kind of default here we may create a bound parameter
+    in the INSERT statement and pre-execute a default generation function,
+    or we may use cursor.lastrowid if supported by the dialect.
 
-def _create_update_prefetch_bind_param(
-    compiler, c, process=True, name=None, **kw
-):
-    param = _create_bind_param(
-        compiler, c, None, process=process, name=name, **kw
-    )
-    compiler.update_prefetch.append(c)
-    return param
-
-
-class _multiparam_column(elements.ColumnElement):
-    _is_multiparam_column = True
-
-    def __init__(self, original, index):
-        self.index = index
-        self.key = "%s_m%d" % (original.key, index + 1)
-        self.original = original
-        self.default = original.default
-        self.type = original.type
-
-    def compare(self, other, **kw):
-        raise NotImplementedError()
-
-    def _copy_internals(self, other, **kw):
-        raise NotImplementedError()
-
-    def __eq__(self, other):
-        return (
-            isinstance(other, _multiparam_column)
-            and other.key == self.key
-            and other.original == self.original
-        )
-
-
-def _process_multiparam_default_bind(compiler, stmt, c, index, kw):
-
-    if not c.default:
-        raise exc.CompileError(
-            "INSERT value for column %s is explicitly rendered as a bound"
-            "parameter in the VALUES clause; "
-            "a Python-side value or SQL expression is required" % c
-        )
-    elif c.default.is_clause_element:
-        return compiler.process(c.default.arg.self_group(), **kw)
-    else:
-        col = _multiparam_column(c, index)
-        if isinstance(stmt, dml.Insert):
-            return _create_insert_prefetch_bind_param(compiler, col, **kw)
-        else:
-            return _create_update_prefetch_bind_param(compiler, col, **kw)
-
-
-def _append_param_insert_pk(compiler, stmt, c, values, kw):
-    """Create a bound parameter in the INSERT statement to receive a
-    'prefetched' default value.
-
-    The 'prefetched' value indicates that we are to invoke a Python-side
-    default function or expliclt SQL expression before the INSERT statement
-    proceeds, so that we have a primary key value available.
-
-    if the column has no noted default generation capabilities, it has
-    no value passed in either; raise an exception.
 
     """
 
@@ -635,12 +596,27 @@ def _append_param_insert_pk(compiler, stmt, c, values, kw):
         # column is the "autoincrement column"
         c is stmt.table._autoincrement_column
         and (
-            # and it's either a "sequence" or a
-            # pre-executable "autoincrement" sequence
-            compiler.dialect.supports_sequences
-            or compiler.dialect.preexecute_autoincrement_sequences
+            # dialect can't use cursor.lastrowid
+            not compiler.dialect.postfetch_lastrowid
+            and (
+                # column has a Sequence and we support those
+                (
+                    c.default is not None
+                    and c.default.is_sequence
+                    and compiler.dialect.supports_sequences
+                )
+                or
+                # column has no default on it, but dialect can run the
+                # "autoincrement" mechanism explictly, e.g. PostrgreSQL
+                # SERIAL we know the sequence name
+                (
+                    c.default is None
+                    and compiler.dialect.preexecute_autoincrement_sequences
+                )
+            )
         )
     ):
+        # do a pre-execute of the default
         values.append(
             (
                 c,
@@ -648,16 +624,26 @@ def _append_param_insert_pk(compiler, stmt, c, values, kw):
                 _create_insert_prefetch_bind_param(compiler, c, **kw),
             )
         )
-    elif c.default is None and c.server_default is None and not c.nullable:
+    elif (
+        c.default is None
+        and c.server_default is None
+        and not c.nullable
+        and c is not stmt.table._autoincrement_column
+    ):
         # no .default, no .server_default, not autoincrement, we have
         # no indication this primary key column will have any value
         _warn_pk_with_no_anticipated_value(c)
+    elif compiler.dialect.postfetch_lastrowid:
+        # finally, where it seems like there will be a generated primary key
+        # value and we haven't set up any other way to fetch it, and the
+        # dialect supports cursor.lastrowid, switch on the lastrowid flag so
+        # that the DefaultExecutionContext calls upon cursor.lastrowid
+        compiler.postfetch_lastrowid = True
 
 
 def _append_param_insert_hasdefault(
     compiler, stmt, c, implicit_return_defaults, values, kw
 ):
-
     if c.default.is_sequence:
         if compiler.dialect.supports_sequences and (
             not c.default.optional or not compiler.dialect.sequences_optional
@@ -765,6 +751,69 @@ def _append_param_update(
         compiler.returning.append(c)
 
 
+def _create_insert_prefetch_bind_param(
+    compiler, c, process=True, name=None, **kw
+):
+
+    param = _create_bind_param(
+        compiler, c, None, process=process, name=name, **kw
+    )
+    compiler.insert_prefetch.append(c)
+    return param
+
+
+def _create_update_prefetch_bind_param(
+    compiler, c, process=True, name=None, **kw
+):
+    param = _create_bind_param(
+        compiler, c, None, process=process, name=name, **kw
+    )
+    compiler.update_prefetch.append(c)
+    return param
+
+
+class _multiparam_column(elements.ColumnElement):
+    _is_multiparam_column = True
+
+    def __init__(self, original, index):
+        self.index = index
+        self.key = "%s_m%d" % (original.key, index + 1)
+        self.original = original
+        self.default = original.default
+        self.type = original.type
+
+    def compare(self, other, **kw):
+        raise NotImplementedError()
+
+    def _copy_internals(self, other, **kw):
+        raise NotImplementedError()
+
+    def __eq__(self, other):
+        return (
+            isinstance(other, _multiparam_column)
+            and other.key == self.key
+            and other.original == self.original
+        )
+
+
+def _process_multiparam_default_bind(compiler, stmt, c, index, kw):
+
+    if not c.default:
+        raise exc.CompileError(
+            "INSERT value for column %s is explicitly rendered as a bound"
+            "parameter in the VALUES clause; "
+            "a Python-side value or SQL expression is required" % c
+        )
+    elif c.default.is_clause_element:
+        return compiler.process(c.default.arg.self_group(), **kw)
+    else:
+        col = _multiparam_column(c, index)
+        if isinstance(stmt, dml.Insert):
+            return _create_insert_prefetch_bind_param(compiler, col, **kw)
+        else:
+            return _create_update_prefetch_bind_param(compiler, col, **kw)
+
+
 def _get_multitable_params(
     compiler,
     stmt,