simonw / sqlite-utils

Python CLI utility and library for manipulating SQLite databases
https://sqlite-utils.datasette.io
Apache License 2.0
1.58k stars 106 forks source link

.transform() fails to drop column if table is part of a view #586

Open simonw opened 11 months ago

simonw commented 11 months ago

I got this error trying to drop a column from a table that was part of a SQL view:

error in view plugins: no such table: main.pypi_releases

Upon further investigation I found that this pattern seemed to fix it:

def transform_the_table(conn):
    # Run this in a transaction:
    with conn:
        # We have to read all the views first, because we need to drop and recreate them
        db = sqlite_utils.Database(conn)
        views = {v.name: v.schema for v in db.views if table.lower() in v.schema.lower()}
        for view in views.keys():
            db[view].drop()
        db[table].transform(
            types=types,
            rename=rename,
            drop=drop,
            column_order=[p[0] for p in order_pairs],
        )
        # Now recreate the views
        for name, schema in views.items():
            db.create_view(name, schema)

So grab a copy of any view that might reference this table, start a transaction, drop those views, run the transform, recreate the views again.

I wonder if this should become an option in sqlite-utils? Maybe a recreate_views=True argument for table.tranform(...)? Should it be opt-in or opt-out?

Originally posted by @simonw in https://github.com/simonw/datasette-edit-schema/issues/35#issuecomment-1683370548

simonw commented 11 months ago

More notes in here:

Not all Python/SQLite installations exhibit this problem by default!

It turns out this is controlled by the legacy_alter_table pragma: https://sqlite.org/pragma.html#pragma_legacy_alter_table

If that PRAGMA is turned on (default in newer SQLites) then alter table will error if you try to rename a table that is referenced in a view.

Here's a one-liner to test if it is on or not:

python -c 'import sqlite3; print(sqlite3.connect(":memory:").execute("PRAGMA legacy_alter_table").fetchall())'
simonw commented 11 months ago

Options:

I'm on the fence as to which of these I like the most. I'm tempted to go with the one which just drops VIEWS and recreates them all the time, because it feels simpler.

simonw commented 11 months ago

I shipped the view recreating fix in datasette-edit-schema, so at least I can start exercising that fix and see if it has any weird issues.

tobych commented 2 months ago

Just noting that the fix @simonw mentions is here in datasette-edit-schema.

I'm using views that reference tables that are changing, so I'm likely to want this fix.

tobych commented 2 months ago

The documentation for the legacy_alter_table pragma states that the behavior it controls is per-connection, and not persistent.

Regardless, using the pragma seems a bad idea to me, because:

The recreate_views idea seems like it might be useful, but I reckon You Ain't Gonna Need It. If someone has a lot of views, and it takes ages to recreate them, perhaps. That seems highly unlikely.