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

sqlite "rename table" is wrong #1065

Closed
crpdt251 opened this issue Jul 12, 2022 · 4 comments
Closed

sqlite "rename table" is wrong #1065

crpdt251 opened this issue Jul 12, 2022 · 4 comments
Labels
bug Something isn't working op directives sqlite

Comments

@crpdt251
Copy link

Describe the bug
During the batch operation to drop a non-null constraints, the final step is to rename the temporary table to the real table name. However, when the table is in a schema, alembic generates DDL of the form ALTER TABLE scehma._alembic_tmp_table RENAME TO schema.table. However, the sqlite ALTER TABLE syntax does not expect or allow a schema name on the target table name.

Expected behavior
The ALTER TABLE statement should respect the sqlite grammar and be of the form ALTER TABLE scehma._alembic_tmp_table RENAME TO table

To Reproduce

import sqlite3
from alembic.migration import MigrationContext
from alembic.operations import Operations
from sqlalchemy import MetaData
from sqlalchemy import Table
from sqlalchemy import Integer
from sqlalchemy import Column
from sqlalchemy import Index
from sqlalchemy import create_engine

SCHEMA = 'sn'

def _create_connection():
    conn = sqlite3.connect(':memory:')
    conn.execute(f"ATTACH DATABASE ':memory:' AS {SCHEMA}")
    conn.commit()
    return conn

engine = create_engine('sqlite://', creator=_create_connection, echo=True)
conn = engine.connect()

t = Table(
    'the_table',
    MetaData(schema=SCHEMA),
    Column("id", Integer, primary_key=True),
    Column("int_col", Integer, nullable=False),
)
t.drop(bind=conn, checkfirst=True)
t.create(bind=conn)

mc = MigrationContext.configure(conn)
op = Operations(mc)

with op.batch_alter_table("the_table", schema=SCHEMA) as batch_op:
    batch_op.alter_column('int_col', existing_type=Integer, nullable=True)

Error

2022-07-12 10:55:14,607 INFO sqlalchemy.engine.Engine PRAGMA "sn".table_info("the_table")
2022-07-12 10:55:14,607 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 10:55:14,608 INFO sqlalchemy.engine.Engine 
CREATE TABLE sn.the_table (
	id INTEGER NOT NULL, 
	int_col INTEGER NOT NULL, 
	PRIMARY KEY (id)
)


2022-07-12 10:55:14,608 INFO sqlalchemy.engine.Engine [no key 0.00003s] ()
2022-07-12 10:55:14,608 INFO sqlalchemy.engine.Engine COMMIT
2022-07-12 10:55:14,608 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2022-07-12 10:55:14,608 INFO sqlalchemy.engine.Engine PRAGMA "sn".table_xinfo("the_table")
2022-07-12 10:55:14,608 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine SELECT sql FROM  (SELECT * FROM "sn".sqlite_master UNION ALL   SELECT * FROM "sn".sqlite_temp_master) WHERE name = ? AND type = 'table'
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine SELECT sql FROM "sn".sqlite_master WHERE name = ? AND type = 'table'
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine PRAGMA "sn".foreign_key_list("the_table")
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine SELECT sql FROM  (SELECT * FROM "sn".sqlite_master UNION ALL   SELECT * FROM "sn".sqlite_temp_master) WHERE name = ? AND type = 'table'
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine SELECT sql FROM "sn".sqlite_master WHERE name = ? AND type = 'table'
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine PRAGMA "sn".index_list("the_table")
2022-07-12 10:55:14,609 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine PRAGMA "sn".index_list("the_table")
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine SELECT sql FROM  (SELECT * FROM "sn".sqlite_master UNION ALL   SELECT * FROM "sn".sqlite_temp_master) WHERE name = ? AND type = 'table'
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine SELECT sql FROM "sn".sqlite_master WHERE name = ? AND type = 'table'
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine 
CREATE TABLE sn._alembic_tmp_the_table (
	id INTEGER NOT NULL, 
	int_col INTEGER, 
	PRIMARY KEY (id)
)


