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

Error if you try to transform a table that is part of a view #35

Closed
simonw opened this issue Aug 18, 2023 · 9 comments
Closed

Error if you try to transform a table that is part of a view #35

simonw opened this issue Aug 18, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@simonw
Copy link
Owner

simonw commented Aug 18, 2023

I tried dropping one of the columns in the pypi_releases table from a copy of https://datasette.io/content and got this weird error:

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

It turns out that's because the https://datasette.io/content/plugins SQL view includes a reference to that table, and SQLite doesn't like the pattern of renaming the table to a temporary table, dropping the original and renaming it back again while there's a view that references it.

@simonw simonw added the bug Something isn't working label Aug 18, 2023
@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

Here's the code at fault:

def transform_the_table(conn):
sqlite_utils.Database(conn)[table].transform(
types=types,
rename=rename,
drop=drop,
column_order=[p[0] for p in order_pairs],
)

I played around a bit and this genuinely seemed to fix it - and since the views are dropped and recreated within the transaction it looks like this might be a safe solution generally:

            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)

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

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?

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

Well this is strange. I can't replicate the bug on this version of SQLite:

sqlite3.connect(":memory:").execute('select sqlite_version()').fetchall()
[('3.39.5',)]

But I can on this one:

sqlite3.connect(":memory:").execute('select sqlite_version()').fetchall()
[('3.42.0',)]

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

Here's /-/versions.json from the Datasette install that does not suffer from this bug:

{
  "python": {
    "version": "3.8.17",
    "full": "3.8.17 (default, Jul 10 2023, 14:05:18) \n[Clang 14.0.0 (clang-1400.0.29.202)]"
  },
  "datasette": {
    "version": "0.64.3"
  },
  "asgi": "3.0",
  "uvicorn": "0.23.2",
  "sqlite": {
    "version": "3.39.5",
    "fts_versions": [
      "FTS5",
      "FTS4",
      "FTS3"
    ],
    "extensions": {
      "json1": null
    },
    "compile_options": [
      "ATOMIC_INTRINSICS=1",
      "BUG_COMPATIBLE_20160819",
      "CCCRYPT256",
      "COMPILER=clang-14.0.3",
      "DEFAULT_AUTOVACUUM",
      "DEFAULT_CACHE_SIZE=2000",
      "DEFAULT_CKPTFULLFSYNC",
      "DEFAULT_FILE_FORMAT=4",
      "DEFAULT_JOURNAL_SIZE_LIMIT=32768",
      "DEFAULT_LOOKASIDE=1200,102",
      "DEFAULT_MEMSTATUS=0",
      "DEFAULT_MMAP_SIZE=0",
      "DEFAULT_PAGE_SIZE=4096",
      "DEFAULT_PCACHE_INITSZ=20",
      "DEFAULT_RECURSIVE_TRIGGERS",
      "DEFAULT_SECTOR_SIZE=4096",
      "DEFAULT_SYNCHRONOUS=2",
      "DEFAULT_WAL_AUTOCHECKPOINT=1000",
      "DEFAULT_WAL_SYNCHRONOUS=1",
      "DEFAULT_WORKER_THREADS=0",
      "ENABLE_API_ARMOR",
      "ENABLE_BYTECODE_VTAB",
      "ENABLE_COLUMN_METADATA",
      "ENABLE_DBSTAT_VTAB",
      "ENABLE_FTS3",
      "ENABLE_FTS3_PARENTHESIS",
      "ENABLE_FTS3_TOKENIZER",
      "ENABLE_FTS4",
      "ENABLE_FTS5",
      "ENABLE_LOCKING_STYLE=1",
      "ENABLE_NORMALIZE",
      "ENABLE_PREUPDATE_HOOK",
      "ENABLE_RTREE",
      "ENABLE_SESSION",
      "ENABLE_SNAPSHOT",
      "ENABLE_SQLLOG",
      "ENABLE_STMT_SCANSTATUS",
      "ENABLE_UNKNOWN_SQL_FUNCTION",
      "ENABLE_UPDATE_DELETE_LIMIT",
      "HAS_CODEC_RESTRICTED",
      "HAVE_ISNAN",
      "MALLOC_SOFT_LIMIT=1024",
      "MAX_ATTACHED=10",
      "MAX_COLUMN=2000",
      "MAX_COMPOUND_SELECT=500",
      "MAX_DEFAULT_PAGE_SIZE=8192",
      "MAX_EXPR_DEPTH=1000",
      "MAX_FUNCTION_ARG=127",
      "MAX_LENGTH=2147483645",
      "MAX_LIKE_PATTERN_LENGTH=50000",
      "MAX_MMAP_SIZE=1073741824",
      "MAX_PAGE_COUNT=1073741823",
      "MAX_PAGE_SIZE=65536",
      "MAX_SQL_LENGTH=1000000000",
      "MAX_TRIGGER_DEPTH=1000",
      "MAX_VARIABLE_NUMBER=500000",
      "MAX_VDBE_OP=250000000",
      "MAX_WORKER_THREADS=8",
      "MUTEX_UNFAIR",
      "OMIT_AUTORESET",
      "OMIT_LOAD_EXTENSION",
      "STMTJRNL_SPILL=131072",
      "SYSTEM_MALLOC",
      "TEMP_STORE=1",
      "THREADSAFE=2",
      "USE_URI"
    ]
  }
}

And from the one that does:

{
  "python": {
    "version": "3.11.4",
    "full": "3.11.4 (main, Jun 20 2023, 17:23:00) [Clang 14.0.3 (clang-1403.0.22.14.1)]"
  },
  "datasette": {
    "version": "1.0a3"
  },
  "asgi": "3.0",
  "uvicorn": "0.23.2",
  "sqlite": {
    "version": "3.42.0",
    "fts_versions": [
      "FTS5",
      "FTS4",
      "FTS3"
    ],
    "extensions": {
      "json1": null
    },
    "compile_options": [
      "ATOMIC_INTRINSICS=1",
      "COMPILER=clang-14.0.3",
      "DEFAULT_AUTOVACUUM",
      "DEFAULT_CACHE_SIZE=-2000",
      "DEFAULT_FILE_FORMAT=4",
      "DEFAULT_JOURNAL_SIZE_LIMIT=-1",
      "DEFAULT_MMAP_SIZE=0",
      "DEFAULT_PAGE_SIZE=4096",
      "DEFAULT_PCACHE_INITSZ=20",
      "DEFAULT_RECURSIVE_TRIGGERS",
      "DEFAULT_SECTOR_SIZE=4096",
      "DEFAULT_SYNCHRONOUS=2",
      "DEFAULT_WAL_AUTOCHECKPOINT=1000",
      "DEFAULT_WAL_SYNCHRONOUS=2",
      "DEFAULT_WORKER_THREADS=0",
      "ENABLE_COLUMN_METADATA",
      "ENABLE_FTS3",
      "ENABLE_FTS3_PARENTHESIS",
      "ENABLE_FTS4",
      "ENABLE_FTS5",
      "ENABLE_GEOPOLY",
      "ENABLE_MATH_FUNCTIONS",
      "ENABLE_PREUPDATE_HOOK",
      "ENABLE_RTREE",
      "ENABLE_SESSION",
      "MALLOC_SOFT_LIMIT=1024",
      "MAX_ATTACHED=10",
      "MAX_COLUMN=2000",
      "MAX_COMPOUND_SELECT=500",
      "MAX_DEFAULT_PAGE_SIZE=8192",
      "MAX_EXPR_DEPTH=1000",
      "MAX_FUNCTION_ARG=127",
      "MAX_LENGTH=1000000000",
      "MAX_LIKE_PATTERN_LENGTH=50000",
      "MAX_MMAP_SIZE=0x7fff0000",
      "MAX_PAGE_COUNT=1073741823",
      "MAX_PAGE_SIZE=65536",
      "MAX_SQL_LENGTH=1000000000",
      "MAX_TRIGGER_DEPTH=1000",
      "MAX_VARIABLE_NUMBER=250000",
      "MAX_VDBE_OP=250000000",
      "MAX_WORKER_THREADS=8",
      "MUTEX_PTHREADS",
      "SYSTEM_MALLOC",
      "TEMP_STORE=1",
      "THREADSAFE=1"
    ]
  }
}

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

The diff between the two (the broken one first, then the working one).

The broken one is the one with GEOPOLY available.

https://gist.github.com/simonw/b828ea31f031a8c01f72d3fa13748a97/revisions

CleanShot 2023-08-17 at 22 45 01@2x

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

That SQLITE_BUG_COMPATIBLE_20160819 thing looked suspicious but it seems to be related to VACUUM so I don't think it's the problem here: https://github.com/sqlite/sqlite/blob/2e0ce58d2cfa6d866f40bdd4bb5330ca4c4225c2/src/vacuum.c#L111-L124

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

Here are the SQLite tests that refer to "error in view" "no such table" - all relating to alter tables: https://github.com/search?q=repo%3Asqlite%2Fsqlite+%22error+in+view%22+%22no+such+table%22&type=code

The most relevant tests seem to be these ones: https://github.com/sqlite/sqlite/blob/2e0ce58d2cfa6d866f40bdd4bb5330ca4c4225c2/test/altertab.test#L276-L292

reset_db
do_execsql_test 9.0 {
  CREATE TABLE t1(a, b, c);
  CREATE VIEW v1 AS SELECT * FROM t2;
}
do_catchsql_test 9.1 {
  ALTER TABLE t1 RENAME TO t3;
} {1 {error in view v1: no such table: main.t2}}
do_execsql_test 9.2 {
  DROP VIEW v1;
  CREATE TRIGGER tr AFTER INSERT ON t1 BEGIN
    INSERT INTO t2 VALUES(new.a);
  END;
}
do_catchsql_test 9.3 {
  ALTER TABLE t1 RENAME TO t3;
} {1 {error in trigger tr: no such table: main.t2}}

Those tests were added in sqlite/sqlite@65372fa - commit message is "Improve the error messages emitted by RENAME TABLE".

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

It looks like this thread figured it out:

Quoted from the SQLite mailing list (not sure if those archives are online anywhere):

"ALTER TABLE ... RENAME TO ... commands fail in new versions of SQLite if the schema contains views or triggers that refer to tables or columns that do not exist. That's what is happening here. To restore the legacy behaviour, run:

PRAGMA legacy_alter_table = 1;

https://sqlite.org/pragma.html#pragma_legacy_alter_table

Dan."

@simonw
Copy link
Owner Author

simonw commented Aug 18, 2023

Confirmed: running this:

python -c 'import sqlite3; print(sqlite3.connect(":memory:").execute("PRAGMA legacy_alter_table").fetchall())'

Returns [(0,)] in the environment that exhibits this bug and [(1,)] in the environment that does not.

simonw added a commit that referenced this issue Aug 18, 2023
@simonw simonw closed this as completed in 334ffab Aug 18, 2023
simonw added a commit that referenced this issue Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant