Add sequence support for MariaDB 10.3+.

Added support for use of the :class:`.Sequence` construct with MariaDB 10.3
and greater, as this is now supported by this database.  The construct
integrates with the :class:`.Table` object in the same way that it does for
other databases like PostrgreSQL and Oracle; if is present on the integer
primary key "autoincrement" column, it is used to generate defaults.   For
backwards compatibility, to support a :class:`.Table` that has a
:class:`.Sequence` on it to support sequence only databases like Oracle,
while still not having the sequence fire off for MariaDB, the optional=True
flag should be set, which indicates the sequence should only be used to
generate the primary key if the target database offers no other option.

Fixes: #4976
Closes: #4996
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/4996
Pull-request-sha: cb2e1426ea0b6bc6c93dbe8f033a11df9d8c4915

Change-Id: I507bc405eee6cae2c5991345d0eac53a37fe7512

10 files changed, 225 insertions, 28 deletions

diff --git a/doc/build/changelog/migration_14.rst b/doc/build/changelog/migration_14.rst
index 7161867ac..41617de2a 100644
--- a/doc/build/changelog/migration_14.rst
+++ b/doc/build/changelog/migration_14.rst
@@ -887,3 +887,48 @@ import the "pysqlite2" driver on Python 3 as this driver does not exist
 on Python 3; a very old warning for old pysqlite2 versions is also dropped.
 
 :ticket:`4895`
+
+
+.. _change_4976:
+
+Added Sequence support for MariaDB 10.3
+----------------------------------------
+
+The MariaDB database as of 10.3 supports sequences.   SQLAlchemy's MySQL
+dialect now implements support for the :class:`.Sequence` object against this
+database, meaning "CREATE SEQUENCE" DDL will be emitted for a
+:class:`.Sequence` that is present in a :class:`.Table` or :class:`.MetaData`
+collection in the same way as it works for backends such as PostgreSQL, Oracle,
+when the dialect's server version check has confirmed the database is MariaDB
+10.3 or greater.    Additionally, the :class:`.Sequence` will act as a
+column default and primary key generation object when used in these ways.
+
+Since this change will impact the assumptions both for DDL as well as the
+behavior of INSERT statements for an application that is currently deployed
+against MariaDB 10.3 which also happens to make explicit use the
+:class:`.Sequence` construct within its table definitions, it is important to
+note that :class:`.Sequence` supports a flag :paramref:`.Sequence.optional`
+which is used to limit the scenarios in which the :class:`.Sequence` to take
+effect. When "optional" is used on a :class:`.Sequence` that is present in the
+integer primary key column of a table::
+
+    Table(
+        "some_table", metadata,
+        Column("id", Integer, Sequence("some_seq", optional=True), primary_key=True)
+    )
+
+The above :class:`.Sequence` is only used for DDL and INSERT statements if the
+target database does not support any other means of generating integer primary
+key values for the column.  That is, the Oracle database above would use the
+sequence, however the PostgreSQL and MariaDB 10.3 databases would not. This may
+be important for an existing application that is upgrading to SQLAlchemy 1.4
+which may not have emitted DDL for this :class:`.Sequence` against its backing
+database, as an INSERT statement will fail if it seeks to use a sequence that
+was not created.
+
+
+.. seealso::
+
+    :ref:`defaults_sequences`
+
+:ticket:`4976`
\ No newline at end of file
diff --git a/doc/build/changelog/unreleased_14/4976.rst b/doc/build/changelog/unreleased_14/4976.rst
new file mode 100644
index 000000000..ec8ae89ec
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/4976.rst
@@ -0,0 +1,19 @@
+.. change::
+    :tags: mysql, usecase
+    :tickets: 4976
+
+    Added support for use of the :class:`.Sequence` construct with MariaDB 10.3
+    and greater, as this is now supported by this database.  The construct
+    integrates with the :class:`.Table` object in the same way that it does for
+    other databases like PostrgreSQL and Oracle; if is present on the integer
+    primary key "autoincrement" column, it is used to generate defaults.   For
+    backwards compatibility, to support a :class:`.Table` that has a
+    :class:`.Sequence` on it to support sequence only databases like Oracle,
+    while still not having the sequence fire off for MariaDB, the optional=True
+    flag should be set, which indicates the sequence should only be used to
+    generate the primary key if the target database offers no other option.
+
+    .. seealso::
+
+        :ref:`change_4976`
+
diff --git a/doc/build/core/defaults.rst b/doc/build/core/defaults.rst
index 7a52f6177..0f9755649 100644
--- a/doc/build/core/defaults.rst
+++ b/doc/build/core/defaults.rst
@@ -316,16 +316,17 @@ For details on using :class:`.FetchedValue` with the ORM, see
 
     :ref:`orm_server_defaults`
 
