Navigation Menu

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

ALTER table - Flask-migrate and sqlite #61

Open
SylvainMartel opened this issue May 22, 2015 · 28 comments
Open

ALTER table - Flask-migrate and sqlite #61

SylvainMartel opened this issue May 22, 2015 · 28 comments
Assignees

Comments

@SylvainMartel
Copy link

I know this is more a SQLlite issue, but I was wondering how you dealt with the many "No support for ALTER of constraints in SQLite dialect" that arise with flask-migrate and alembic. I am stuck right now because db upgrade cannot update the database correctly due to such an issue.

@SylvainMartel SylvainMartel changed the title ALTER table ALTER table - Flask-migrate and sqlite May 22, 2015
@miguelgrinberg
Copy link
Owner

This is really between Alembic and SQLite. The latest version of Alembic is able to do some of these unsupported migrations by moving the data to a new table, so maybe you can address your problem.

@italomaia
Copy link

I believe this is what miguel is referring to http://alembic.readthedocs.org/en/latest/batch.html#running-batch-migrations-for-sqlite-and-other-databases

It would be nice if Flask-Migrate could have native support to batch when sqlite is the database.

@italomaia
Copy link

Following this link: https://bitbucket.org/zzzeek/alembic/issues/287
I've found that this:

DATABASE_URI = getattr(app.config, 'SQLALCHEMY_DATABASE_URI', '')
is_sqlite = DATABASE_URI.startswith('sqlite:')
migrate.init_app(app, db, render_as_batch=is_sqlite)

Is partially required to use batch. The second step would be to alter the upgrade command to use batch operation. Not quite sure where to look further into the matter ...

[EDIT] Actually, I also have to update the script.py.mako; let's see how to do this ... no idea how to populate the script template context with a is_sqlite variable

@avacariu
Copy link

Just a note for anyone coming across this issue:

I enabled batch mode for auto-generated migrations by adding a render_as_batch=config.get_main_option('sqlalchemy.url').startswith('sqlite:') argument to context.configure() in run_migrations_online() in env.py.

So my full function looks like:

def run_migrations_online():
    """Run migrations in 'online' mode.
    In this scenario we need to create an Engine
    and associate a connection with the context.
    """
    engine = engine_from_config(
                config.get_section(config.config_ini_section),
                prefix='sqlalchemy.',
                poolclass=pool.NullPool)

    connection = engine.connect()
    context.configure(
                connection=connection,
                target_metadata=target_metadata,
                render_as_batch=config.get_main_option('sqlalchemy.url').startswith('sqlite:///')
                )

    try:
        with context.begin_transaction():
            context.run_migrations()
    finally:
        connection.close()

And it seems to work fine for me.

@nMustaki
Copy link

@Vlad003 : thanks for your snippet, it helped me get started !

A full Sqlite support should include something about naming constraints, and the migrations for sqlite should look something like this from Alembic doc :

naming_convention = {
    "fk":
    "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
}
with self.op.batch_alter_table(
        "bar", naming_convention=naming_convention) as batch_op:
    batch_op.drop_constraint(
        "fk_bar_foo_id_foo", type_="foreignkey")

@miguelgrinberg is extending support for sqlite in your roadmap, or would you prefer pull requests ?

@miguelgrinberg
Copy link
Owner

@nMustaki there is absolutely nothing in this project that performs migrations. I just facilitate the use of Alembic. Any migration improvements you make should go directly into Alembic.

@nMustaki
Copy link

@miguelgrinberg Ok thanks. Do you control the env.py file ?

@miguelgrinberg
Copy link
Owner

This project provides a generic env.py file template. Once you initialize a project, a copy is placed in the migrations directory. From that point on, you can customize it if you need something that isn't provided in the generic version.

@nMustaki
Copy link

Ok,

what do you think about adding the snippet from @Vlad003 in the
documentation in a section about sqlite ?

/*
** "What do you despise? By this you are truly known."
** from Manual of Muad'Dib by the Princess Irulan.
*/

On Thu, Dec 17, 2015 at 4:34 PM, Miguel Grinberg notifications@github.com
wrote:

This project provides a generic env.py file template. Once you initialize
a project, a copy is placed in the migrations directory. From that point
on, you can customize it if you need something that isn't provided in the
generic version.


Reply to this email directly or view it on GitHub
#61 (comment)
.

@miguelgrinberg
Copy link
Owner

Oh okay, I see what you are getting at now. For some reason I assumed this snippet needed to go in the migration script, that's why I sent you to Alembic.

