From: Gord Thompson Date: Thu, 21 Nov 2019 14:43:40 +0000 (-0500) Subject: Add sequence support for MariaDB 10.3+. X-Git-Tag: rel_1_4_0b1~610^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6f99bdf013f3a0637f0544c4c3daeac0392553d6;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 --- diff --git a/doc/build/changelog/migration_14.rst b/doc/build/changelog/migration_14.rst index 7161867ac1..41617de2ac 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 0000000000..ec8ae89eca --- /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 7a52f61779..0f97556492 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 fb123bc0f2..81e80fdbc4 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 881ea9fcda..58abc10df2 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 8c325538c6..c7d699a5ff 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 e2237fb171..bc7a6120c3 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 5b26ac72e2..8bbb7ec45d 100644 --- a/lib/sqlalchemy/testing/requirements.py +++ b/lib/sqlalchemy/testing/requirements.py @@ -430,6 +430,13 @@ class SuiteRequirements(Requirements): "no sequence support", ) + @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 @@ -443,6 +450,24 @@ class SuiteRequirements(Requirements): "no sequence support, or sequences not optional", ) + @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 652cea3f35..b71eb88378 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 ed7af2572e..f033abab23 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", )