]> git.ipfire.org Git - thirdparty/sqlalchemy/alembic.git/commitdiff
collapse all chars for mssql defaults, move quoting
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 14 Feb 2023 17:45:24 +0000 (12:45 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Wed, 15 Feb 2023 18:17:48 +0000 (13:17 -0500)
Ongoing fixes for SQL Server server default comparisons under autogenerate,
adjusting for SQL Server's collapsing of whitespace between SQL function
arguments when reporting on a function-based server default, as well as its
arbitrary addition of parenthesis within arguments; the approach has now
been made more aggressive by stripping the two default strings to compare
of all whitespace, parenthesis, and quoting characters.

Fixed PostgreSQL server default comparison to handle SQL expressions
sent as ``text()`` constructs, such as ``text("substring('name', 1, 3)")``,
which previously would raise errors when attempting to run a server-based
comparison.

Change-Id: Icd861f62653fc7b3900164c0d047821125e1305e
Fixes: #1177
alembic/ddl/mssql.py
alembic/ddl/postgresql.py
docs/build/unreleased/1177.rst [new file with mode: 0644]
tests/test_autogen_diffs.py

index b622bc583ad377a150e47a19052593acc7bcd096..bdf215d8ee7fbb82996d6056472c4cc845de5dbe 100644 (file)
@@ -233,26 +233,17 @@ class MSSQLImpl(DefaultImpl):
     ):
 
         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
+                r"[\(\) \"\']", "", rendered_metadata_default
             )
 
         if rendered_inspector_default is not None:
-
-            # the iteration is a quick hack to remove balanced parens only
-            # up to two levels deep, like ((foo)) but not (foo())
-            # see issue #1152
-            for i in range(2):
-                rendered_inspector_default = re.sub(
-                    r"^\((.+)\)$", r"\1", rendered_inspector_default
-                )
+            # SQL Server collapses whitespace and adds arbitrary parenthesis
+            # within expressions.   our only option is collapse all of it
 
             rendered_inspector_default = re.sub(
-                r"^\"?'(.+)'\"?$", r"\1", rendered_inspector_default
+                r"[\(\) \"\']", "", rendered_inspector_default
             )
 
         return rendered_inspector_default != rendered_metadata_default
index 29efe4c91264dbabb31da423530bd9c9a951285e..32674d2a677f2d9bdb25390169372a4b585d6fd0 100644 (file)
@@ -12,6 +12,7 @@ from typing import TYPE_CHECKING
 from typing import Union
 
 from sqlalchemy import Column
+from sqlalchemy import literal_column
 from sqlalchemy import Numeric
 from sqlalchemy import text
 from sqlalchemy import types as sqltypes
@@ -112,22 +113,26 @@ class PostgresqlImpl(DefaultImpl):
         if defaults_equal:
             return False
 
-        if None in (conn_col_default, rendered_metadata_default):
+        if None in (
+            conn_col_default,
+            rendered_metadata_default,
+            metadata_column.server_default,
+        ):
             return not defaults_equal
 
-        # check for unquoted string and quote for PG String types
-        if (
-            not isinstance(inspector_column.type, Numeric)
-            and metadata_column.server_default is not None
-            and isinstance(metadata_column.server_default.arg, str)
-            and not re.match(r"^'.*'$", rendered_metadata_default)
-        ):
-            rendered_metadata_default = "'%s'" % rendered_metadata_default
+        metadata_default = metadata_column.server_default.arg
+
+        if isinstance(metadata_default, str):
+            if not isinstance(inspector_column.type, Numeric):
+                metadata_default = re.sub(r"^'|'$", "", metadata_default)
+                metadata_default = f"'{metadata_default}'"
+
+            metadata_default = literal_column(metadata_default)
 
+        # run a real compare against the server
         return not self.connection.scalar(
-            text(
-                "SELECT %s = %s"
-                % (conn_col_default, rendered_metadata_default)
+            sqla_compat._select(
+                literal_column(conn_col_default) == metadata_default
             )
         )
 
diff --git a/docs/build/unreleased/1177.rst b/docs/build/unreleased/1177.rst
new file mode 100644 (file)
index 0000000..965490b
--- /dev/null
@@ -0,0 +1,21 @@
+.. change::
+    :tags: bug, mssql
+    :tickets: 1177
+
+    Ongoing fixes for SQL Server server default comparisons under autogenerate,
+    adjusting for SQL Server's collapsing of whitespace between SQL function
+    arguments when reporting on a function-based server default, as well as its
+    arbitrary addition of parenthesis within arguments; the approach has now
+    been made more aggressive by stripping the two default strings to compare
+    of all whitespace, parenthesis, and quoting characters.
+
+
+.. change::
+    :tags: bug, postgresql
+
+    Fixed PostgreSQL server default comparison to handle SQL expressions
+    sent as ``text()`` constructs, such as ``text("substring('name', 1, 3)")``,
+    which previously would raise errors when attempting to run a server-based
+    comparison.
+
+
index aa775d70786f922262a8bb598fe30859ce1035c8..70ea10a4badd650a8e47dc7538f14db701c5d41e 100644 (file)
@@ -11,6 +11,7 @@ from sqlalchemy import Enum
 from sqlalchemy import FLOAT
 from sqlalchemy import ForeignKey
 from sqlalchemy import ForeignKeyConstraint
+from sqlalchemy import func
 from sqlalchemy import Index
 from sqlalchemy import inspect
 from sqlalchemy import INTEGER
@@ -913,6 +914,37 @@ class CompareServerDefaultTest(TestBase):
         (VARCHAR(30), "some default"),
         (VARCHAR(30), text("'//slash'")),
         (VARCHAR(30), text("'has '' quote'")),
+        (
+            VARCHAR(30),
+            func.substring("name", 1, 3),
+            testing.exclusions.only_on(["mssql", "postgresql"]),
+        ),  # note no space
+        (
+            VARCHAR(30),
+            text("substring('name',1,3)"),
+            testing.exclusions.only_on(["mssql", "postgresql"]),
+        ),  # note no space
+        (
+            VARCHAR(30),
+            text("substring('name', 1, 3)"),
+            testing.exclusions.only_on(["mssql", "postgresql"]),
+        ),  # note spaces
+        (
+            VARCHAR(50),
+            text(
+                "substring(user_name(),"  # note no space
+                "charindex('',user_name())+(1),len(user_name()))"
+            ),
+            testing.exclusions.only_on("mssql"),
+        ),
+        (
+            VARCHAR(50),
+            text(
+                "substring(user_name(), "  # note space
+                "charindex('',user_name())+(1),len(user_name()))"
+            ),
+            testing.exclusions.only_on("mssql"),
+        ),
         (DateTime(), text("(getdate())"), testing.exclusions.only_on("mssql")),
         (
             DateTime(),