From 8d97de0ba7ee8dbff4301be6a2bd76a963327d0f Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 14 Feb 2023 12:45:24 -0500 Subject: [PATCH] collapse all chars for mssql defaults, move quoting 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 | 17 ++++------------- alembic/ddl/postgresql.py | 29 +++++++++++++++++------------ docs/build/unreleased/1177.rst | 21 +++++++++++++++++++++ tests/test_autogen_diffs.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 25 deletions(-) create mode 100644 docs/build/unreleased/1177.rst diff --git a/alembic/ddl/mssql.py b/alembic/ddl/mssql.py index b622bc58..bdf215d8 100644 --- a/alembic/ddl/mssql.py +++ b/alembic/ddl/mssql.py @@ -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 diff --git a/alembic/ddl/postgresql.py b/alembic/ddl/postgresql.py index 29efe4c9..32674d2a 100644 --- a/alembic/ddl/postgresql.py +++ b/alembic/ddl/postgresql.py @@ -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 index 00000000..965490bd --- /dev/null +++ b/docs/build/unreleased/1177.rst @@ -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. + + diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index aa775d70..70ea10a4 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -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(), -- 2.47.2