Open jdavcs opened 1 year ago
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_table
part 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.
Thanks for the fast reply!
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.
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.
not understanding the
legacy_alter_table
part 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.
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:
$ sqlite3 demo.sqlite
SQLite version 3.34.1 2021-01-20 14:10:07
sqlite> CREATE TABLE foo (id INTEGER, data INTEGER);
CREATE VIEW fooview AS SELECT id FROM foo;
CREATE TABLE tmp_foo (id INTEGER);
DROP TABLE foo;
# At this point we have the new tmp_foo table and the old view.
# The drop succeeded, and the view points to the old, now non-existent table.
sqlite> .tables
fooview tmp_foo
sqlite> .schema fooview
CREATE VIEW fooview AS SELECT id FROM foo;
# Now trigger the error:
sqlite> ALTER TABLE tmp_foo RENAME TO foo;
Error: error in view fooview: no such table: main.foo
# My guess is that it checked the sqlite_schema table and found an entry that doesn't parse
(the view pointing to foo):
sqlite> select * from sqlite_schema;
view|fooview|fooview|0|CREATE VIEW fooview AS SELECT id FROM foo
table|tmp_foo|tmp_foo|3|CREATE TABLE tmp_foo (id INTEGER)
# So here if we issue the PRAGMA statement enabling the legacy mode, I think it does NOT make that check:
sqlite> PRAGMA legacy_alter_table=1;
sqlite> ALTER TABLE tmp_foo RENAME TO foo;
# And we end with no errors and the expected schema/database state:
sqlite> .schema fooview
CREATE VIEW fooview AS SELECT id FROM foo
/* fooview(id) */;
sqlite> .tables
foo fooview
With regard to that statement affecting the RENAME operation, I've checked, and here's what it does:
looking at the problem I would think it's not solvable unless the view is fully dropped and recreated outside the block.
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.
I'm not sure if there is anything to do here. @jdavcs what would a solution be here?
Is documenting that pragma an option?
@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.
Ok, care to provide a PR with some docs? Your implementation could also maybe be a cookbook recipe
Will do, thanks!
Thanks!
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:
The specific schema of foo, as well as the column we are adding/dropping are irrelevant. But here's an example:
Running migrations results in the following:
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):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 thePRAGMA 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:Versions.