Repository navigation
Running alter table revisions w/batch operations on SQLite raises an error if table referenced in a view #1207
Description
Describe the bug
This may be not so much an Alembic bug as a limitation of SQLite; nevertheless, the following use case results in an error that is not mentioned in the documentation; besides, there seems to be a relatively simple fix for this.
Consider the following scenario:
- create table foo
- create view myfoo
- add a revision that adds a column from foo
- run upgrade, then downgrade. The downgrade will raise an error.
The specific schema of foo, as well as the column we are adding/dropping are irrelevant. But here's an example:
########################## REVISION 1 ##########################
def upgrade() -> None:
op.create_table("foo", sa.Column("id", sa.Integer, primary_key=True))
def downgrade() -> None:
op.drop_table("foo")
########################## REVISION 2 ##########################
def upgrade() -> None:
op.execute("CREATE VIEW my_foo AS SELECT id FROM foo")
def downgrade() -> None:
op.execute("DROP VIEW my_foo")
########################## REVISION 3 ##########################
def upgrade() -> None:
with op.batch_alter_table("foo") as batch_op:
batch_op.add_column(sa.Column("stuff", sa.String))
def downgrade() -> None:
with op.batch_alter_table("foo") as batch_op:
batch_op.drop_column("stuff")
# (I can provide the full revision files if needed)Running migrations results in the following:
$ alembic upgrade heads
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running upgrade -> 0a35dd0c400b, create table foo
INFO [alembic.runtime.migration] Running upgrade 0a35dd0c400b -> 70ec189d8dea, create view myfoo
INFO [alembic.runtime.migration] Running upgrade 70ec189d8dea -> c907a34d4a26, add column to table foo
$ alembic downgrade -1
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running downgrade c907a34d4a26 -> 70ec189d8dea, add column to table foo
Traceback (most recent call last):
File "/home/jdavcs/0dev/drafts/python/sqlalchemy/views/v2/.venv/lib64/python3.9/site-packages/sqlalchemy/engine/base.py", line 1964, in _exec_single_context
self.dialect.do_execute(
File "/home/jdavcs/0dev/drafts/python/sqlalchemy/views/v2/.venv/lib64/python3.9/site-packages/sqlalchemy/engine/default.py", line 748, in do_execute
cursor.execute(statement, parameters)
sqlite3.OperationalError: error in view my_foo: no such table: main.foo
... [output truncated]
[SQL: ALTER TABLE _alembic_tmp_foo RENAME TO foo]I believe this is happening primarily due to how SQLite handles the ALTER TABLE statement. The error can be easily reproduced by reducing the logic of the above revisions to the following SQL statements (which is a simplified version of what alembic does):
CREATE TABLE foo (id INTEGER, data INTEGER);
CREATE VIEW fooview AS SELECT id FROM foo;
/* here we drop the `data` column from foo by altering the table via copy/drop/rename */
CREATE TABLE tmp_foo (id INTEGER);
/* skipped step as irrelevant: here we'd copy the rows from foo into tmp_foo */
DROP TABLE foo;
ALTER TABLE tmp_foo RENAME TO foo; /* Error: error in view fooview: no such table: main.foo */Expected behavior
No error, OR a note in the documentation on batch migrations explaining that if a view references the table being altered, and SQLite's version is < 3.35, an error will happen.
To Reproduce
See description. SQLite version: 3.34 (must be < 3.35, as 3.35 introduced the ALTER TABLE DROP COLUMN statement.)
Possible Solution
As per SQLite's documentation, enabling the legacy_alter_table setting should solve this. Indeed, adding the PRAGMA legacy_alter_table=1 statement to the example above appears to solve the problem. The following code runs without errors, making the correct changes to the database:
CREATE TABLE foo (id INTEGER, data INTEGER);
CREATE VIEW fooview AS SELECT id FROM foo;
CREATE TABLE tmp_foo (id INTEGER);
DROP TABLE foo;
PRAGMA legacy_alter_table=1;
ALTER TABLE tmp_foo RENAME TO foo;
PRAGMA legacy_alter_table=0Versions.
- OS: Fedora 34
- Python: 3.9.12
- Alembic: 1.10.2
- SQLAlchemy: 2.0.7 (irrelevant)
- Database: SQLite: 3.34.1
Activity
zzzeek commentedon Mar 20, 2023
hi -
I'm not understanding the SQLite <3.35 requirement. If 3.35 introduced a DROP COLUMN alter, then that's great, but IIUC Alembic is not using that for a SQLite batch migration.
not understanding the
legacy_alter_tablepart either - per the doc, that means our RENAME will not affect the view. But the DROP TABLE foo would still have to error out, I would think.looking at the problem I would think it's not solvable unless the view is fully dropped and recreated outside the block.
jdavcs commentedon Mar 20, 2023
Thanks for the fast reply!
Then I assumed incorrectly. I thought alembic would check if DROP COLUMN was available, and use the copy/rename/drop approach only if it were not available (I noticed that's the behavior on ALTER TABLE ADD COLUMN: my script uses batch ops, but the generated sql just uses the add column statement). In this case <3.35 is irrelevant.
Based on what I see in sqlite, the DROP TABLE doesn't raise an error, it's the RENAME that raises it. Here's a detailed transcript w/inline comments:
With regard to that statement affecting the RENAME operation, I've checked, and here's what it does:
If the legacy_alter_table setting is not an option, then that would be my guess too. In that case, do you think it would make sense to add a note to the documentation? Unless this is too much of an edge case.
Wrap batch ops in pragma statement for sqlite
Wrap batch ops in pragma statement for sqlite
Wrap batch ops in pragma statement for sqlite
Wrap batch ops in pragma statement for sqlite
CaselIT commentedon May 11, 2023
I'm not sure if there is anything to do here. @jdavcs what would a solution be here?
Is documenting that pragma an option?
jdavcs commentedon May 11, 2023
@CaselIT Yes, I think mentioning it in the documentation would be helpful. I've implemented it like this; we use it to wrap our batch operation statements. It solved our problem. If you think this can be added to the docs, I'm happy to open a draft PR.
CaselIT commentedon May 11, 2023
Ok, care to provide a PR with some docs? Your implementation could also maybe be a cookbook recipe
jdavcs commentedon May 11, 2023
Will do, thanks!
CaselIT commentedon May 11, 2023
Thanks!