cast() types in batch only if type_affinity is different

Batch mode will not use CAST() to copy data if type_ is given, however
the basic type affinity matches that of the existing type.  This to
avoid SQLite's CAST of TIMESTAMP which results in truncation of the
data, in those cases where the user needs to add redundant type_ for
other reasons.

Change-Id: I20f7b399cd3cd7740d67ff7d624aa1da874ebc71
Fixes: #391

diff --git a/alembic/operations/batch.py b/alembic/operations/batch.py
index 2a859ef5fc480db7679f01eac46e89d44aa72b11..24c1a816ca412787c0320ff9d305d2423b52b3d5 100644 (file)
--- a/alembic/operations/batch.py
+++ b/alembic/operations/batch.py
@@ -301,6 +301,10 @@ class ApplyBatchImpl(object):
                 existing.type._create_events = \
                     existing.type.create_constraint = False
 
+            if existing.type._type_affinity is not type_._type_affinity:
+                existing_transfer["expr"] = cast(
+                    existing_transfer["expr"], type_)
+
             existing.type = type_
 
             # we *dont* however set events for the new type, because
@@ -308,7 +312,6 @@ class ApplyBatchImpl(object):
             # Operations.implementation_for(alter_column) which already
             # will emit an add_constraint()
 
-            existing_transfer["expr"] = cast(existing_transfer["expr"], type_)
         if nullable is not None:
             existing.nullable = nullable
         if server_default is not False:

diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index b78eaa7bfaf907d1ff2936717c0b65743d8a1ba5..14ad7051a28cd16a3158fef04a4114452b2457ee 100644 (file)
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -6,6 +6,16 @@ Changelog
 .. changelog::
     :version: 0.8.9
 
+    .. change::
+      :tags: bug, batch
+      :tickets: 391
+
+      Batch mode will not use CAST() to copy data if type_ is given, however
+      the basic type affinity matches that of the existing type.  This to
+      avoid SQLite's CAST of TIMESTAMP which results in truncation of the
+      data, in those cases where the user needs to add redundant type_ for
+      other reasons.
+
     .. change::
       :tags: bug, autogenerate
       :tickets: 393

diff --git a/tests/test_batch.py b/tests/test_batch.py
index 7dad62ffaa5c3bcca66a904abc118f27b0124e2d..3124baa7bf8b3338cdcd5de4a2762c3952c40866 100644 (file)
--- a/tests/test_batch.py
+++ b/tests/test_batch.py
@@ -13,9 +13,9 @@ from alembic.runtime.migration import MigrationContext
 
 from sqlalchemy import Integer, Table, Column, String, MetaData, ForeignKey, \
     UniqueConstraint, ForeignKeyConstraint, Index, Boolean, CheckConstraint, \
-    Enum
+    Enum, DateTime
 from sqlalchemy.engine.reflection import Inspector
-from sqlalchemy.sql import column, text
+from sqlalchemy.sql import column, text, select
 from sqlalchemy.schema import CreateTable, CreateIndex
 from sqlalchemy import exc
 
@@ -215,6 +215,7 @@ class BatchApplyTest(TestBase):
             impl.new_table.c[name].name
             for name in colnames
             if name in impl.table.c])
+
         args['tname_colnames'] = ", ".join(
             "CAST(%(schema)stname.%(name)s AS %(type)s) AS anon_1" % {
                 'schema': args['schema'],
@@ -243,9 +244,9 @@ class BatchApplyTest(TestBase):
 
     def test_change_type(self):
         impl = self._simple_fixture()
-        impl.alter_column('tname', 'x', type_=Integer)
+        impl.alter_column('tname', 'x', type_=String)
         new_table = self._assert_impl(impl)
-        assert new_table.c.x.type._type_affinity is Integer
+        assert new_table.c.x.type._type_affinity is String
 
     def test_rename_col(self):
         impl = self._simple_fixture()
@@ -751,7 +752,6 @@ class CopyFromTest(TestBase):
         with self.op.batch_alter_table(
                 "foo", copy_from=self.table) as batch_op:
             batch_op.alter_column('data', type_=Integer)
-
         context.assert_(
             'CREATE TABLE _alembic_batch_temp (id INTEGER NOT NULL, '
             'data INTEGER, x INTEGER, PRIMARY KEY (id))',
@@ -889,7 +889,7 @@ class CopyFromTest(TestBase):
             'CREATE TABLE _alembic_batch_temp (id INTEGER NOT NULL, '
             'data VARCHAR, x INTEGER, PRIMARY KEY (id))',
             'INSERT INTO _alembic_batch_temp (id, data, x) SELECT foo.id, '
-            'CAST(foo.data AS VARCHAR) AS anon_1, foo.x FROM foo',
+            'foo.data, foo.x FROM foo',
             'DROP TABLE foo',
             'ALTER TABLE _alembic_batch_temp RENAME TO foo'
         )
@@ -970,6 +970,14 @@ class BatchRoundTripTest(TestBase):
         )
         t.create(self.conn)
 
+    def _timestamp_fixture(self):
+        t = Table(
+            'hasts', self.metadata,
+            Column('x', DateTime()),
+        )
+        t.create(self.conn)
+        return t
+
     def _int_to_boolean_fixture(self):
         t = Table(
             'hasbool', self.metadata,
@@ -993,6 +1001,23 @@ class BatchRoundTripTest(TestBase):
             [Integer]
         )
 
+    def test_no_net_change_timestamp(self):
+        t = self._timestamp_fixture()
+
+        import datetime
+        self.conn.execute(
+            t.insert(),
+            {"x": datetime.datetime(2012, 5, 18, 15, 32, 5)}
+        )
+
+        with self.op.batch_alter_table("hasts") as batch_op:
+            batch_op.alter_column("x", type_=DateTime())
+
+        eq_(
+            self.conn.execute(select([t.c.x])).fetchall(),
+            [(datetime.datetime(2012, 5, 18, 15, 32, 5),)]
+        )
+
     def test_drop_col_schematype(self):
         self._boolean_fixture()
         with self.op.batch_alter_table(