Merge "dont use repr to quote string in compare_server_default" into main

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