From: Federico Caselli Date: Wed, 21 Apr 2021 20:49:09 +0000 (+0200) Subject: Propertly ignore ``Identity`` in MySQL and MariaDb. X-Git-Tag: rel_1_4_12~8^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d3c73ad8012e15bf47529b3fcb0bac1298fbdb90;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Propertly ignore ``Identity`` in MySQL and MariaDb. Ensure that the MySQL and MariaDB dialect ignore the :class:`_sql.Identity` construct while rendering the ``AUTO_INCREMENT`` keyword in a create table. The Oracle and PostgreSQL compiler was updated to not render :class:`_sql.Identity` if the database version does not support it (Oracle < 12 and PostgreSQL < 10). Previously it was rendered regardless of the database version. Fixes: #6338 Change-Id: I2ca0902fdd7b4be4fc1a563cf5585504cbea9360 --- diff --git a/doc/build/changelog/unreleased_14/6338.rst b/doc/build/changelog/unreleased_14/6338.rst new file mode 100644 index 0000000000..2ad45db25c --- /dev/null +++ b/doc/build/changelog/unreleased_14/6338.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, schema, mysql, mariadb, oracle, postgresql + :tickets: 6338 + + Ensure that the MySQL and MariaDB dialect ignore the + :class:`_sql.Identity` construct while rendering the ``AUTO_INCREMENT`` + keyword in a create table. + + The Oracle and PostgreSQL compiler was updated to not render + :class:`_sql.Identity` if the database version does not support it + (Oracle < 12 and PostgreSQL < 10). Previously it was rendered regardless + of the database version. diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py index d4c70a78e6..88c03a0113 100644 --- a/lib/sqlalchemy/dialects/mysql/base.py +++ b/lib/sqlalchemy/dialects/mysql/base.py @@ -1933,7 +1933,10 @@ class MySQLDDLCompiler(compiler.DDLCompiler): if ( column.table is not None and column is column.table._autoincrement_column - and column.server_default is None + and ( + column.server_default is None + or isinstance(column.server_default, sa_schema.Identity) + ) and not ( self.dialect.supports_sequences and isinstance(column.default, sa_schema.Sequence) diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py index 9571eb46f0..57f55c1824 100644 --- a/lib/sqlalchemy/dialects/oracle/base.py +++ b/lib/sqlalchemy/dialects/oracle/base.py @@ -1457,6 +1457,7 @@ class OracleDialect(default.DefaultDialect): supports_default_values = False supports_default_metavalue = True supports_empty_insert = False + supports_identity_columns = True statement_compiler = OracleCompiler ddl_compiler = OracleDDLCompiler @@ -1513,6 +1514,8 @@ class OracleDialect(default.DefaultDialect): self.colspecs.pop(sqltypes.Interval) self.use_ansi = False + self.supports_identity_columns = self.server_version_info >= (12,) + def _get_effective_compat_server_version_info(self, connection): # dialect does not need compat levels below 12.2, so don't query # in those cases diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index 47a933479f..26b025b1a4 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -2464,6 +2464,11 @@ class PGDDLCompiler(compiler.DDLCompiler): if isinstance(impl_type, sqltypes.TypeDecorator): impl_type = impl_type.impl + has_identity = ( + column.identity is not None + and self.dialect.supports_identity_columns + ) + if ( column.primary_key and column is column.table._autoincrement_column @@ -2471,7 +2476,7 @@ class PGDDLCompiler(compiler.DDLCompiler): self.dialect.supports_smallserial or not isinstance(impl_type, sqltypes.SmallInteger) ) - and column.identity is None + and not has_identity and ( column.default is None or ( @@ -2498,12 +2503,12 @@ class PGDDLCompiler(compiler.DDLCompiler): if column.computed is not None: colspec += " " + self.process(column.computed) - if column.identity is not None: + if has_identity: colspec += " " + self.process(column.identity) - if not column.nullable and not column.identity: + if not column.nullable and not has_identity: colspec += " NOT NULL" - elif column.nullable and column.identity: + elif column.nullable and has_identity: colspec += " NULL" return colspec @@ -3086,6 +3091,8 @@ class PGDialect(default.DefaultDialect): supports_empty_insert = False supports_multivalues_insert = True + supports_identity_columns = True + default_paramstyle = "pyformat" ischema_names = ischema_names colspecs = colspecs @@ -3193,6 +3200,7 @@ class PGDialect(default.DefaultDialect): 9, 2, ) + self.supports_identity_columns = self.server_version_info >= (10,) def on_connect(self): if self.isolation_level is not None: diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index a917228adb..aa7e4e5e9d 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -74,6 +74,7 @@ class DefaultDialect(interfaces.Dialect): supports_sequences = False sequences_optional = False preexecute_autoincrement_sequences = False + supports_identity_columns = False postfetch_lastrowid = True implicit_returning = False full_returning = False @@ -808,6 +809,8 @@ class StrCompileDialect(DefaultDialect): supports_statement_cache = True + supports_identity_columns = True + supports_sequences = True sequences_optional = True preexecute_autoincrement_sequences = False diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 6168248ff7..bd93f5199e 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -4257,10 +4257,15 @@ class DDLCompiler(Compiled): if column.computed is not None: colspec += " " + self.process(column.computed) - if column.identity is not None: + if ( + column.identity is not None + and self.dialect.supports_identity_columns + ): colspec += " " + self.process(column.identity) - if not column.nullable and not column.identity: + if not column.nullable and ( + not column.identity or not self.dialect.supports_identity_columns + ): colspec += " NOT NULL" return colspec diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py index 8a70cc6924..673fa15cdd 100644 --- a/lib/sqlalchemy/testing/requirements.py +++ b/lib/sqlalchemy/testing/requirements.py @@ -1417,3 +1417,10 @@ class SuiteRequirements(Requirements): or ties. basically this is "not mssql" """ return exclusions.closed() + + @property + def autoincrement_without_sequence(self): + """If autoincrement=True on a column does not require an explicit + sequence. This should be false only for oracle. + """ + return exclusions.open() diff --git a/lib/sqlalchemy/testing/suite/test_select.py b/lib/sqlalchemy/testing/suite/test_select.py index 7b35dc3fa3..8f34129299 100644 --- a/lib/sqlalchemy/testing/suite/test_select.py +++ b/lib/sqlalchemy/testing/suite/test_select.py @@ -1442,6 +1442,31 @@ class IdentityColumnTest(fixtures.TablesTest): assert_raises((DatabaseError, ProgrammingError), fn) +class IdentityAutoincrementTest(fixtures.TablesTest): + __backend__ = True + __requires__ = ("autoincrement_without_sequence",) + + @classmethod + def define_tables(cls, metadata): + Table( + "tbl", + metadata, + Column( + "id", + Integer, + Identity(), + primary_key=True, + autoincrement=True, + ), + Column("desc", String(100)), + ) + + def test_autoincrement_with_identity(self, connection): + res = connection.execute(self.tables.tbl.insert(), {"desc": "row"}) + res = connection.execute(self.tables.tbl.select()).first() + eq_(res, (1, "row")) + + class ExistsTest(fixtures.TablesTest): __backend__ = True diff --git a/test/dialect/oracle/test_compiler.py b/test/dialect/oracle/test_compiler.py index 5e9f46e1a4..e198fa48a9 100644 --- a/test/dialect/oracle/test_compiler.py +++ b/test/dialect/oracle/test_compiler.py @@ -1333,6 +1333,17 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): "CREATE TABLE t (y INTEGER GENERATED %s AS IDENTITY)" % text, ) + def test_column_identity_not_supported(self): + m = MetaData() + t = Table("t", m, Column("y", Integer, Identity(always=None))) + dd = oracle.OracleDialect() + dd.supports_identity_columns = False + self.assert_compile( + schema.CreateTable(t), + "CREATE TABLE t (y INTEGER NOT NULL)", + dialect=dd, + ) + class SequenceTest(fixtures.TestBase, AssertsCompiledSQL): def test_basic(self): diff --git a/test/dialect/postgresql/test_compiler.py b/test/dialect/postgresql/test_compiler.py index 4b2004a5ff..a517ad1ac0 100644 --- a/test/dialect/postgresql/test_compiler.py +++ b/test/dialect/postgresql/test_compiler.py @@ -1857,18 +1857,50 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): dialect=postgresql.dialect(), ) - def test_column_identity(self): + @testing.combinations(True, False) + def test_column_identity(self, pk): # all other tests are in test_identity_column.py m = MetaData() t = Table( "t", m, - Column("y", Integer, Identity(always=True, start=4, increment=7)), + Column( + "y", + Integer, + Identity(always=True, start=4, increment=7), + primary_key=pk, + ), ) self.assert_compile( schema.CreateTable(t), "CREATE TABLE t (y INTEGER GENERATED ALWAYS AS IDENTITY " - "(INCREMENT BY 7 START WITH 4))", + "(INCREMENT BY 7 START WITH 4)%s)" + % (", PRIMARY KEY (y)" if pk else ""), + ) + + @testing.combinations(True, False) + def test_column_identity_no_support(self, pk): + m = MetaData() + t = Table( + "t", + m, + Column( + "y", + Integer, + Identity(always=True, start=4, increment=7), + primary_key=pk, + ), + ) + dd = PGDialect() + dd.supports_identity_columns = False + self.assert_compile( + schema.CreateTable(t), + "CREATE TABLE t (y %s%s)" + % ( + "SERIAL NOT NULL" if pk else "INTEGER NOT NULL", + ", PRIMARY KEY (y)" if pk else "", + ), + dialect=dd, ) def test_column_identity_null(self): diff --git a/test/requirements.py b/test/requirements.py index 98dca61249..6628d9ef38 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -1776,3 +1776,7 @@ class DefaultRequirements(SuiteRequirements): @property def fetch_offset_with_options(self): return skip_if("mssql") + + @property + def autoincrement_without_sequence(self): + return skip_if("oracle") diff --git a/test/sql/test_identity_column.py b/test/sql/test_identity_column.py index 1ce15f38c9..00404dae79 100644 --- a/test/sql/test_identity_column.py +++ b/test/sql/test_identity_column.py @@ -1,3 +1,5 @@ +import re + from sqlalchemy import Column from sqlalchemy import Identity from sqlalchemy import Integer @@ -5,6 +7,7 @@ from sqlalchemy import MetaData from sqlalchemy import Sequence from sqlalchemy import Table from sqlalchemy import testing +from sqlalchemy.engine import URL from sqlalchemy.exc import ArgumentError from sqlalchemy.schema import CreateTable from sqlalchemy.testing import assert_raises_message @@ -63,9 +66,9 @@ class _IdentityDDLFixture(testing.AssertsCompiledSQL): ) def test_create_ddl(self, identity_args, text): - if getattr(self, "__dialect__", None) != "default" and testing.against( - "oracle" - ): + if getattr( + self, "__dialect__", None + ) != "default_enhanced" and testing.against("oracle"): text = text.replace("NO MINVALUE", "NOMINVALUE") text = text.replace("NO MAXVALUE", "NOMAXVALUE") text = text.replace("NO CYCLE", "NOCYCLE") @@ -138,9 +141,9 @@ class _IdentityDDLFixture(testing.AssertsCompiledSQL): is_(t.c.c.nullable, False) nullable = "" - if getattr(self, "__dialect__", None) != "default" and testing.against( - "postgresql" - ): + if getattr( + self, "__dialect__", None + ) != "default_enhanced" and testing.against("postgresql"): nullable = " NULL" self.assert_compile( @@ -183,22 +186,67 @@ class IdentityDDL(_IdentityDDLFixture, fixtures.TestBase): class DefaultDialectIdentityDDL(_IdentityDDLFixture, fixtures.TestBase): # this uses the default dialect - __dialect__ = "default" + __dialect__ = "default_enhanced" class NotSupportingIdentityDDL(testing.AssertsCompiledSQL, fixtures.TestBase): - # a dialect that doesn't render IDENTITY - __dialect__ = "sqlite" + def get_dialect(self, dialect): + dd = URL.create(dialect).get_dialect()() + if dialect in {"oracle", "postgresql"}: + dd.supports_identity_columns = False + return dd + + @testing.combinations("sqlite", "mysql", "mariadb", "postgresql", "oracle") + def test_identity_is_ignored(self, dialect): - @testing.skip_if(testing.requires.identity_columns) - def test_identity_is_ignored(self): t = Table( "foo_table", MetaData(), Column("foo", Integer(), Identity("always", start=3)), ) + t_exp = Table( + "foo_table", + MetaData(), + Column("foo", Integer(), nullable=False), + ) + dialect = self.get_dialect(dialect) + exp = CreateTable(t_exp).compile(dialect=dialect).string + self.assert_compile( + CreateTable(t), re.sub(r"[\n\t]", "", exp), dialect=dialect + ) + + @testing.combinations( + "sqlite", + "mysql", + "mariadb", + "postgresql", + "oracle", + argnames="dialect", + ) + @testing.combinations(True, "auto", argnames="autoincrement") + def test_identity_is_ignored_in_pk(self, dialect, autoincrement): + t = Table( + "foo_table", + MetaData(), + Column( + "foo", + Integer(), + Identity("always", start=3), + primary_key=True, + autoincrement=autoincrement, + ), + ) + t_exp = Table( + "foo_table", + MetaData(), + Column( + "foo", Integer(), primary_key=True, autoincrement=autoincrement + ), + ) + dialect = self.get_dialect(dialect) + exp = CreateTable(t_exp).compile(dialect=dialect).string self.assert_compile( - CreateTable(t), "CREATE TABLE foo_table (foo INTEGER NOT NULL)" + CreateTable(t), re.sub(r"[\n\t]", "", exp), dialect=dialect )