2022-07-12 10:55:14,610 INFO sqlalchemy.engine.Engine [no key 0.00003s] ()
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine INSERT INTO sn._alembic_tmp_the_table (id, int_col) SELECT sn.the_table.id, sn.the_table.int_col 
FROM sn.the_table
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine [generated in 0.00004s] ()
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine 
DROP TABLE sn.the_table
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine [no key 0.00002s] ()
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine ALTER TABLE sn._alembic_tmp_the_table RENAME TO sn.the_table
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine [no key 0.00002s] ()
2022-07-12 10:55:14,611 INFO sqlalchemy.engine.Engine ROLLBACK
Traceback (most recent call last):
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1819, in _execute_context
    self.dialect.do_execute(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
    cursor.execute(statement, parameters)
sqlite3.OperationalError: near ".": syntax error

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/user/src/alembic-bug/bug.py", line 36, in <module>
    batch_op.alter_column('int_col', existing_type=Integer, nullable=True)
  File "/Users/user/homebrew/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/contextlib.py", line 126, in __exit__
    next(self.gen)
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/alembic/operations/base.py", line 376, in batch_alter_table
    impl.flush()
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/alembic/operations/batch.py", line 144, in flush
    batch_impl._create(self.impl)
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/alembic/operations/batch.py", line 457, in _create
    op_impl.rename_table(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/alembic/ddl/impl.py", line 346, in rename_table
    self._exec(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/alembic/ddl/impl.py", line 195, in _exec
    return conn.execute(construct, multiparams)
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1306, in execute
    return meth(self, multiparams, params, _EMPTY_EXECUTION_OPTS)
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/sql/ddl.py", line 80, in _execute_on_connection
    return connection._execute_ddl(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1398, in _execute_ddl
    ret = self._execute_context(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1862, in _execute_context
    self._handle_dbapi_exception(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 2043, in _handle_dbapi_exception
    util.raise_(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/util/compat.py", line 208, in raise_
    raise exception
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1819, in _execute_context
    self.dialect.do_execute(
  File "/Users/user/src/alembic-bug/alembic-bug/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) near ".": syntax error
[SQL: ALTER TABLE sn._alembic_tmp_the_table RENAME TO sn.the_table]

Versions.

  • OS: macOS 12.4
  • Python: 3.9.12
  • Alembic: 1.8.0
  • SQLAlchemy: 1.4.39
  • Database: built-in sqlite
  • DBAPI: n/a

Have a nice day!
Thanks!

@crpdt251 crpdt251 added the requires triage New issue that requires categorization label Jul 12, 2022
@zzzeek zzzeek added bug Something isn't working op directives sqlite and removed requires triage New issue that requires categorization labels Jul 12, 2022
@zzzeek zzzeek changed the title Incorrect sqlite grammar when dropping not null constraint on table in a schema sqlite "rename table" is wrong Jul 12, 2022
@zzzeek
Copy link
Member

zzzeek commented Jul 12, 2022

this is not related to constraints or batch, this is the RENAME TABLE command op, can you confirm this patch resolves thanks

diff --git a/alembic/ddl/sqlite.py b/alembic/ddl/sqlite.py
index 9b38766..f986c32 100644
--- a/alembic/ddl/sqlite.py
+++ b/alembic/ddl/sqlite.py
@@ -11,12 +11,17 @@ from sqlalchemy import cast
 from sqlalchemy import JSON
 from sqlalchemy import schema
 from sqlalchemy import sql
+from sqlalchemy.ext.compiler import compiles
 
+from .base import alter_table
+from .base import format_table_name
+from .base import RenameTable
 from .impl import DefaultImpl
 from .. import util
 
 if TYPE_CHECKING:
     from sqlalchemy.engine.reflection import Inspector
+    from sqlalchemy.sql.compiler import DDLCompiler
     from sqlalchemy.sql.elements import Cast
     from sqlalchemy.sql.elements import ClauseElement
     from sqlalchemy.sql.schema import Column
@@ -178,6 +183,16 @@ class SQLiteImpl(DefaultImpl):
             )
 
 
+@compiles(RenameTable, "sqlite")
+def visit_rename_table(
+    element: "RenameTable", compiler: "DDLCompiler", **kw
+) -> str:
+    return "%s RENAME TO %s" % (
+        alter_table(compiler, element.table_name, element.schema),
+        format_table_name(compiler, element.new_table_name, None),
+    )
+
+
 # @compiles(AddColumn, 'sqlite')
 # def visit_add_column(element, compiler, **kw):
 #    return "%s %s" % (

@crpdt251
Copy link
Author

Yep, that works as expected:

$ python bug.py                                                                                                            
2022-07-12 17:28:15,240 INFO sqlalchemy.engine.Engine PRAGMA "sn".table_info("the_table")
2022-07-12 17:28:15,240 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 17:28:15,240 INFO sqlalchemy.engine.Engine 
CREATE TABLE sn.the_table (
	id INTEGER NOT NULL, 
	int_col INTEGER NOT NULL, 
	PRIMARY KEY (id)
)


2022-07-12 17:28:15,240 INFO sqlalchemy.engine.Engine [no key 0.00004s] ()
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine COMMIT
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine PRAGMA "sn".table_xinfo("the_table")
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine SELECT sql FROM  (SELECT * FROM "sn".sqlite_master UNION ALL   SELECT * FROM "sn".sqlite_temp_master) WHERE name = ? AND type = 'table'
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine SELECT sql FROM "sn".sqlite_master WHERE name = ? AND type = 'table'
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine PRAGMA "sn".foreign_key_list("the_table")
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine SELECT sql FROM  (SELECT * FROM "sn".sqlite_master UNION ALL   SELECT * FROM "sn".sqlite_temp_master) WHERE name = ? AND type = 'table'
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 17:28:15,241 INFO sqlalchemy.engine.Engine SELECT sql FROM "sn".sqlite_master WHERE name = ? AND type = 'table'
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine PRAGMA "sn".index_list("the_table")
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine PRAGMA "sn".index_list("the_table")
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine [raw sql] ()
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine SELECT sql FROM  (SELECT * FROM "sn".sqlite_master UNION ALL   SELECT * FROM "sn".sqlite_temp_master) WHERE name = ? AND type = 'table'
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine SELECT sql FROM "sn".sqlite_master WHERE name = ? AND type = 'table'
2022-07-12 17:28:15,242 INFO sqlalchemy.engine.Engine [raw sql] ('the_table',)
2022-07-12 17:28:15,243 INFO sqlalchemy.engine.Engine 
CREATE TABLE sn._alembic_tmp_the_table (
	id INTEGER NOT NULL, 
	int_col INTEGER, 
	PRIMARY KEY (id)
)


2022-07-12 17:28:15,243 INFO sqlalchemy.engine.Engine [no key 0.00003s] ()
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine INSERT INTO sn._alembic_tmp_the_table (id, int_col) SELECT sn.the_table.id, sn.the_table.int_col 
FROM sn.the_table
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine [generated in 0.00004s] ()
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine 
DROP TABLE sn.the_table
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine [no key 0.00002s] ()
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine ALTER TABLE sn._alembic_tmp_the_table RENAME TO the_table
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine [no key 0.00002s] ()
2022-07-12 17:28:15,244 INFO sqlalchemy.engine.Engine COMMIT

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

implement SQLite RENAME TABLE w schema syntax https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/3985

@zzzeek
Copy link
Member

zzzeek commented Jul 12, 2022

thanks for confirming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working op directives sqlite
Projects
None yet
Development

No branches or pull requests

3 participants