+.. _defaults_sequences:
 
 Defining Sequences
 ------------------
 
 SQLAlchemy represents database sequences using the
 :class:`~sqlalchemy.schema.Sequence` object, which is considered to be a
-special case of "column default". It only has an effect on databases which
-have explicit support for sequences, which currently includes PostgreSQL,
-Oracle, and Firebird. The :class:`~sqlalchemy.schema.Sequence` object is
-otherwise ignored.
+special case of "column default". It only has an effect on databases which have
+explicit support for sequences, which currently includes PostgreSQL, Oracle,
+MariaDB 10.3 or greater, and Firebird. The :class:`~sqlalchemy.schema.Sequence`
+object is otherwise ignored.
 
 The :class:`~sqlalchemy.schema.Sequence` may be placed on any column as a
 "default" generator to be used during INSERT operations, and can also be
diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py
index fb123bc0f..81e80fdbc 100644
--- a/lib/sqlalchemy/dialects/mysql/base.py
+++ b/lib/sqlalchemy/dialects/mysql/base.py
@@ -1192,6 +1192,15 @@ class MySQLExecutionContext(default.DefaultExecutionContext):
         else:
             raise NotImplementedError()
 
+    def fire_sequence(self, seq, type_):
+        return self._execute_scalar(
+            (
+                "select nextval(%s)"
+                % self.dialect.identifier_preparer.format_sequence(seq)
+            ),
+            type_,
+        )
+
 
 class MySQLCompiler(compiler.SQLCompiler):
 
@@ -1204,6 +1213,9 @@ class MySQLCompiler(compiler.SQLCompiler):
     def visit_random_func(self, fn, **kw):
         return "rand%s" % self.function_argspec(fn)
 
+    def visit_sequence(self, seq, **kw):
+        return "nextval(%s)" % self.preparer.format_sequence(seq)
+
     def visit_sysdate_func(self, fn, **kw):
         return "SYSDATE()"
 
@@ -2146,6 +2158,11 @@ class MySQLDialect(default.DefaultDialect):
 
     supports_native_enum = True
 
+    supports_sequences = False  # default for MySQL ...
+    # ... may be updated to True for MariaDB 10.3+ in initialize()
+
+    sequences_optional = True
+
     supports_sane_rowcount = True
     supports_sane_multi_rowcount = False
     supports_multivalues_insert = True
@@ -2421,6 +2438,22 @@ class MySQLDialect(default.DefaultDialect):
             if rs:
                 rs.close()
 
+    def has_sequence(self, connection, sequence_name, schema=None):
+        if not schema:
+            schema = self.default_schema_name
+        # MariaDB implements sequences as a special type of table
+        #
+        cursor = connection.execute(
+            sql.text(
+                "SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES "
+                "WHERE TABLE_NAME=:name AND "
+                "TABLE_SCHEMA=:schema_name"
+            ),
+            name=sequence_name,
+            schema_name=schema,
+        )
+        return cursor.first() is not None
+
     def initialize(self, connection):
         self._connection_charset = self._detect_charset(connection)
         self._detect_sql_mode(connection)
@@ -2435,6 +2468,10 @@ class MySQLDialect(default.DefaultDialect):
 
         default.DefaultDialect.initialize(self, connection)
 
+        self.supports_sequences = (
+            self._is_mariadb and self.server_version_info >= (10, 3)
+        )
+
         self._needs_correct_for_88718_96365 = (
             not self._is_mariadb and self.server_version_info >= (8,)
         )
diff --git a/lib/sqlalchemy/sql/crud.py b/lib/sqlalchemy/sql/crud.py
index 881ea9fcd..58abc10df 100644
--- a/lib/sqlalchemy/sql/crud.py
+++ b/lib/sqlalchemy/sql/crud.py
@@ -534,13 +534,21 @@ def _append_param_insert_pk(compiler, stmt, c, values, kw):
     no value passed in either; raise an exception.
 
     """
+
     if (
         # column has a Python-side default
         c.default is not None
         and (
-            # and it won't be a Sequence
+            # and it either is not a sequence, or it is and we support
+            # sequences and want to invoke it
             not c.default.is_sequence
-            or compiler.dialect.supports_sequences
+            or (
+                compiler.dialect.supports_sequences
+                and (
+                    not c.default.optional
+                    or not compiler.dialect.sequences_optional
+                )
+            )
         )
     ) or (
         # column is the "autoincrement column"
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 8c325538c..c7d699a5f 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -2281,6 +2281,8 @@ class Sequence(roles.StatementRole, DefaultGenerator):
 
     .. seealso::
 
+        :ref:`defaults_sequences`
+
         :class:`.CreateSequence`
 
         :class:`.DropSequence`
diff --git a/lib/sqlalchemy/testing/fixtures.py b/lib/sqlalchemy/testing/fixtures.py
index e2237fb17..bc7a6120c 100644
--- a/lib/sqlalchemy/testing/fixtures.py
+++ b/lib/sqlalchemy/testing/fixtures.py
@@ -120,16 +120,11 @@ class TablesTest(TestBase):
 
     def _setup_each_tables(self):
         if self.run_define_tables == "each":
-            self.tables.clear()
-            if self.run_create_tables == "each":
-                drop_all_tables(self.metadata, self.bind)
-            self.metadata.clear()
             self.define_tables(self.metadata)
             if self.run_create_tables == "each":
                 self.metadata.create_all(self.bind)
             self.tables.update(self.metadata.tables)
         elif self.run_create_tables == "each":
-            drop_all_tables(self.metadata, self.bind)
             self.metadata.create_all(self.bind)
 
     def _setup_each_inserts(self):
@@ -138,6 +133,14 @@ class TablesTest(TestBase):
             self.insert_data()
 
     def _teardown_each_tables(self):
+        if self.run_define_tables == "each":
+            self.tables.clear()
+            if self.run_create_tables == "each":
+                drop_all_tables(self.metadata, self.bind)
+            self.metadata.clear()
+        elif self.run_create_tables == "each":
+            drop_all_tables(self.metadata, self.bind)
+
         # no need to run deletes if tables are recreated on setup
         if self.run_define_tables != "each" and self.run_deletes == "each":
             with self.bind.connect() as conn:
diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py
index 5b26ac72e..8bbb7ec45 100644
--- a/lib/sqlalchemy/testing/requirements.py
+++ b/lib/sqlalchemy/testing/requirements.py
@@ -431,6 +431,13 @@ class SuiteRequirements(Requirements):
         )
 
     @property
+    def no_sequences(self):
+        """the oppopsite of "sequences", DB does not support sequences at
+        all."""
+
+        return exclusions.NotPredicate(self.sequences)
+
+    @property
     def sequences_optional(self):
         """Target database supports sequences, but also optionally
         as a means of generating new PK values."""
@@ -444,6 +451,24 @@ class SuiteRequirements(Requirements):
         )
 
     @property
+    def supports_lastrowid(self):
+        """target database / driver supports cursor.lastrowid as a means
+        of retrieving the last inserted primary key value.
+
+        note that if the target DB supports sequences also, this is still
+        assumed to work.  This is a new use case brought on by MariaDB 10.3.
+
+        """
+        return exclusions.only_if(
+            [lambda config: config.db.dialect.postfetch_lastrowid]
+        )
+
+    @property
+    def no_lastrowid_support(self):
+        """the opposite of supports_lastrowid"""
+        return exclusions.NotPredicate(self.supports_lastrowid)
+
+    @property
     def reflects_pk_names(self):
         return exclusions.closed()
 
diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index 652cea3f3..b71eb8837 100644
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -1761,16 +1761,23 @@ class EngineEventsTest(fixtures.TestBase):
             implicit_returning=False,
         )
         self.metadata.create_all(engine)
+
         with engine.begin() as conn:
             event.listen(
                 conn, "before_cursor_execute", tracker("cursor_execute")
             )
             conn.execute(t.insert())
-        # we see the sequence pre-executed in the first call
-        assert "t_id_seq" in canary[0][0]
-        assert "INSERT" in canary[1][0]
-        # same context
-        is_(canary[0][1], canary[1][1])
+
+        if testing.requires.supports_lastrowid.enabled:
+            # new MariaDB 10.3 supports sequences + lastrowid; only
+            # one statement
+            assert "INSERT" in canary[0][0]
+        else:
+            # we see the sequence pre-executed in the first call
+            assert "t_id_seq" in canary[0][0]
+            assert "INSERT" in canary[1][0]
+            # same context
+            is_(canary[0][1], canary[1][1])
 
     def test_transactional(self):
         canary = []
diff --git a/test/sql/test_defaults.py b/test/sql/test_defaults.py
index ed7af2572..f033abab2 100644
--- a/test/sql/test_defaults.py
+++ b/test/sql/test_defaults.py
@@ -1147,34 +1147,67 @@ class AutoIncrementTest(fixtures.TablesTest):
         with expect_warnings(".*has no Python-side or server-side default.*"):
             go()
 
-    def test_col_w_sequence_non_autoinc_no_firing(self):
-        metadata = self.metadata
+    @testing.metadata_fixture(ddl="function")
+    def dataset_no_autoinc(self, metadata):
         # plain autoincrement/PK table in the actual schema
         Table("x", metadata, Column("set_id", Integer, primary_key=True))
-        metadata.create_all()
 
         # for the INSERT use a table with a Sequence
         # and autoincrement=False.  Using a ForeignKey
         # would have the same effect
+
+        some_seq = Sequence("some_seq")
+
         dataset_no_autoinc = Table(
             "x",
             MetaData(),
             Column(
                 "set_id",
                 Integer,
-                Sequence("some_seq"),
+                some_seq,
                 primary_key=True,
                 autoincrement=False,
             ),
         )
+        return dataset_no_autoinc
 
-        testing.db.execute(dataset_no_autoinc.insert())
-        eq_(
-            testing.db.scalar(
-                select([func.count("*")]).select_from(dataset_no_autoinc)
-            ),
-            1,
-        )
+    def test_col_w_optional_sequence_non_autoinc_no_firing(
+        self, dataset_no_autoinc
+    ):
+        """this is testing that a Table which includes a Sequence, when
+        run against a DB that does not support sequences, the Sequence
+        does not get in the way.
+
+        """
+        dataset_no_autoinc.c.set_id.default.optional = True
+
+        with testing.db.connect() as conn:
+            conn.execute(dataset_no_autoinc.insert())
+            eq_(
+                conn.scalar(
+                    select([func.count("*")]).select_from(dataset_no_autoinc)
+                ),
+                1,
+            )
+
+    @testing.fails_if(testing.requires.sequences)
+    def test_col_w_nonoptional_sequence_non_autoinc_no_firing(
+        self, dataset_no_autoinc
+    ):
+        """When the sequence is not optional and sequences are supported,
+        the test fails because we didn't create the sequence.
+
+        """
+        dataset_no_autoinc.c.set_id.default.optional = False
+
+        with testing.db.connect() as conn:
+            conn.execute(dataset_no_autoinc.insert())
+            eq_(
+                conn.scalar(
+                    select([func.count("*")]).select_from(dataset_no_autoinc)
+                ),
+                1,
+            )
 
 
 class SequenceDDLTest(fixtures.TestBase, testing.AssertsCompiledSQL):
@@ -1336,8 +1369,24 @@ class SequenceExecTest(fixtures.TestBase):
         testing.db.execute(t1.insert().values(x=s.next_value()))
         self._assert_seq_result(testing.db.scalar(t1.select()))
 
+    @testing.requires.supports_lastrowid
+    @testing.provide_metadata
+    def test_inserted_pk_no_returning_w_lastrowid(self):
+        """test inserted_primary_key contains the pk when
+        pk_col=next_value(), lastrowid is supported."""
+
+        metadata = self.metadata
+        e = engines.testing_engine(options={"implicit_returning": False})
+        s = Sequence("my_sequence")
+        metadata.bind = e
+        t1 = Table("t", metadata, Column("x", Integer, primary_key=True))
+        t1.create()
+        r = e.execute(t1.insert().values(x=s.next_value()))
+        self._assert_seq_result(r.inserted_primary_key[0])
+
+    @testing.requires.no_lastrowid_support
     @testing.provide_metadata
-    def test_inserted_pk_no_returning(self):
+    def test_inserted_pk_no_returning_no_lastrowid(self):
         """test inserted_primary_key contains [None] when
         pk_col=next_value(), implicit returning is not used."""
 
@@ -1397,6 +1446,7 @@ class SequenceTest(fixtures.TestBase, testing.AssertsCompiledSQL):
         for s in (Sequence("my_seq"), Sequence("my_seq", optional=True)):
             assert str(s.next_value().compile(dialect=testing.db.dialect)) in (
                 "nextval('my_seq')",
+                "nextval(my_seq)",
                 "gen_id(my_seq, 1)",
                 "my_seq.nextval",
             )