diff options
-rw-r--r-- | alembic/autogenerate/compare.py | 2 | ||||
-rw-r--r-- | alembic/ddl/mssql.py | 30 | ||||
-rw-r--r-- | alembic/ddl/oracle.py | 29 | ||||
-rw-r--r-- | alembic/ddl/sqlite.py | 4 | ||||
-rw-r--r-- | docs/build/unreleased/1145.rst | 29 | ||||
-rw-r--r-- | tests/test_autogen_diffs.py | 73 |
6 files changed, 156 insertions, 11 deletions
diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index c9971ea..8301e34 100644 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@ -1022,7 +1022,7 @@ def _render_server_default_for_compare( if isinstance(metadata_default, str): if metadata_col.type._type_affinity is sqltypes.String: metadata_default = re.sub(r"^'|'$", "", metadata_default) - return repr(metadata_default) + return f"'{metadata_default}'" else: return metadata_default else: diff --git a/alembic/ddl/mssql.py b/alembic/ddl/mssql.py index 6a208ec..00c5f0e 100644 --- a/alembic/ddl/mssql.py +++ b/alembic/ddl/mssql.py @@ -1,5 +1,6 @@ from __future__ import annotations +import re from typing import Any from typing import List from typing import Optional @@ -230,16 +231,25 @@ class MSSQLImpl(DefaultImpl): rendered_metadata_default, rendered_inspector_default, ): - def clean(value): - if value is not None: - value = value.strip() - while value[0] == "(" and value[-1] == ")": - value = value[1:-1] - return value - - return clean(rendered_inspector_default) != clean( - rendered_metadata_default - ) + if rendered_metadata_default is not None: + rendered_metadata_default = re.sub( + r"^\((.+)\)$", r"\1", rendered_metadata_default + ) + + rendered_metadata_default = re.sub( + r"^\"?'(.+)'\"?$", r"\1", rendered_metadata_default + ) + + if rendered_inspector_default is not None: + rendered_inspector_default = re.sub( + r"^\(+(.+?)\)+$", r"\1", rendered_inspector_default + ) + + rendered_inspector_default = re.sub( + r"^\"?'(.+)'\"?$", r"\1", rendered_inspector_default + ) + + return rendered_inspector_default != rendered_metadata_default def _compare_identity_default(self, metadata_identity, inspector_identity): diff, ignored, is_alter = super()._compare_identity_default( diff --git a/alembic/ddl/oracle.py b/alembic/ddl/oracle.py index 920b70a..9715c1e 100644 --- a/alembic/ddl/oracle.py +++ b/alembic/ddl/oracle.py @@ -1,5 +1,6 @@ from __future__ import annotations +import re from typing import Any from typing import Optional from typing import TYPE_CHECKING @@ -52,6 +53,34 @@ class OracleImpl(DefaultImpl): self.static_output(self.batch_separator) return result + def compare_server_default( + self, + inspector_column, + metadata_column, + rendered_metadata_default, + rendered_inspector_default, + ): + if rendered_metadata_default is not None: + rendered_metadata_default = re.sub( + r"^\((.+)\)$", r"\1", rendered_metadata_default + ) + + rendered_metadata_default = re.sub( + r"^\"?'(.+)'\"?$", r"\1", rendered_metadata_default + ) + + if rendered_inspector_default is not None: + rendered_inspector_default = re.sub( + r"^\((.+)\)$", r"\1", rendered_inspector_default + ) + + rendered_inspector_default = re.sub( + r"^\"?'(.+)'\"?$", r"\1", rendered_inspector_default + ) + + rendered_inspector_default = rendered_inspector_default.strip() + return rendered_inspector_default != rendered_metadata_default + def emit_begin(self) -> None: self._exec("SET TRANSACTION READ WRITE") diff --git a/alembic/ddl/sqlite.py b/alembic/ddl/sqlite.py index 5123332..6b8c293 100644 --- a/alembic/ddl/sqlite.py +++ b/alembic/ddl/sqlite.py @@ -112,6 +112,10 @@ class SQLiteImpl(DefaultImpl): if rendered_inspector_default is not None: rendered_inspector_default = re.sub( + r"^\((.+)\)$", r"\1", rendered_inspector_default + ) + + rendered_inspector_default = re.sub( r"^\"?'(.+)'\"?$", r"\1", rendered_inspector_default ) diff --git a/docs/build/unreleased/1145.rst b/docs/build/unreleased/1145.rst new file mode 100644 index 0000000..eb11a0a --- /dev/null +++ b/docs/build/unreleased/1145.rst @@ -0,0 +1,29 @@ +.. change:: + :tags: bug, autogenerate + :tickets: 1145 + + Fixed issue where server default compare would not work for string defaults + that contained backslashes, due to mis-rendering of these values when + comparing their contents. + + +.. change:: + :tags: bug, oracle + + Implemented basic server default comparison for the Oracle backend; + previously, Oracle's formatting of reflected defaults prevented any + matches from occurring. + +.. change:: + :tags: bug, sqlite + + Adjusted SQLite's compare server default implementation to better handle + defaults with or without parens around them, from both the reflected and + the local metadata side. + +.. change:: + :tags: bug, mssql + + Adjusted SQL Server's compare server default implementation to better + handle defaults with or without parens around them, from both the reflected + and the local metadata side. diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index 86b2460..88806ff 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -862,6 +862,79 @@ class CompareTypeSpecificityTest(TestBase): ) +class CompareServerDefaultTest(TestBase): + __backend__ = True + + @testing.fixture() + def connection(self): + with config.db.begin() as conn: + yield conn + + @testing.fixture() + def metadata(self, connection): + m = MetaData() + yield m + m.drop_all(connection) + + @testing.combinations( + (VARCHAR(30), text("'some default'"), text("'some new default'")), + (VARCHAR(30), "some default", "some new default"), + (VARCHAR(30), text("'//slash'"), text("'s//l//ash'")), + (Integer(), text("15"), text("20")), + (Integer(), "15", "20"), + id_="sss", + argnames="type_,default_text,new_default_text", + ) + def test_server_default_yes_positives( + self, type_, default_text, new_default_text, connection, metadata + ): + t1 = Table( + "t1", metadata, Column("x", type_, server_default=default_text) + ) + t1.create(connection) + + new_metadata = MetaData() + Table( + "t1", + new_metadata, + Column("x", type_, server_default=new_default_text), + ) + + mc = MigrationContext.configure( + connection, opts={"compare_server_default": True} + ) + + diff = api.compare_metadata(mc, new_metadata) + eq_(len(diff), 1) + eq_(diff[0][0][0], "modify_default") + + @testing.combinations( + (VARCHAR(30), text("'some default'")), + (VARCHAR(30), "some default"), + (VARCHAR(30), text("'//slash'")), + (VARCHAR(30), text("'has '' quote'")), + (Integer(), text("15")), + (Integer(), "15"), + id_="ss", + argnames="type_,default_text", + ) + def test_server_default_no_false_positives( + self, type_, default_text, connection, metadata + ): + t1 = Table( + "t1", metadata, Column("x", type_, server_default=default_text) + ) + t1.create(connection) + + mc = MigrationContext.configure( + connection, opts={"compare_server_default": True} + ) + + diff = api.compare_metadata(mc, metadata) + + assert not diff + + class CompareMetadataToInspectorTest(TestBase): __backend__ = True |