Let me think about how to best handle this. I could, for example, provide an alternate env.py, in the same way I provide one for multidb support. What's unclear to me is if these changes are okay to have when you work with other databases, or if they will prevent you from using anything other than sqlite. I guess I need to experiment with this a bit to know what to do.

@nMustaki
Copy link

Great, keep me posted if I can be of any help !

/*
** "What do you despise? By this you are truly known."
** from Manual of Muad'Dib by the Princess Irulan.
*/

On Thu, Dec 17, 2015 at 7:19 PM, Miguel Grinberg notifications@github.com
wrote:

Oh okay, I see what you are getting at now. For some reason I assumed this
snippet needed to go in the migration script, that's why I sent you to
Alembic.

Let me think about how to best handle this. I could, for example, provide
an alternate env.py, in the same way I provide one for multidb support.
What's unclear to me is if these changes are okay to have when you work
with other databases, or if they will prevent you from using anything other
than sqlite. I guess I need to experiment with this a bit to know what to
do.


Reply to this email directly or view it on GitHub
#61 (comment)
.

@marksteward
Copy link

I've hit against this tonight, and did the following:

naming_convention = {
    "ix": 'ix_%(column_0_label)s',
    "uq": "uq_%(table_name)s_%(column_0_name)s",
    "ck": "ck_%(table_name)s_%(column_0_name)s",
    "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
    "pk": "pk_%(table_name)s"
}
db = SQLAlchemy(metadata=MetaData(naming_convention=naming_convention))

Which is possible in Flask-SQLAlchemy 2.1 thanks to pallets-eco/flask-sqlalchemy#255

Then:

with app.app_context():
    if db.engine.url.drivername == 'sqlite':
        migrate.init_app(app, db, render_as_batch=True)
    else:
        migrate.init_app(app, db)

and suddenly realised the multidb problem. You need to use render_as_batch from the start and on all migrations for a particular DB or you still risk getting unnamed constraint issues later down the line. This means sqlite-specific migrations, or always using render_as_batch.

Did you get to do much experimenting with this? I'm wondering if there's any reason not to always set render_as_batch=True?

@nMustaki
Copy link

I stopped playing with it because it messed with the db constraints (they disappeared after migration) and I had no time to debug it (sadly).

Reading the doc, render_as_batch only purpose is to handle sqlite while being compatible with others db:

On other backends, we’d see the usual ALTER statements done as though there were no batch directive - the batch context by default only does the “move and copy” process if SQLite is in use
https://alembic.readthedocs.org/en/latest/batch.html#batch-migrations

Docs still warn that is beta code, so I would be wary to enable it by default for now if it is not needed. Sqlite users have no choice though, so we could provide a working env.py and docs ?

@miguelgrinberg
Copy link
Owner

To be honest, I'm not sure I want to encourage the use of this feature until it becomes rock solid.

@nMustaki
Copy link

The fact is that any migration of sqlite need this feature because the
alternative is to manually migrate.
The feature will be stable someday so we can start documenting its use with
appropriate warnings

On Sat, 16 Apr 2016 00:28 Miguel Grinberg, notifications@github.com wrote:

To be honest, I'm not sure I want to encourage the use of this feature
until it becomes rock solid.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#61 (comment)

@miguelgrinberg miguelgrinberg self-assigned this May 31, 2016
openstack-gerrit pushed a commit to ansible-community/ara that referenced this issue Aug 6, 2017
The naming convention allows us to properly name constraints which
some database engines (such as SQLite) don't natively do by default.

The fact that SQLite doesn't name constraints ends up being a huge
pain to manage if you don't name them first [1][2] so let's do that
now.

[1]: miguelgrinberg/Flask-Migrate#61
[2]: http://alembic.zzzcomputing.com/en/latest/batch.html#dealing-with-constraints

Change-Id: Ib4bc7b73f4c18f8d4f666f526be51f81d7b2bed4
@desmond75
Copy link

Hello  Miguelgrinberg  thanks for this tutorial, please i need help i get this error when i want to upgrade my database after migration "No support for ALTER of constraints in SQLite dialect")

NotImplementedError: No support for ALTER of constraints in SQLite dialect

@miguelgrinberg
Copy link
Owner

@desmond75 There is nothing I can do. SQLite has limited support for modifying existing columns. If you need to make these type of changes without complications you may want to consider using MySQL or Postgres. Alembic has an experimental support for applying changes by moving all the data to a new table, but I haven't had much experience with that. See above in this issue for people who have tried that option.

@ashishnitinpatil
Copy link

ashishnitinpatil commented Oct 17, 2017

Mark Steward's solution worked perfectly for me for the case of adding a simple foreignkey column.

