Skip to content

Commit

Permalink
Removed server default quoting from compare
Browse files Browse the repository at this point in the history
Don't modify the metadata server default when comparing it in the
autogenerate process.
This impacts the value passes to user provided functions passed in
:paramref:`.EnvironmentContext.configure.compare_server_default`
and third party dialect that implement a custom ``compare_server_default``.

Fixes: #1178
Change-Id: Ib429efcf9077337f768ad5aad91659867e89391a
  • Loading branch information
CaselIT committed May 11, 2023
1 parent 92e54a0 commit 230a293
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 14 deletions.
12 changes: 3 additions & 9 deletions alembic/autogenerate/compare.py
Expand Up @@ -1023,9 +1023,7 @@ def _compare_type(


def _render_server_default_for_compare(
metadata_default: Optional[Any],
metadata_col: Column,
autogen_context: AutogenContext,
metadata_default: Optional[Any], autogen_context: AutogenContext
) -> Optional[str]:

if isinstance(metadata_default, sa_schema.DefaultClause):
Expand All @@ -1039,11 +1037,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 f"'{metadata_default}'"
else:
return metadata_default
return metadata_default
else:
return None

Expand Down Expand Up @@ -1190,7 +1184,7 @@ def _compare_server_default(
)
else:
rendered_metadata_default = _render_server_default_for_compare(
metadata_default, metadata_col, autogen_context
metadata_default, autogen_context
)

rendered_conn_default = (
Expand Down
17 changes: 16 additions & 1 deletion alembic/context.pyi
Expand Up @@ -22,6 +22,8 @@ if TYPE_CHECKING:
from sqlalchemy.engine.base import Connection
from sqlalchemy.engine.url import URL
from sqlalchemy.sql.elements import ClauseElement
from sqlalchemy.sql.schema import Column
from sqlalchemy.sql.schema import FetchedValue
from sqlalchemy.sql.schema import MetaData
from sqlalchemy.sql.schema import SchemaItem

Expand Down Expand Up @@ -144,7 +146,20 @@ def configure(
]
] = None,
compare_type: bool = False,
compare_server_default: bool = False,
compare_server_default: Union[
bool,
Callable[
[
MigrationContext,
Column,
Column,
Optional[str],
Optional[FetchedValue],
Optional[str],
],
Optional[bool],
],
] = False,
render_item: Optional[
Callable[[str, Any, AutogenContext], Union[str, Literal[False]]]
] = None,
Expand Down
11 changes: 10 additions & 1 deletion alembic/ddl/mysql.py
Expand Up @@ -185,13 +185,22 @@ def compare_server_default(
and rendered_inspector_default == "'0'"
):
return False
elif inspector_column.type._type_affinity is sqltypes.Integer:
elif (
rendered_inspector_default
and inspector_column.type._type_affinity is sqltypes.Integer
):
rendered_inspector_default = (
re.sub(r"^'|'$", "", rendered_inspector_default)
if rendered_inspector_default is not None
else None
)
return rendered_inspector_default != rendered_metadata_default
elif (
rendered_metadata_default
and metadata_column.type._type_affinity is sqltypes.String
):
metadata_default = re.sub(r"^'|'$", "", rendered_metadata_default)
return rendered_inspector_default != f"'{metadata_default}'"
elif rendered_inspector_default and rendered_metadata_default:
# adjust for "function()" vs. "FUNCTION" as can occur particularly
# for the CURRENT_TIMESTAMP function on newer MariaDB versions
Expand Down
16 changes: 15 additions & 1 deletion alembic/runtime/environment.py
Expand Up @@ -15,6 +15,8 @@
from typing import TYPE_CHECKING
from typing import Union

from sqlalchemy.sql.schema import Column
from sqlalchemy.sql.schema import FetchedValue
from typing_extensions import Literal

from .migration import _ProxyTransaction
Expand Down Expand Up @@ -79,6 +81,18 @@
None,
]

CompareServerDefault = Callable[
[
MigrationContext,
Column,
Column,
Optional[str],
Optional[FetchedValue],
Optional[str],
],
Optional[bool],
]


class EnvironmentContext(util.ModuleClsProxy):

Expand Down Expand Up @@ -398,7 +412,7 @@ def configure(
ProcessRevisionDirectiveFn
] = None,
compare_type: bool = False,
compare_server_default: bool = False,
compare_server_default: Union[bool, CompareServerDefault] = False,
render_item: Optional[RenderItemFn] = None,
literal_binds: bool = False,
upgrade_token: str = "upgrades",
Expand Down
9 changes: 9 additions & 0 deletions docs/build/unreleased/1178.rst
@@ -0,0 +1,9 @@
.. change::
:tags: changed, autogenerate
:tickets: 1178

Don't modify the metadata server default when comparing it in the
autogenerate process.
This impacts the value passes to user provided functions passed in
:paramref:`.EnvironmentContext.configure.compare_server_default`
and third party dialect that implement a custom ``compare_server_default``.
4 changes: 2 additions & 2 deletions tests/test_postgresql.py
Expand Up @@ -846,7 +846,7 @@ def _expect_default(self, c_expected, col, seq=None):

eq_(
_render_server_default_for_compare(
tab.c.x.server_default, tab.c.x, self.autogen_context
tab.c.x.server_default, self.autogen_context
),
c_expected,
)
Expand All @@ -867,7 +867,7 @@ def _expect_default(self, c_expected, col, seq=None):
server_default = diffs[0][0][4]["existing_server_default"]
eq_(
_render_server_default_for_compare(
server_default, tab.c.x, self.autogen_context
server_default, self.autogen_context
),
c_expected,
)
Expand Down

0 comments on commit 230a293

Please sign in to comment.