Do not CAST(x as JSON) in SQLite

Fixed issue where the CAST applied to a JSON column when copying a SQLite
table during batch mode would cause the data to be lost, as SQLite's CAST
with JSON appears to convert the data to the value "0". The CAST is now
skipped in a dialect-specific manner, including for JSON columns on SQLite.
Pull request courtesy Sebastián Ramírez.

Fixes: #697
Closes: #698
Pull-request: https://github.com/sqlalchemy/alembic/pull/698
Pull-request-sha: 6618325258bd90ec257b09c17d44421a5642b1b1

Change-Id: Ia152ea7386e64efb2194aa836dc57754f979e204

diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py
index 9df2a72dfb720b7ff1ed1b25d252e48fd27a76dd..7b32b01d1ebd40600afbbb1369bee34b7bb14ce7 100644 (file)
--- a/alembic/ddl/impl.py
+++ b/alembic/ddl/impl.py
@@ -1,6 +1,7 @@
 from collections import namedtuple
 import re
 
+from sqlalchemy import cast
 from sqlalchemy import schema
 from sqlalchemy import text
 
@@ -449,6 +450,12 @@ class DefaultImpl(with_metaclass(ImplMeta)):
     ):
         pass
 
+    def cast_for_batch_migrate(self, existing, existing_transfer, new_type):
+        if existing.type._type_affinity is not new_type._type_affinity:
+            existing_transfer["expr"] = cast(
+                existing_transfer["expr"], new_type
+            )
+
     def render_ddl_sql_expr(self, expr, is_server_default=False, **kw):
         """Render a SQL expression that is typically a server default,
         index expression, etc.

diff --git a/alembic/ddl/sqlite.py b/alembic/ddl/sqlite.py
index 84e9dd8962fad41357bb26c29169d2826c19979a..8b78966a113f0669a2746a97f8bb64fb592816de 100644 (file)
--- a/alembic/ddl/sqlite.py
+++ b/alembic/ddl/sqlite.py
@@ -1,5 +1,8 @@
 import re
 
+from sqlalchemy import cast
+from sqlalchemy import JSON
+
 from .impl import DefaultImpl
 from .. import util
 
@@ -117,6 +120,15 @@ class SQLiteImpl(DefaultImpl):
             str_expr = "(%s)" % (str_expr,)
         return str_expr
 
+    def cast_for_batch_migrate(self, existing, existing_transfer, new_type):
+        if (
+            existing.type._type_affinity is not new_type._type_affinity
+            and not isinstance(new_type, JSON)
+        ):
+            existing_transfer["expr"] = cast(
+                existing_transfer["expr"], new_type
+            )
+
 
 # @compiles(AddColumn, 'sqlite')
 # def visit_add_column(element, compiler, **kw):

diff --git a/alembic/operations/batch.py b/alembic/operations/batch.py
index 6ca6f90c9f260b1b9c6b4a2aec780c819f73f8d8..c46180167d1cb845551de77dd6a6336ba3d99e37 100644 (file)
--- a/alembic/operations/batch.py
+++ b/alembic/operations/batch.py
@@ -1,4 +1,3 @@
-from sqlalchemy import cast
 from sqlalchemy import CheckConstraint
 from sqlalchemy import Column
 from sqlalchemy import ForeignKeyConstraint
@@ -103,6 +102,7 @@ class BatchOperationsImpl(object):
                 reflected = True
 
             batch_impl = ApplyBatchImpl(
+                self.impl,
                 existing_table,
                 self.table_args,
                 self.table_kwargs,
@@ -155,8 +155,15 @@ class BatchOperationsImpl(object):
 
 class ApplyBatchImpl(object):
     def __init__(
-        self, table, table_args, table_kwargs, reflected, partial_reordering=()
+        self,
+        impl,
+        table,
+        table_args,
+        table_kwargs,
+        reflected,
+        partial_reordering=(),
     ):
+        self.impl = impl
         self.table = table  # this is a Table object
         self.table_args = table_args
         self.table_kwargs = table_kwargs
@@ -405,10 +412,9 @@ class ApplyBatchImpl(object):
                     existing.type.create_constraint
                 ) = False
 
-            if existing.type._type_affinity is not type_._type_affinity:
-                existing_transfer["expr"] = cast(
-                    existing_transfer["expr"], type_
-                )
+            self.impl.cast_for_batch_migrate(
+                existing, existing_transfer, type_
+            )
 
             existing.type = type_
 

diff --git a/docs/build/unreleased/697.rst b/docs/build/unreleased/697.rst
new file mode 100644 (file)
index 0000000..0eff4a0
--- /dev/null
+++ b/docs/build/unreleased/697.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, sqlite, batch
+    :tickets: 697
+
+    Fixed issue where the CAST applied to a JSON column when copying a SQLite
+    table during batch mode would cause the data to be lost, as SQLite's CAST
+    with JSON appears to convert the data to the value "0". The CAST is now
+    skipped in a dialect-specific manner, including for JSON columns on SQLite.
+    Pull request courtesy Sebastián Ramírez.

diff --git a/tests/test_batch.py b/tests/test_batch.py
index 4c32518d3c5dcbfd7671a551783d84c8ebb3f1d3..c88ec53787c9e2133ad21b1ff6d6d9d3ef9d8da7 100644 (file)
--- a/tests/test_batch.py
+++ b/tests/test_batch.py
@@ -13,17 +13,21 @@ from sqlalchemy import func
 from sqlalchemy import Index
 from sqlalchemy import inspect
 from sqlalchemy import Integer
+from sqlalchemy import JSON
 from sqlalchemy import MetaData
 from sqlalchemy import PrimaryKeyConstraint
 from sqlalchemy import String
 from sqlalchemy import Table
+from sqlalchemy import Text
 from sqlalchemy import UniqueConstraint
+from sqlalchemy.dialects import sqlite as sqlite_dialect
 from sqlalchemy.schema import CreateIndex
 from sqlalchemy.schema import CreateTable
 from sqlalchemy.sql import column
 from sqlalchemy.sql import select
 from sqlalchemy.sql import text
 
+from alembic.ddl import sqlite
 from alembic.operations import Operations
 from alembic.operations.batch import ApplyBatchImpl
 from alembic.runtime.migration import MigrationContext
@@ -41,6 +45,9 @@ from alembic.util.sqla_compat import sqla_14
 class BatchApplyTest(TestBase):
     def setUp(self):
         self.op = Operations(mock.Mock(opts={}))
+        self.impl = sqlite.SQLiteImpl(
+            sqlite_dialect.dialect(), None, False, False, None, {}
+        )
 
     def _simple_fixture(self, table_args=(), table_kwargs={}, **kw):
         m = MetaData()
@@ -51,7 +58,9 @@ class BatchApplyTest(TestBase):
             Column("x", String(10)),
             Column("y", Integer),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False, **kw)
+        return ApplyBatchImpl(
+            self.impl, t, table_args, table_kwargs, False, **kw
+        )
 
     def _uq_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -63,7 +72,7 @@ class BatchApplyTest(TestBase):
             Column("y", Integer),
             UniqueConstraint("y", name="uq1"),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _ix_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -75,7 +84,7 @@ class BatchApplyTest(TestBase):
             Column("y", Integer),
             Index("ix1", "y"),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _pk_fixture(self):
         m = MetaData()
@@ -87,7 +96,7 @@ class BatchApplyTest(TestBase):
             Column("y", Integer),
             PrimaryKeyConstraint("id", name="mypk"),
         )
-        return ApplyBatchImpl(t, (), {}, False)
+        return ApplyBatchImpl(self.impl, t, (), {}, False)
 
     def _literal_ck_fixture(
         self, copy_from=None, table_args=(), table_kwargs={}
@@ -103,7 +112,7 @@ class BatchApplyTest(TestBase):
                 Column("email", String()),
                 CheckConstraint("email LIKE '%@%'"),
             )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _sql_ck_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -114,7 +123,7 @@ class BatchApplyTest(TestBase):
             Column("email", String()),
         )
         t.append_constraint(CheckConstraint(t.c.email.like("%@%")))
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _fk_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -125,7 +134,7 @@ class BatchApplyTest(TestBase):
             Column("email", String()),
             Column("user_id", Integer, ForeignKey("user.id")),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _multi_fk_fixture(self, table_args=(), table_kwargs={}, schema=None):
         m = MetaData()
@@ -149,7 +158,7 @@ class BatchApplyTest(TestBase):
             ),
             schema=schema,
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _named_fk_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -160,7 +169,7 @@ class BatchApplyTest(TestBase):
             Column("email", String()),
             Column("user_id", Integer, ForeignKey("user.id", name="ufk")),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _selfref_fk_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -171,7 +180,7 @@ class BatchApplyTest(TestBase):
             Column("parent_id", Integer, ForeignKey("tname.id")),
             Column("data", String),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _boolean_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -181,7 +190,7 @@ class BatchApplyTest(TestBase):
             Column("id", Integer, primary_key=True),
             Column("flag", Boolean),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _boolean_no_ck_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -191,7 +200,7 @@ class BatchApplyTest(TestBase):
             Column("id", Integer, primary_key=True),
             Column("flag", Boolean(create_constraint=False)),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _enum_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -201,7 +210,7 @@ class BatchApplyTest(TestBase):
             Column("id", Integer, primary_key=True),
             Column("thing", Enum("a", "b", "c")),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _server_default_fixture(self, table_args=(), table_kwargs={}):
         m = MetaData()
@@ -211,7 +220,7 @@ class BatchApplyTest(TestBase):
             Column("id", Integer, primary_key=True),
             Column("thing", String(), server_default=""),
         )
-        return ApplyBatchImpl(t, table_args, table_kwargs, False)
+        return ApplyBatchImpl(self.impl, t, table_args, table_kwargs, False)
 
     def _assert_impl(
         self,
@@ -984,18 +993,28 @@ class CopyFromTest(TestBase):
         self.op = Operations(context)
         return context
 
+    @config.requirements.sqlalchemy_13
     def test_change_type(self):
         context = self._fixture()
+        self.table.append_column(Column("toj", Text))
+        self.table.append_column(Column("fromj", JSON))
         with self.op.batch_alter_table(
             "foo", copy_from=self.table
         ) as batch_op:
             batch_op.alter_column("data", type_=Integer)
+            batch_op.alter_column("toj", type_=JSON)
+            batch_op.alter_column("fromj", type_=Text)
         context.assert_(
             "CREATE TABLE _alembic_tmp_foo (id INTEGER NOT NULL, "
-            "data INTEGER, x INTEGER, PRIMARY KEY (id))",
-            "INSERT INTO _alembic_tmp_foo (id, data, x) SELECT foo.id, "
-            "CAST(foo.data AS INTEGER) AS %s, foo.x FROM foo"
-            % (("data" if sqla_14 else "anon_1"),),
+            "data INTEGER, x INTEGER, toj JSON, fromj TEXT, PRIMARY KEY (id))",
+            "INSERT INTO _alembic_tmp_foo (id, data, x, toj, fromj) "
+            "SELECT foo.id, "
+            "CAST(foo.data AS INTEGER) AS %s, foo.x, foo.toj, "
+            "CAST(foo.fromj AS TEXT) AS %s FROM foo"
+            % (
+                ("data" if sqla_14 else "anon_1"),
+                ("fromj" if sqla_14 else "anon_2"),
+            ),
             "DROP TABLE foo",
             "ALTER TABLE _alembic_tmp_foo RENAME TO foo",
         )