@and-semakin
Copy link

Can somebody post full env.py file with Mark Steward's solution merged please?

@ashishnitinpatil
Copy link

ashishnitinpatil commented Jan 14, 2018

@BrokeRU This is the code (maybe old now) I was referring to in my above comment - This is the file you are looking for env.py, however, this is the commit fix which doesn't touch the env.py

redshiftzero added a commit to freedomofpress/securedrop that referenced this issue Jul 10, 2018
redshiftzero added a commit to freedomofpress/securedrop that referenced this issue Jul 17, 2018
emkll pushed a commit to freedomofpress/securedrop that referenced this issue Jul 24, 2018
@pfabri
Copy link

pfabri commented Feb 24, 2019

For those who struggle to put this all together, here's an explanation video I found very helpful.

noahbrenner added a commit to noahbrenner/instrument-catalog-py that referenced this issue Jan 23, 2020
Sqlite doesn't support dropping columns of existing tables, so we need
to use batch processing, which creates a new table, migrates the data,
deletes the old table, and renames the new table to the original's name:
miguelgrinberg/Flask-Migrate#61 (comment)

The existing migration version files also needed to be edited:
https://www.youtube.com/watch?v=CxCK1DkikgA
@sgabello
Copy link

sgabello commented Mar 3, 2020

On StackOverflow, the user yarlen has found a way more elegant solution to this. Check the last answer here: https://stackoverflow.com/questions/30394222/why-flask-migrate-cannot-upgrade-when-drop-column/54322445#54322445

with app.app_context():
if db.engine.url.drivername == 'sqlite':
    migrate.init_app(app, db, render_as_batch=True)
else:
    migrate.init_app(app, db)

By adding the code above on initialisation of the Flask app / migrate module, you automatically enable batch rendering of the migrations ONLY when the database in use is SQLite. No mess around with env.py files, or manual edits to migrations files. This is the safest solution available. This answer on stack overflow should be upvoted and made the correct answer.

@joegiglio
Copy link

@sgabello This definitely seems to be the most elegant solution out there, but unfortunately it does not work for me. I continue to get the dreaded No support for ALTER of constraints in SQLite dialect Please refer to the batch mode feature which allows for SQLite migrations using a copy-and-move strategy.

I added the code to my app file, restarted the app and added some debug print statements to make sure SQLite is being identified correctly and it is but still the error...

Any tips?

@sgabello
Copy link

sgabello commented May 6, 2020

Did you re-build the migrations files from scratch?
If you add the suggested code half-way your app development, only new migrations will use the render_as_batch option and therefore old migration files will still have ALTER commands.

The error message is pretty clear... somewhere in your code you are still using ALTER... potentially you could find where with a simple search in your project folder.

@marksteward
Copy link

@sgabello that's a copy of my comment at #61 (comment). You probably also want the other bits because the default naming convention is slightly off.

@sgabello
Copy link

sgabello commented May 6, 2020

Totally missed your piece of code. Probably my eyes skipped your comment because of the naming convention tweaking code that in my case was not necessary. Didn't mean to steal any merit... Apologies! 🤦 And btw... I made it pretty clear that the code in my comment was not my fix but rather someone's on SO anyway 😄

@joegiglio
Copy link

Did you re-build the migrations files from scratch?
If you add the suggested code half-way your app development, only new migrations will use the render_as_batch option and therefore old migration files will still have ALTER commands.

Thanks. That was it. Over that hurdle and now on to the next battle: ValueError: Constraint must have a name. Sigh...

I wonder if this is all worth it on a dev environment. I could just blow it away and start over but was trying to "do it right".

@gurkin33
Copy link

gurkin33 commented May 9, 2022

For those who struggle to put this all together, here's an explanation video I found very helpful.

@pfabri , many thanks, video was very helpful.

I tried to add new column to a table but also add foreign key. Add column works create but I had issue with foreign key, I applied some logic described in the video and finally I have this upgrade function:

def upgrade():
    op.add_column('api_users', sa.Column('group_id', sa.Integer(), nullable=True))
    with op.batch_alter_table('api_users') as batch_op:
        batch_op.create_foreign_key(None, 'api_user_groups', ['group_id'], ['id'])

before it was (pay attention, when you use batch_alter_table you have to remove target table name from action function, for example, the main difference between create_foreign_key for both cases is that I removed target column name 'api_users'):

def upgrade():
    op.add_column('api_users', sa.Column('group_id', sa.Integer(), nullable=True))
    op.create_foreign_key(None, 'api_users', 'api_user_groups', ['group_id'], ['id'])

Hope it helps to some one 😃

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