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

render type.impl when type is a TypeDecorator instance #1387

Closed
wants to merge 3 commits into from

Conversation

saifelse
Copy link
Contributor

@saifelse saifelse commented Jan 5, 2024

Description

When rendering the type, we check to see if we are dealing with a TypeDecorator, and if so, proceed but with type.impl instead... this results in the underlying type from the sqlalchemy package to be rendered into the migration instead of the TypeDecorator (wherever it may be defined).

Fixes: #1386

Checklist

This pull request is:

  • A documentation / typographical error fix
    • Good to go, no issue or tests are needed
  • A short code fix
    • please include the issue number, and create an issue if none exists, which
      must include a complete example of the issue. one line code fixes without an
      issue and demonstration will not be accepted.
    • Please include: Fixes: #<issue number> in the commit message
    • please include tests. one line code fixes without tests will not be accepted.
  • A new feature implementation
    • please include the issue number, and create an issue if none exists, which must
      include a complete example of how the feature would look.
    • Please include: Fixes: #<issue number> in the commit message
    • please include tests.

Have a nice day!

@zzzeek
Copy link
Member

zzzeek commented Jan 5, 2024

as mentioned in my comment I dont think we want to do this. We definitely get users complaining etc. when their exact type is not rendered, even if they don't need it to be. This seems like a big leap of assumptions to make for not much reason, not the least of which that the column as returned inside the Table from a create_table() directive will not have their preferred type.

As mentioned in my comment, this particular preference can be suited by a simple type rendering recipe right now for those who really want to control this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autogenerate renders TypeDecorator instance instead of underlying impl type
2 participants