-
-
Notifications
You must be signed in to change notification settings - Fork 326
Fix MySQL native ENUM autogenerate detection #1746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
14cfe65
600418b
5031ce9
9a9a042
49b1784
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,6 +350,30 @@ def correct_for_autogen_foreignkeys(self, conn_fks, metadata_fks): | |
| ): | ||
| cnfk.onupdate = "RESTRICT" | ||
|
|
||
| def compare_type( | ||
| self, | ||
| inspector_column: schema.Column[Any], | ||
| metadata_column: schema.Column, | ||
| ) -> bool: | ||
| """Override compare_type to properly detect MySQL native ENUM changes. | ||
|
|
||
| This addresses the issue where autogenerate fails to detect when new | ||
| values are added to or removed from MySQL native ENUM columns. | ||
| """ | ||
| metadata_type = metadata_column.type | ||
| inspector_type = inspector_column.type | ||
|
|
||
| # Check if both columns are MySQL native ENUMs | ||
| if isinstance(metadata_type, sqltypes.Enum) and isinstance( | ||
| inspector_type, sqltypes.Enum | ||
| ): | ||
| # Compare the actual enum values, ignoring order | ||
| if set(metadata_type.enums) != set(inspector_type.enums): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Michael Bayer (zzzeek) wrote: yeah ive adjusted it to use set but also not to run the fallback compare_type which will report those as different View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/6520 |
||
| return True | ||
|
|
||
| # Fall back to default comparison for non-ENUM types | ||
| return super().compare_type(inspector_column, metadata_column) | ||
|
|
||
|
|
||
| class MariaDBImpl(MySQLImpl): | ||
| __dialect__ = "mariadb" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| from sqlalchemy import Column | ||
| from sqlalchemy import Computed | ||
| from sqlalchemy import DATETIME | ||
| from sqlalchemy import Enum | ||
| from sqlalchemy import exc | ||
| from sqlalchemy import Float | ||
| from sqlalchemy import func | ||
|
|
@@ -14,10 +15,12 @@ | |
| from sqlalchemy import Table | ||
| from sqlalchemy import text | ||
| from sqlalchemy import TIMESTAMP | ||
| from sqlalchemy.dialects.mysql import ENUM as MySQL_ENUM | ||
| from sqlalchemy.dialects.mysql import VARCHAR | ||
|
|
||
| from alembic import autogenerate | ||
| from alembic import op | ||
| from alembic import testing | ||
| from alembic import util | ||
| from alembic.autogenerate import api | ||
| from alembic.autogenerate.compare.constraints import _compare_nullable | ||
|
|
@@ -27,6 +30,7 @@ | |
| from alembic.testing import combinations | ||
| from alembic.testing import config | ||
| from alembic.testing import eq_ignore_whitespace | ||
| from alembic.testing import is_ | ||
| from alembic.testing.env import clear_staging_env | ||
| from alembic.testing.env import staging_env | ||
| from alembic.testing.fixtures import AlterColRoundTripFixture | ||
|
|
@@ -40,6 +44,7 @@ | |
| from alembic.autogenerate.compare.types import ( | ||
| _dialect_impl_compare_type as _compare_type, | ||
| ) | ||
| from alembic.ddl.mysql import MySQLImpl | ||
|
|
||
|
|
||
| class MySQLOpTest(TestBase): | ||
|
|
@@ -784,3 +789,53 @@ def test_render_add_index_expr_func(self): | |
| "op.create_index('foo_idx', 't', " | ||
| "['x', sa.literal_column('(coalesce(y, 0))')], unique=False)", | ||
| ) | ||
|
|
||
|
|
||
| class MySQLEnumCompareTest(TestBase): | ||
| """Test MySQL native ENUM comparison in autogenerate.""" | ||
|
|
||
| __only_on__ = "mysql", "mariadb" | ||
| __backend__ = True | ||
|
|
||
| @testing.fixture() | ||
| def connection(self): | ||
| with config.db.begin() as conn: | ||
| yield conn | ||
|
|
||
| @testing.combinations( | ||
| ( | ||
| Enum("A", "B", "C", native_enum=True), | ||
| Enum("A", "B", "C", native_enum=True), | ||
| False, | ||
| ), | ||
| ( | ||
| Enum("A", "B", "C", native_enum=True), | ||
| Enum("A", "B", "C", "D", native_enum=True), | ||
| True, | ||
| ), | ||
| ( | ||
| Enum("A", "B", "C", "D", native_enum=True), | ||
| Enum("A", "B", "C", native_enum=True), | ||
| True, | ||
| ), | ||
| ( | ||
| Enum("A", "B", "C", native_enum=True), | ||
| Enum("C", "B", "A", native_enum=True), | ||
| False, | ||
| ), | ||
| (MySQL_ENUM("A", "B", "C"), MySQL_ENUM("A", "B", "C"), False), | ||
| (MySQL_ENUM("A", "B", "C"), MySQL_ENUM("A", "B", "C", "D"), True), | ||
| id_="ssa", | ||
| argnames="inspected_type,metadata_type,expected", | ||
| ) | ||
zzzeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| def test_compare_enum_types( | ||
| self, inspected_type, metadata_type, expected, connection | ||
| ): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Michael Bayer (zzzeek) wrote: Done View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/6520 |
||
| impl = MySQLImpl(connection.dialect, connection, False, None, None, {}) | ||
|
|
||
| is_( | ||
| impl.compare_type( | ||
| Column("x", inspected_type), Column("x", metadata_type) | ||
| ), | ||
| expected, | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.