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

using "postgresql.ARRAY(Unicode())" breaks automigrations #85

Closed
sqlalchemy-bot opened this issue Nov 10, 2012 · 17 comments
Closed

using "postgresql.ARRAY(Unicode())" breaks automigrations #85

sqlalchemy-bot opened this issue Nov 10, 2012 · 17 comments

Comments

@sqlalchemy-bot
Copy link

Migrated issue, originally created by Domen Kožar (@iElectric)

Because Unicode is undefined, alembic expects:

postgresql.ARRAY(sa.Unicode())

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

we'd have to run type repr() generation through impl-specific hooks on this one, since a simple concatenation is not going to cut it in this case...

@sqlalchemy-bot
Copy link
Author

Changes by Michael Bayer (@zzzeek):

  • added labels: postgresql typing issues

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

Issue #215 was marked as a duplicate of this issue.

@sqlalchemy-bot
Copy link
Author

Adam Gradzki wrote:

when I have a model with ARRAY(TEXT), alembic incorrectly fails to expand TEXT to sa.TEXT()

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

Issue #303 was marked as a duplicate of this issue.

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

Issue #368 was marked as a duplicate of this issue.

@sqlalchemy-bot
Copy link
Author

Andrei Mukhametvaleev (@Mukhametvaleev) wrote:

This is not fixed yet? I have a same problem with some other field types in "postgresql.ARRAY()"

@sqlalchemy-bot
Copy link
Author

Stavros Filargyropoulos (@sfilargi) wrote:

Same here on postgresql.ARRAY() with String() vs sa.String()

@sqlalchemy-bot
Copy link
Author

Andy Gan (@skavie) wrote:

Same here on postgresql.JSON(astext_type=Text()) vs postgresql.JSON(astext_type=sa.Text())

Thanks Michael.

@sqlalchemy-bot
Copy link
Author

aleksanb-td wrote:

I encountered this bug in my project at work yesterday, and after a deep dive in the code i managed to find the correct search terms to find this issue.

Relevant links for future people arriving in this issue tracker:
http://disq.us/p/1df9tlt
http://stackoverflow.com/questions/39997252/flask-python-manage-py-db-upgrade-raise-error
.

There are two solutions for the concrete case with postgresql.JSON(astext_type=Text()) as shown in the stack overflow link. Either remove the astext annotation from the migration manually, as it's actually a default value, secondly add the missing import manually.

A third option is fixing the source itself, which will require changing the _repr_ function called from here https://bitbucket.org/zzzeek/alembic/src/3926ac6b000ed7f3dbdb5376a54f39351b3b4e35/alembic/autogenerate/render.py?at=master&fileviewer=file-view-default#render.py-355.

I tried looking at changing it myself, but without full architectural knowledge it would be a hack at best.

@sqlalchemy-bot
Copy link
Author

Paul Brackin (@pbrackin) wrote:

Had this with Integer type:

db.Column( postgresql.ARRAY(db.Integer) )
'''
op.add_column('post', sa.Column('post_path', postgresql.ARRAY(Integer()), nullable=True))
NameError: name 'Integer' is not defined
'''

Patched it by editing the migration file, manually adding 'sa' like so:

op.add_column('post', sa.Column('path', postgresql.ARRAY(Integer()), nullable=True))
#becomes
op.add_column('post', sa.Column('path', postgresql.ARRAY(sa.Integer()), nullable=True))

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

5 years, 16 watchers, 10 votes, zero pull requests :(.

Here is what "impl-specific hook" means:

  1. go into autogen/render.py

  2. go into def _repr_type. right below where it checks for a _user_defined_render(), we do another check, just like that, against the "impl", which is the thing in alembic/ddl/impl.py -> DefaultImpl. With Postgresql it's an alembic.ddl.postgresql.PostgresqlImpl. In _repr_type, this object is right there at autogen_context.migration_context.impl. if it returns False, keep going. If it returns a string, use it.

  3. DefaultImpl gets a new method _render_type(type_obj, autogen_context) that returns False.

  4. PosgresqlImpl gets a new method _render_type(type_obj, autogen_context) that looks for ARRAY and then makes the string. Make sure it consults autogen_context.opts['sqlalchemy_module_prefix'] too.

  5. new test is added in test_autogen_render.py -> AutogenRenderTest. Look at test_repr_dialect_type for a general dialect-specific type test.

  6. run the test to make sure it works!

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

see also #411. We have a PR at zzzeek/alembic#38 which does the above architecture very nicely and I have to pull it into gerrit and get it ready to go. JSON/JSONB should also be pulled into it for #411.

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

Fix postgresql automigration for ARRAY types

Adds a new codepath into render._repr_type() that will consult
the dialect impl for specific types. On the postgresql side,
the exisiting repr() is combined with a replace featuring
the full autogen render of the nested type.

Co-authored-by: Mike Bayer mike_mp@zzzcomputing.com
Fixes: #85
Change-Id: I8796bfeea27d48e6f8bb5ea4562bdc04961ba0d5
Pull-request: zzzeek/alembic#38

f1cf86e

@sqlalchemy-bot
Copy link
Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot
Copy link
Author

Vladimir Goncharov (@AmatanHead) wrote:

Hi!

I'm able to reproduce this with 0.9.5. Is this fix released?

My db model's code looks like this:

from sqlalchemy import MetaData, Table, Column, types, schema, text

metadata = MetaData()

task = Table(
    'task', metadata,

    Column('id', types.Integer, primary_key=True),
    Column('experiments', types.ARRAY(types.String), nullable=False),
    ...
)

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

seems to only be taking effect for postgresql.ARRAY. See #442.

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

No branches or pull requests

2 participants