Skip to content
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

The Enum's name property is lost within an autogenerated migration #1551

Open
mberdyshev opened this issue Oct 14, 2024 · 7 comments
Open

The Enum's name property is lost within an autogenerated migration #1551

mberdyshev opened this issue Oct 14, 2024 · 7 comments
Labels
autogenerate - rendering autogenerate for enums a long term subject, tagging issues related to this bug Something isn't working PRs (with tests!) welcome

Comments

@mberdyshev
Copy link

mberdyshev commented Oct 14, 2024

Describe the bug
A migration doesn't provide the name property of Enum if sqlalchemy.Enum is augmented with sqlalchemy.types.Decorator. Even if the name property is explicitly given.

Expected behavior
The property has to be provided in Enum's definition inside a migration.

To Reproduce
sql_types.py

from typing import Any
from sqlalchemy import Enum, TypeDecorator

class Enum1(Enum):
    def __init__(self, *enums: object, **kw: Any):
        validate_strings = kw.pop("validate_strings", True)
        super().__init__(*enums, **kw, validate_strings=validate_strings)

class Enum2(TypeDecorator):
    impl = Enum
    cache_ok = True

    def __init__(self, *enums: object, **kw: Any):
        validate_strings = kw.pop("validate_strings", True)
        super().__init__(*enums, **kw, validate_strings=validate_strings)

tables.py

import enum
from sqlalchemy import Column, MetaData, Table
from sql_types import Enum1, Enum2

class TestEnum(str, enum.Enum):
    FIRST = enum.auto()
    SECOND = enum.auto()

metadata = MetaData()

test_table = Table(
    "test",
    metadata,
    Column("col1", Enum1(TestEnum), nullable=False),
    Column("col2", Enum2(TestEnum, name="enum2"), nullable=False),
)

alembic revision --autogenerate -m "Enums" provides (shortened):

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('test',
    sa.Column('col1', db.sql_types.Enum1('FIRST', 'SECOND', name='testenum'), nullable=False),
    sa.Column('col2', db.sql_types.Enum2('FIRST', 'SECOND'), nullable=False)
    )
    # ### end Alembic commands ###

def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_table('test')
    # ### end Alembic commands ###

Error
As you can see from the migration enums are defined differently - the second one misses its name property.
If I try to run it on PostgreSQL I get the error:

Stacktrace
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Generating static SQL
INFO  [alembic.runtime.migration] Will assume transactional DDL.
BEGIN;

CREATE TABLE alembic_version (
    version_num VARCHAR(32) NOT NULL,
    CONSTRAINT alembic_version_pkc PRIMARY KEY (version_num)
);

INFO  [alembic.runtime.migration] Running upgrade  -> 264b6f57ae76, Enums
-- Running upgrade  -> 264b6f57ae76

CREATE TYPE testenum AS ENUM ('FIRST', 'SECOND');

Traceback (most recent call last):
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\__main__.py", line 4, in <module>
    main(prog="alembic")
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\config.py", line 636, in main
    CommandLine(prog=prog).main(argv=argv)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\config.py", line 626, in main
    self.run_cmd(cfg, options)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\config.py", line 603, in run_cmd
    fn(
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\command.py", line 406, in upgrade
    script.run_env()
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\script\base.py", line 586, in run_env
    util.load_python_file(self.dir, "env.py")
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\util\pyfiles.py", line 95, in load_python_file
    module = load_module_py(module_id, path)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\util\pyfiles.py", line 113, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "c:\Users\Михаил\PycharmProjects\Example Project\db\alembic\env.py", line 78, in <module>
    run_migrations_offline()
  File "c:\Users\Михаил\PycharmProjects\Example Project\db\alembic\env.py", line 52, in run_migrations_offline
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\runtime\environment.py", line 946, in run_migrations
    self.get_context().run_migrations(**kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\runtime\migration.py", line 628, in run_migrations
    step.migration_fn(**kw)
  File "C:\Users\Михаил\PycharmProjects\Example Project\db\alembic\versions\264b6f57ae76_enums.py", line 24, in upgrade
    op.create_table('test',
  File "<string>", line 8, in create_table
  File "<string>", line 3, in create_table
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\operations\ops.py", line 1318, in create_table
    return operations.invoke(op)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\operations\base.py", line 442, in invoke
    return fn(self, operation)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\operations\toimpl.py", line 143, in create_table
    operations.impl.create_table(table, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\ddl\impl.py", line 366, in create_table
    table.dispatch.before_create(
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\event\attr.py", line 497, in __call__
    fn(*args, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\util\langhelpers.py", line 852, in __call__
    return getattr(self.target, self.name)(*arg, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\sqltypes.py", line 1130, in _on_table_create
    t._on_table_create(target, bind, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\dialects\postgresql\named_types.py", line 98, in _on_table_create
    self.create(bind=bind, checkfirst=checkfirst)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\dialects\postgresql\named_types.py", line 338, in create
    super().create(bind, checkfirst=checkfirst)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\dialects\postgresql\named_types.py", line 51, in create
    bind._run_ddl_visitor(self.DDLGenerator, self, checkfirst=checkfirst)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\engine\mock.py", line 61, in _run_ddl_visitor
    visitorcallable(self.dialect, self, **kwargs).traverse_single(element)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\visitors.py", line 664, in traverse_single
    return meth(obj, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\dialects\postgresql\named_types.py", line 153, in visit_enum
    self.connection.execute(CreateEnumType(enum))
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\engine\mock.py", line 69, in execute
    return self._execute_impl(obj, parameters)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\runtime\migration.py", line 675, in dump
    self.impl._exec(construct)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\ddl\impl.py", line 190, in _exec
    compiled = construct.compile(dialect=self.dialect, **compile_kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\elements.py", line 308, in compile
    return self._compiler(dialect, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\ddl.py", line 69, in _compiler
    return dialect.ddl_compiler(dialect, self, **kw)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\compiler.py", line 870, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\compiler.py", line 915, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\sql\visitors.py", line 141, in _compiler_dispatch
    return meth(self, **kw)  # type: ignore  # noqa: E501
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\dialects\postgresql\base.py", line 2236, in visit_create_enum_type
    self.preparer.format_type(type_),
  File "C:\Users\Михаил\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\sqlalchemy\dialects\postgresql\base.py", line 2728, in format_type
    raise exc.CompileError(
sqlalchemy.exc.CompileError: PostgreSQL ENUM type requires a name.

Versions.

  • OS: Windows 11 23H2
  • Python: 3.10.11
  • Alembic: 1.13.3
  • SQLAlchemy: 2.0.35
  • Database: PostgreSQL 17.0.1
  • DBAPI: psycopg2 2.9.9

Additional context
The docs state that subclassing from TypeDecorator should be preferred:

This method is preferred to direct subclassing of SQLAlchemy’s built-in types as it ensures that all required functionality of the underlying type is kept in place.

Have a nice day!

@mberdyshev mberdyshev added the requires triage New issue that requires categorization label Oct 14, 2024
@CaselIT CaselIT added bug Something isn't working autogenerate - rendering PRs (with tests!) welcome and removed requires triage New issue that requires categorization labels Oct 15, 2024
@CaselIT
Copy link
Member

CaselIT commented Oct 16, 2024

Hi,

Managing all attributes of enum with a type decorator is not easy, so it's likely missing something here.

If your use case I would more simply do something like

def Enum1(*args,**kwargs):
  kwargs.set_default('validate_strings', True)
  return Enum(*args, **kwargs)

In any case it's a valid bug

@CaselIT CaselIT added the autogenerate for enums a long term subject, tagging issues related to this label Oct 16, 2024
@mberdyshev
Copy link
Author

I've looked up the source code and found that type rendering depends on __repr__(). So, inside SQLAlchemy I managed to find that Enum and TypeDecorator representations are different. To sum it up, the difference is the following:

enum = Enum(TestEnum)
print(util.generic_repr(enum))  # TypeDecorator
# Enum('FIRST', 'SECOND')
print(util.generic_repr(enum, to_inspect=[Enum, SchemaType]))  # Enum
# Enum('FIRST', 'SECOND', name='testenum')

It seems now that this bug is more related to SQLAlchemy. Probably TypeDecorator's __repr__() should respect the contained type's __repr__().

@CaselIT
Copy link
Member

CaselIT commented Oct 17, 2024

nice finding. Seems like that may be the issue. I'm not sure why there is a reason for that repr in type decorator (there usually is).

@zzzeek maybe a type should have a generic method to format itself that type decorator could hook into?

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2024

But the TypeDecorator does not repr() as the inner type does, it has its ownr repr() that does something different:

from sqlalchemy import *

print(repr(Enum("a", "b", "c", name="e1")))

class MyType(TypeDecorator):
    impl = Enum


print(repr(MyType("a", "b", "c", name="e2")))

prints:

Enum('a', 'b', 'c', name='e1')
MyType('a', 'b', 'c')

note it uses MyType. so that's not just calling Enum.__repr__(). also no I'm not going to take the string from Enum.__repr__() and manipulate it, that's too much guessing.

so there's no quick fix here. choices include:

  1. add a render() rule to Alembic for typedecorator + enum
  2. fix util.generic_repr() to not need all those special instructions inside of Enum.repr() so that it can be used more generically
  3. add a new series of methods to TypeEngine that break out the internals of __repr__() into commands that are used to build out a composed repr

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2024

this would be the most expedient fix for the moment, but it wouldn't get the extra arguments used by enum beyond those used for schema type:

diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py
index bc2d898ab..6676a61e0 100644
--- a/lib/sqlalchemy/sql/sqltypes.py
+++ b/lib/sqlalchemy/sql/sqltypes.py
@@ -3826,3 +3826,4 @@ type_api.MATCHTYPE = MATCHTYPE
 type_api.INDEXABLE = INDEXABLE = Indexable
 type_api.TABLEVALUE = TABLEVALUE
 type_api._resolve_value_to_type = _resolve_value_to_type
+type_api.SchemaType = SchemaType
diff --git a/lib/sqlalchemy/sql/type_api.py b/lib/sqlalchemy/sql/type_api.py
index 9f40905fa..f484e26db 100644
--- a/lib/sqlalchemy/sql/type_api.py
+++ b/lib/sqlalchemy/sql/type_api.py
@@ -58,6 +58,7 @@ if typing.TYPE_CHECKING:
     from .sqltypes import NUMERICTYPE as NUMERICTYPE  # noqa: F401
     from .sqltypes import STRINGTYPE as STRINGTYPE  # noqa: F401
     from .sqltypes import TABLEVALUE as TABLEVALUE  # noqa: F401
+    from .sqltypes import SchemaType as SchemaType  # noqa: F401
     from ..engine.interfaces import Dialect
     from ..util.typing import GenericProtocol
 
@@ -2276,7 +2277,13 @@ class TypeDecorator(SchemaEventTarget, ExternalType, TypeEngine[_T]):
         return self.impl_instance.sort_key_function
 
     def __repr__(self) -> str:
-        return util.generic_repr(self, to_inspect=self.impl_instance)
+        inst = self.impl_instance
+        if isinstance(inst, SchemaType):
+            to_inspect=[inst, SchemaType]
+        else:
+            to_inspect = [inst]
+
+        return util.generic_repr(self, to_inspect=to_inspect)
 
 
 class Variant(TypeDecorator[_T]):

@CaselIT
Copy link
Member

CaselIT commented Oct 17, 2024

But the TypeDecorator does not repr() as the inner type does, it has its ownr repr() that does something different:

I know, that's why I suggested to have new methods for this.
regarding the fix you proposed it's likely better to have a proper method for it. the effort should be similar

@abhiaagarwal
Copy link

abhiaagarwal commented Nov 7, 2024

For what it's worth, I ran into a similar issue. I have a PydanticModelType generic

class PydanticModelType(TypeDecorator, Generic[T]):
    """Generic class representing a pydantic model that can be serialized to Python."""

    cache_ok = True
    impl = JSON()

    def __init__(self, pydantic_type: type[T]) -> None:
        """Initializes class with the type of the pydantic model."""
        self.pydantic_type = pydantic_type
        super().__init__()

    @override
    def load_dialect_impl(self, dialect: Dialect) -> TypeEngine:
        # Use JSONB for PostgreSQL and JSON for other databases.
        if dialect.name == "postgresql":
            return dialect.type_descriptor(JSONB())

        return dialect.type_descriptor(JSON())

    @override
    def process_bind_param(self, value: T | None, _dialect: Dialect) -> str | None:
        if value is not None:
            value = self.pydantic_type.model_dump_json(value)
        return value

    @override
    def process_result_value(
        self, value: str | None, _dialect: Dialect
    ) -> BaseModel | None:
        if value is not None:
            value = self.pydantic_type.model_validate_json(value)
        return value

And when I declare it in my class

configuration: Mapped[Config] = mapped_column(
        PydanticModelType(Config), nullable=False
    )

Alembic generates this:

sa.Column('configuration', app.database.types.PydanticModelType(), nullable=False),

Not sure if it's the same bug. I'll try overriding repr later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autogenerate - rendering autogenerate for enums a long term subject, tagging issues related to this bug Something isn't working PRs (with tests!) welcome
Projects
None yet
Development

No branches or pull requests

4 participants