]> git.ipfire.org Git - thirdparty/sqlalchemy/alembic.git/commitdiff
resolve for variant before testing for schema type
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 1 Feb 2022 14:12:17 +0000 (09:12 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Tue, 1 Feb 2022 14:33:12 +0000 (09:33 -0500)
Fixed regression where usage of a ``with_variant()`` datatype in
conjunction with the ``existing_type`` option of ``op.alter_column()``
under batch mode would lead to an internal exception.

note this exception would only occur under 1.x, under 2.0
the new variant architecture would have prevented this problem,
however it is best here that the ultimate variant type is
unwrapped in the case that we have a plain type with a
schema type as a variant.

also sqla 1.3 needs pytest < 7 to run correctly

Change-Id: I0f5480f97f014e707b8bd4e70f8495d3bd27fd21
Fixes: #982
alembic/operations/batch.py
alembic/util/sqla_compat.py
docs/build/unreleased/982.rst [new file with mode: 0644]
tests/test_batch.py
tox.ini

index ef182fdc7ac8a74de9ec120099bb985ad2c4fb39..b163d790cbe23a86fc08fbe6c4ac6d7367d308c5 100644 (file)
@@ -28,6 +28,7 @@ from ..util.sqla_compat import _fk_is_self_referential
 from ..util.sqla_compat import _insert_inline
 from ..util.sqla_compat import _is_type_bound
 from ..util.sqla_compat import _remove_column_from_collection
+from ..util.sqla_compat import _resolve_for_variant
 from ..util.sqla_compat import _select
 
 if TYPE_CHECKING:
@@ -462,16 +463,22 @@ class ApplyBatchImpl:
             existing.name = name
             existing_transfer["name"] = name
 
-            # pop named constraints for Boolean/Enum for rename
-            if (
-                "existing_type" in kw
-                and isinstance(kw["existing_type"], SchemaEventTarget)
-                and kw["existing_type"].name  # type:ignore[attr-defined]
-            ):
-                self.named_constraints.pop(
-                    kw["existing_type"].name, None  # type:ignore[attr-defined]
+            existing_type = kw.get("existing_type", None)
+            if existing_type:
+                resolved_existing_type = _resolve_for_variant(
+                    kw["existing_type"], self.impl.dialect
                 )
 
+                # pop named constraints for Boolean/Enum for rename
+                if (
+                    isinstance(resolved_existing_type, SchemaEventTarget)
+                    and resolved_existing_type.name  # type:ignore[attr-defined]  # noqa E501
+                ):
+                    self.named_constraints.pop(
+                        resolved_existing_type.name,
+                        None,  # type:ignore[attr-defined]
+                    )
+
         if type_ is not None:
             type_ = sqltypes.to_instance(type_)
             # old type is being discarded so turn off eventing
index 0f2762cea41d92da12f7c8af33e26ecd6da7e1b8..0f8c12ff7b5f7dbb0bac43c3a32dd7a6a69d7080 100644 (file)
@@ -250,6 +250,14 @@ def _reflect_table(
         return inspector.reflecttable(table, None)
 
 
+def _resolve_for_variant(type_, dialect):
+    if _type_has_variants(type_):
+        base_type, mapping = _get_variant_mapping(type_)
+        return mapping.get(dialect.name, base_type)
+    else:
+        return type_
+
+
 if hasattr(sqltypes.TypeEngine, "_variant_mapping"):
 
     def _type_has_variants(type_):
diff --git a/docs/build/unreleased/982.rst b/docs/build/unreleased/982.rst
new file mode 100644 (file)
index 0000000..715cb1f
--- /dev/null
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, batch, regression
+    :tickets: 982
+
+    Fixed regression where usage of a ``with_variant()`` datatype in
+    conjunction with the ``existing_type`` option of ``op.alter_column()``
+    under batch mode would lead to an internal exception.
index 700056acb462209ff3acbb5277ff580ad5aec98c..f0bbd7509bacdd623ef2b19cbab20c24679c2d81 100644 (file)
@@ -1138,6 +1138,30 @@ class CopyFromTest(TestBase):
             "ALTER TABLE _alembic_tmp_foo RENAME TO foo",
         )
 
+    def test_change_name_from_existing_variant_type(self):
+        """test #982"""
+        context = self._fixture()
+        self.table.append_column(
+            Column("y", Text().with_variant(Text(10000), "mysql"))
+        )
+
+        with self.op.batch_alter_table(
+            "foo", copy_from=self.table
+        ) as batch_op:
+            batch_op.alter_column(
+                column_name="y",
+                new_column_name="q",
+                existing_type=Text().with_variant(Text(10000), "mysql"),
+            )
+        context.assert_(
+            "CREATE TABLE _alembic_tmp_foo (id INTEGER NOT NULL, "
+            "data VARCHAR(50), x INTEGER, q TEXT, PRIMARY KEY (id))",
+            "INSERT INTO _alembic_tmp_foo (id, data, x, q) "
+            "SELECT foo.id, foo.data, foo.x, foo.y FROM foo",
+            "DROP TABLE foo",
+            "ALTER TABLE _alembic_tmp_foo RENAME TO foo",
+        )
+
     def test_change_type_to_schematype(self):
         context = self._fixture()
         self.table.append_column(Column("y", Integer))
diff --git a/tox.ini b/tox.ini
index f554f6306222f377f1758833da7376e68ded51ed..58a7df76d3db83ce2208bd84784b0a5626378af0 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -10,6 +10,7 @@ cov_args=--cov=alembic --cov-report term --cov-report xml
 
 deps=pytest>4.6
      pytest-xdist
+     sqla13: pytest<7
      sqla13: {[tox]SQLA_REPO}@rel_1_3#egg=sqlalchemy
      sqla14: {[tox]SQLA_REPO}@rel_1_4#egg=sqlalchemy
      sqlamain: {[tox]SQLA_REPO}#egg=sqlalchemy