Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions alembic/ddl/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,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.
Comment on lines +356 to +359
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring should document the return value and parameters for clarity. Consider expanding it:

"""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.

Args:
    inspector_column: Column as reflected from the database
    metadata_column: Column as defined in the metadata/model

Returns:
    True if the column types differ, False if they are the same.
    For ENUM types, comparison includes the enum values and their order.
"""
Suggested change
"""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.
"""
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.
Args:
inspector_column (sqlalchemy.schema.Column): Column as reflected from the database.
metadata_column (sqlalchemy.schema.Column): Column as defined in the metadata/model.
Returns:
bool: True if the column types differ, False if they are the same.
For ENUM types, comparison includes the enum values and their order.

Copilot uses AI. Check for mistakes.
"""
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
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining that order matters for MySQL ENUM comparison since the PR description emphasizes this. This helps future maintainers understand the importance:

# Compare the actual enum values (order matters for MySQL ENUMs)
if metadata_type.enums != inspector_type.enums:
    return True
Suggested change
# Compare the actual enum values
# Compare the actual enum values; order matters for MySQL ENUMs.
# Changing the order of ENUM values is a schema change in MySQL.

Copilot uses AI. Check for mistakes.
if metadata_type.enums != inspector_type.enums:
return True

# Fall back to default comparison for non-ENUM types
return super().compare_type(inspector_column, metadata_column)


class MariaDBImpl(MySQLImpl):
__dialect__ = "mariadb"
Expand Down
58 changes: 58 additions & 0 deletions tests/test_mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,6 +15,7 @@
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
Expand All @@ -23,10 +25,12 @@
from alembic.autogenerate import compare
from alembic.migration import MigrationContext
from alembic.operations import ops
from alembic import testing
from alembic.testing import assert_raises_message
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
Expand Down Expand Up @@ -771,3 +775,57 @@ 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),
True,
),
(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",
)
Comment on lines +791 to +816
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test coverage is missing several important scenarios:

  1. Mixed type comparison: Testing ENUM vs non-ENUM (e.g., Enum("A", "B") vs VARCHAR(20))
  2. Empty ENUM: Testing behavior with empty enum lists
  3. Single value ENUM: Testing Enum("A") vs Enum("A", "B")
  4. Case sensitivity: Testing if enum value comparison is case-sensitive (e.g., Enum("a") vs Enum("A"))
  5. Duplicate values: Testing behavior with duplicate enum values if SQLAlchemy allows them
  6. Non-native ENUM: Testing Enum("A", "B", native_enum=False) to ensure it's not incorrectly handled

Consider adding test cases for these edge cases to ensure robust behavior.

Copilot uses AI. Check for mistakes.
def test_compare_enum_types(
self, inspected_type, metadata_type, expected, connection
):
from alembic.ddl.mysql import MySQLImpl

impl = MySQLImpl(
"mysql", connection, (), {}, None, None, None, lambda: None
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MySQLImpl constructor is being called with incorrect arguments. The correct signature is:

def __init__(
    self,
    dialect: Dialect,           # Should be a Dialect object, not string "mysql"
    connection: Optional[Connection],
    as_sql: bool,               # Currently passing () instead
    transactional_ddl: Optional[bool],  # Currently passing {}
    output_buffer: Optional[TextIO],    # Currently passing None
    context_opts: Dict[str, Any],       # Currently passing None
)

It should be:

impl = MySQLImpl(
    connection.dialect, connection, False, None, None, {}
)
Suggested change
"mysql", connection, (), {}, None, None, None, lambda: None
connection.dialect, connection, False, None, None, {}

Copilot uses AI. Check for mistakes.
)

is_(
impl.compare_type(
Column("x", inspected_type), Column("x", metadata_type)
),
expected,
)