summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2014-11-29 16:31:30 -0500
committerMike Bayer <mike_mp@zzzcomputing.com>2014-11-29 16:31:30 -0500
commit2851811c9fdaa7b5d62288b31a38b1a8e6c8e8d0 (patch)
tree6983bb387ecbc0aff20e8c81fed8671acaa1992b
parent2a06536d6e4bbdf8e3973877912ada1c0a54664c (diff)
downloadalembic-2851811c9fdaa7b5d62288b31a38b1a8e6c8e8d0.tar.gz
- Repaired the inspection, copying and rendering of CHECK constraints
and so-called "schema" types such as Boolean, Enum within the batch copy system; the CHECK constraint will not be "doubled" when the table is copied, and additionally the inspection of the CHECK constraint for its member columns will no longer fail with an attribute error. fixes #249 - Added two new arguments :paramref:`.Operations.batch_alter_table.reflect_args` and :paramref:`.Operations.batch_alter_table.reflect_kwargs`, so that arguments may be passed directly to suit the :class:`~.sqlalchemy.schema.Table` object that will be reflected.
-rw-r--r--alembic/batch.py24
-rw-r--r--alembic/ddl/base.py37
-rw-r--r--alembic/operations.py30
-rw-r--r--alembic/testing/fixtures.py2
-rw-r--r--docs/build/batch.rst53
-rw-r--r--docs/build/changelog.rst27
-rw-r--r--tests/requirements.py10
-rw-r--r--tests/test_batch.py197
8 files changed, 360 insertions, 20 deletions
diff --git a/alembic/batch.py b/alembic/batch.py
index f5957ca..f5c1f31 100644
--- a/alembic/batch.py
+++ b/alembic/batch.py
@@ -3,11 +3,13 @@ from sqlalchemy import Table, MetaData, Index, select, Column, \
from sqlalchemy import types as sqltypes
from sqlalchemy.util import OrderedDict
from . import util
+from .ddl.base import _columns_for_constraint, _is_type_bound
class BatchOperationsImpl(object):
def __init__(self, operations, table_name, schema, recreate,
- copy_from, table_args, table_kwargs):
+ copy_from, table_args, table_kwargs,
+ reflect_args, reflect_kwargs):
if not util.sqla_08:
raise NotImplementedError(
"batch mode requires SQLAlchemy 0.8 or greater.")
@@ -21,6 +23,8 @@ class BatchOperationsImpl(object):
self.copy_from = copy_from
self.table_args = table_args
self.table_kwargs = table_kwargs
+ self.reflect_args = reflect_args
+ self.reflect_kwargs = reflect_kwargs
self.batch = []
@property
@@ -48,9 +52,13 @@ class BatchOperationsImpl(object):
fn(*arg, **kw)
else:
m1 = MetaData()
+
existing_table = Table(
- self.table_name, m1, schema=self.schema,
- autoload=True, autoload_with=self.operations.get_bind())
+ self.table_name, m1,
+ schema=self.schema,
+ autoload=True,
+ autoload_with=self.operations.get_bind(),
+ *self.reflect_args, **self.reflect_kwargs)
batch_impl = ApplyBatchImpl(
existing_table, self.table_args, self.table_kwargs)
@@ -113,6 +121,8 @@ class ApplyBatchImpl(object):
self.unnamed_constraints = []
self.indexes = {}
for const in self.table.constraints:
+ if _is_type_bound(const):
+ continue
if const.name:
self.named_constraints[const.name] = const
else:
@@ -136,7 +146,7 @@ class ApplyBatchImpl(object):
self.unnamed_constraints:
const_columns = set([
- c.key for c in self._constraint_columns(const)])
+ c.key for c in _columns_for_constraint(const)])
if not const_columns.issubset(self.column_transfers):
continue
@@ -151,12 +161,6 @@ class ApplyBatchImpl(object):
*[new_table.c[col] for col in index.columns.keys()],
**index.kwargs)
- def _constraint_columns(self, constraint):
- if isinstance(constraint, ForeignKeyConstraint):
- return [fk.parent for fk in constraint.elements]
- else:
- return list(constraint.columns)
-
def _setup_referent(self, metadata, constraint):
spec = constraint.elements[0]._get_colspec()
parts = spec.split(".")
diff --git a/alembic/ddl/base.py b/alembic/ddl/base.py
index aa5fbe1..32878b1 100644
--- a/alembic/ddl/base.py
+++ b/alembic/ddl/base.py
@@ -1,9 +1,11 @@
import functools
from sqlalchemy.ext.compiler import compiles
-from sqlalchemy.schema import DDLElement, Column
+from sqlalchemy.schema import DDLElement, Column, \
+ ForeignKeyConstraint, CheckConstraint
from sqlalchemy import Integer
from sqlalchemy import types as sqltypes
+from sqlalchemy.sql.visitors import traverse
from .. import util
if util.sqla_09:
@@ -152,6 +154,39 @@ def visit_column_default(element, compiler, **kw):
)
+def _columns_for_constraint(constraint):
+ if isinstance(constraint, ForeignKeyConstraint):
+ return [fk.parent for fk in constraint.elements]
+ elif isinstance(constraint, CheckConstraint):
+ return _find_columns(constraint.sqltext)
+ else:
+ return list(constraint.columns)
+
+
+def _is_type_bound(constraint):
+ # this deals with SQLAlchemy #3260, don't copy CHECK constraints
+ # that will be generated by the type.
+ if util.sqla_100:
+ # new feature added for #3260
+ return constraint._type_bound
+ else:
+ # old way, look at what we know Boolean/Enum to use
+ return (
+ constraint._create_rule is not None and
+ isinstance(
+ getattr(constraint._create_rule, "target", None),
+ sqltypes.SchemaType)
+ )
+
+
+def _find_columns(clause):
+ """locate Column objects within the given expression."""
+
+ cols = set()
+ traverse(clause, {}, {'column': cols.add})
+ return cols
+
+
def quote_dotted(name, quote):
"""quote the elements of a dotted name"""
diff --git a/alembic/operations.py b/alembic/operations.py
index fc52e03..62d4842 100644
--- a/alembic/operations.py
+++ b/alembic/operations.py
@@ -192,7 +192,8 @@ class Operations(object):
@contextmanager
def batch_alter_table(
self, table_name, schema=None, recreate="auto", copy_from=None,
- table_args=(), table_kwargs=util.immutabledict()):
+ table_args=(), table_kwargs=util.immutabledict(),
+ reflect_args=(), reflect_kwargs=util.immutabledict()):
"""Invoke a series of per-table migrations in batch.
Batch mode allows a series of operations specific to a table
@@ -251,6 +252,31 @@ class Operations(object):
:param copy_from: optional :class:`~sqlalchemy.schema.Table` object
that will act as the structure of the table being copied. If omitted,
table reflection is used to retrieve the structure of the table.
+
+ .. seealso::
+
+ :paramref:`~.Operations.batch_alter_table.reflect_args`
+
+ :paramref:`~.Operations.batch_alter_table.reflect_kwargs`
+
+ :param reflect_args: a sequence of additional positional arguments that
+ will be applied to the table structure being reflected / copied;
+ this may be used to pass column and constraint overrides to the
+ table that will be reflected, in lieu of passing the whole
+ :class:`~sqlalchemy.schema.Table` using
+ :paramref:`~.Operations.batch_alter_table.copy_from`.
+
+ .. versionadded:: 0.7.1
+
+ :param reflect_kwargs: a dictionary of additional keyword arguments
+ that will be applied to the table structure being copied; this may be
+ used to pass additional table and reflection options to the table that
+ will be reflected, in lieu of passing the whole
+ :class:`~sqlalchemy.schema.Table` using
+ :paramref:`~.Operations.batch_alter_table.copy_from`.
+
+ .. versionadded:: 0.7.1
+
:param table_args: a sequence of additional positional arguments that
will be applied to the new :class:`~sqlalchemy.schema.Table` when
created, in addition to those copied from the source table.
@@ -273,7 +299,7 @@ class Operations(object):
"""
impl = batch.BatchOperationsImpl(
self, table_name, schema, recreate,
- copy_from, table_args, table_kwargs)
+ copy_from, table_args, table_kwargs, reflect_args, reflect_kwargs)
batch_op = BatchOperations(self.migration_context, impl=impl)
yield batch_op
impl.flush()
diff --git a/alembic/testing/fixtures.py b/alembic/testing/fixtures.py
index f0b0000..6336967 100644
--- a/alembic/testing/fixtures.py
+++ b/alembic/testing/fixtures.py
@@ -100,7 +100,7 @@ def op_fixture(dialect='default', as_sql=False, naming_convention=None):
# TODO: this might need to
# be more like a real connection
# as tests get more involved
- self.connection = None
+ self.connection = mock.Mock(dialect=dialect)
def _exec(self, construct, *args, **kw):
if isinstance(construct, string_types):
diff --git a/docs/build/batch.rst b/docs/build/batch.rst
index 338ab8a..4c03a66 100644
--- a/docs/build/batch.rst
+++ b/docs/build/batch.rst
@@ -61,6 +61,55 @@ which is the one kind of column-level ALTER statement that SQLite supports.
to run "move and copy" unconditionally in all cases, including on databases
other than SQLite; more on this is below.
+.. _batch_controlling_table_reflection:
+
+Controlling Table Reflection
+----------------------------
+
+The :class:`~sqlalchemy.schema.Table` object that is reflected when
+"move and copy" proceeds is performed using the standard ``autoload=True``
+approach. This call can be affected using the
+:paramref:`~.Operations.batch_alter_table.reflect_args` and
+:paramref:`~.Operations.batch_alter_table.reflect_kwargs` arguments.
+For example, to override a :class:`~sqlalchemy.schema.Column` within
+the reflection process such that a :class:`~sqlalchemy.types.Boolean`
+object is reflected with the ``create_constraint`` flag set to ``False``::
+
+ with self.op.batch_alter_table(
+ "bar",
+ reflect_args=[Column('flag', Boolean(create_constraint=False))]
+ ) as batch_op:
+ batch_op.alter_column(
+ 'flag', new_column_name='bflag', existing_type=Boolean)
+
+Another use case, add a listener to the :class:`~sqlalchemy.schema.Table`
+as it is reflected so that special logic can be applied to columns or
+types, using the :meth:`~sqlalchemy.events.DDLEvents.column_reflect` event::
+
+ def listen_for_reflect(inspector, table, column_info):
+ "correct an ENUM type"
+ if column_info['name'] == 'my_enum':
+ column_info['type'] = Enum('a', 'b', 'c')
+
+ with self.op.batch_alter_table(
+ "bar",
+ reflect_kwargs=dict(
+ listeners=[
+ ('column_reflect', listen_for_reflect)
+ ]
+ )
+ ) as batch_op:
+ batch_op.alter_column(
+ 'flag', new_column_name='bflag', existing_type=Boolean)
+
+The reflection process may also be bypassed entirely by sending a
+pre-fabricated :class:`~sqlalchemy.schema.Table` object; see
+:ref:`batch_offline_mode` for an example.
+
+.. versionadded:: 0.7.1
+ added :paramref:`.Operations.batch_alter_table.reflect_args`
+ and :paramref:`.Operations.batch_alter_table.reflect_kwargs` options.
+
Dealing with Constraints
------------------------
@@ -101,6 +150,8 @@ the recreation of unnamed UNIQUE constraints, either they should be named
in the first place, or again specified within
:paramref:`.Operations.batch_alter_table.table_args`.
+.. _batch_offline_mode:
+
Working in Offline Mode
-----------------------
@@ -113,7 +164,7 @@ get this information, which means that "online" mode is required; the
To support offline mode, the system must work without table reflection
present, which means the full table as it intends to be created must be
passed to :meth:`.Operations.batch_alter_table` using
-:paramref:`.Operations.batch_alter_table.copy_from`::
+:paramref:`~.Operations.batch_alter_table.copy_from`::
meta = MetaData()
some_table = Table(
diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index 2287b9b..3c9927c 100644
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -4,6 +4,33 @@
Changelog
==========
.. changelog::
+ :version: 0.7.1
+
+ .. change::
+ :tags: bug, batch
+ :tickets: 249
+
+ Repaired the inspection, copying and rendering of CHECK constraints
+ and so-called "schema" types such as Boolean, Enum within the batch
+ copy system; the CHECK constraint will not be "doubled" when the table is
+ copied, and additionally the inspection of the CHECK constraint for
+ its member columns will no longer fail with an attribute error.
+
+ .. change::
+ :tags: feature, batch
+
+ Added two new arguments
+ :paramref:`.Operations.batch_alter_table.reflect_args`
+ and :paramref:`.Operations.batch_alter_table.reflect_kwargs`, so that
+ arguments may be passed directly to suit the
+ :class:`~.sqlalchemy.schema.Table`
+ object that will be reflected.
+
+ .. seealso::
+
+ :ref:`batch_controlling_table_reflection`
+
+.. changelog::
:version: 0.7.0
:released: November 24, 2014
diff --git a/tests/requirements.py b/tests/requirements.py
index 65c59d0..7512adf 100644
--- a/tests/requirements.py
+++ b/tests/requirements.py
@@ -19,3 +19,13 @@ class DefaultRequirements(SuiteRequirements):
"""test will fail if referential integrity is enforced"""
return exclusions.fails_on_everything_except("sqlite")
+
+ @property
+ def non_native_boolean(self):
+ """test will fail if native boolean is provided"""
+
+ return exclusions.fails_if(
+ exclusions.LambdaPredicate(
+ lambda config: config.db.dialect.supports_native_boolean
+ )
+ )
diff --git a/tests/test_batch.py b/tests/test_batch.py
index 3892951..662714e 100644
--- a/tests/test_batch.py
+++ b/tests/test_batch.py
@@ -10,7 +10,8 @@ from alembic.batch import ApplyBatchImpl
from alembic.migration import MigrationContext
from sqlalchemy import Integer, Table, Column, String, MetaData, ForeignKey, \
- UniqueConstraint, ForeignKeyConstraint, Index
+ UniqueConstraint, ForeignKeyConstraint, Index, Boolean, CheckConstraint, \
+ Enum
from sqlalchemy.sql import column
from sqlalchemy.schema import CreateTable, CreateIndex
@@ -53,6 +54,30 @@ class BatchApplyTest(TestBase):
)
return ApplyBatchImpl(t, table_args, table_kwargs)
+ def _literal_ck_fixture(
+ self, copy_from=None, table_args=(), table_kwargs={}):
+ m = MetaData()
+ if copy_from is not None:
+ t = copy_from
+ else:
+ t = Table(
+ 'tname', m,
+ Column('id', Integer, primary_key=True),
+ Column('email', String()),
+ CheckConstraint("email LIKE '%@%'")
+ )
+ return ApplyBatchImpl(t, table_args, table_kwargs)
+
+ def _sql_ck_fixture(self, table_args=(), table_kwargs={}):
+ m = MetaData()
+ t = Table(
+ 'tname', m,
+ Column('id', Integer, primary_key=True),
+ Column('email', String())
+ )
+ t.append_constraint(CheckConstraint(t.c.email.like('%@%')))
+ return ApplyBatchImpl(t, table_args, table_kwargs)
+
def _fk_fixture(self, table_args=(), table_kwargs={}):
m = MetaData()
t = Table(
@@ -83,6 +108,33 @@ class BatchApplyTest(TestBase):
)
return ApplyBatchImpl(t, table_args, table_kwargs)
+ def _boolean_fixture(self, table_args=(), table_kwargs={}):
+ m = MetaData()
+ t = Table(
+ 'tname', m,
+ Column('id', Integer, primary_key=True),
+ Column('flag', Boolean)
+ )
+ return ApplyBatchImpl(t, table_args, table_kwargs)
+
+ def _boolean_no_ck_fixture(self, table_args=(), table_kwargs={}):
+ m = MetaData()
+ t = Table(
+ 'tname', m,
+ Column('id', Integer, primary_key=True),
+ Column('flag', Boolean(create_constraint=False))
+ )
+ return ApplyBatchImpl(t, table_args, table_kwargs)
+
+ def _enum_fixture(self, table_args=(), table_kwargs={}):
+ m = MetaData()
+ t = Table(
+ 'tname', m,
+ Column('id', Integer, primary_key=True),
+ Column('thing', Enum('a', 'b', 'c'))
+ )
+ return ApplyBatchImpl(t, table_args, table_kwargs)
+
def _assert_impl(self, impl, colnames=None,
ddl_contains=None, ddl_not_contains=None,
dialect='default'):
@@ -129,8 +181,8 @@ class BatchApplyTest(TestBase):
"CAST(tname.%s AS %s) AS anon_1" % (
name, impl.new_table.c[name].type)
if (
- impl.new_table.c[name].type
- is not impl.table.c[name].type)
+ impl.new_table.c[name].type._type_affinity
+ is not impl.table.c[name].type._type_affinity)
else "tname.%s" % name
for name in colnames if name in impl.table.c
)
@@ -153,6 +205,92 @@ class BatchApplyTest(TestBase):
new_table = self._assert_impl(impl)
eq_(new_table.c.x.name, 'q')
+ def test_rename_col_boolean(self):
+ impl = self._boolean_fixture()
+ impl.alter_column('tname', 'flag', name='bflag')
+ new_table = self._assert_impl(
+ impl, ddl_contains="CHECK (bflag IN (0, 1)",
+ colnames=["id", "flag"])
+ eq_(new_table.c.flag.name, 'bflag')
+ eq_(
+ len([
+ const for const
+ in new_table.constraints
+ if isinstance(const, CheckConstraint)]),
+ 1)
+
+ def test_rename_col_boolean_no_ck(self):
+ impl = self._boolean_no_ck_fixture()
+ impl.alter_column('tname', 'flag', name='bflag')
+ new_table = self._assert_impl(
+ impl, ddl_not_contains="CHECK",
+ colnames=["id", "flag"])
+ eq_(new_table.c.flag.name, 'bflag')
+ eq_(
+ len([
+ const for const
+ in new_table.constraints
+ if isinstance(const, CheckConstraint)]),
+ 0)
+
+ def test_rename_col_enum(self):
+ impl = self._enum_fixture()
+ impl.alter_column('tname', 'thing', name='thang')
+ new_table = self._assert_impl(
+ impl, ddl_contains="CHECK (thang IN ('a', 'b', 'c')",
+ colnames=["id", "thing"])
+ eq_(new_table.c.thing.name, 'thang')
+ eq_(
+ len([
+ const for const
+ in new_table.constraints
+ if isinstance(const, CheckConstraint)]),
+ 1)
+
+ def test_rename_col_literal_ck(self):
+ impl = self._literal_ck_fixture()
+ impl.alter_column('tname', 'email', name='emol')
+ new_table = self._assert_impl(
+ # note this is wrong, we don't dig into the SQL
+ impl, ddl_contains="CHECK (email LIKE '%@%')",
+ colnames=["id", "email"])
+ eq_(
+ len([c for c in new_table.constraints
+ if isinstance(c, CheckConstraint)]), 1)
+
+ eq_(new_table.c.email.name, 'emol')
+
+ def test_rename_col_literal_ck_workaround(self):
+ impl = self._literal_ck_fixture(
+ copy_from=Table(
+ 'tname', MetaData(),
+ Column('id', Integer, primary_key=True),
+ Column('email', String),
+ ),
+ table_args=[CheckConstraint("emol LIKE '%@%'")])
+
+ impl.alter_column('tname', 'email', name='emol')
+ new_table = self._assert_impl(
+ impl, ddl_contains="CHECK (emol LIKE '%@%')",
+ colnames=["id", "email"])
+ eq_(
+ len([c for c in new_table.constraints
+ if isinstance(c, CheckConstraint)]), 1)
+ eq_(new_table.c.email.name, 'emol')
+
+ def test_rename_col_sql_ck(self):
+ impl = self._sql_ck_fixture()
+
+ impl.alter_column('tname', 'email', name='emol')
+ new_table = self._assert_impl(
+ impl, ddl_contains="CHECK (emol LIKE '%@%')",
+ colnames=["id", "email"])
+ eq_(
+ len([c for c in new_table.constraints
+ if isinstance(c, CheckConstraint)]), 1)
+
+ eq_(new_table.c.email.name, 'emol')
+
def test_add_col(self):
impl = self._simple_fixture()
col = Column('g', Integer)
@@ -428,9 +566,10 @@ class BatchRoundTripTest(TestBase):
self.metadata.drop_all(self.conn)
self.conn.close()
- def _assert_data(self, data):
+ def _assert_data(self, data, tablename='foo'):
eq_(
- [dict(row) for row in self.conn.execute("select * from foo")],
+ [dict(row) for row
+ in self.conn.execute("select * from %s" % tablename)],
data
)
@@ -506,6 +645,54 @@ class BatchRoundTripTest(TestBase):
{"id": 5, "data": "d5", "y": 9}
])
+ def test_rename_column_boolean(self):
+ bar = Table(
+ 'bar', self.metadata,
+ Column('id', Integer, primary_key=True),
+ Column('flag', Boolean()),
+ mysql_engine='InnoDB'
+ )
+ bar.create(self.conn)
+ self.conn.execute(bar.insert(), {'id': 1, 'flag': True})
+ self.conn.execute(bar.insert(), {'id': 2, 'flag': False})
+
+ with self.op.batch_alter_table(
+ "bar"
+ ) as batch_op:
+ batch_op.alter_column(
+ 'flag', new_column_name='bflag', existing_type=Boolean)
+
+ self._assert_data([
+ {"id": 1, 'bflag': True},
+ {"id": 2, 'bflag': False},
+ ], 'bar')
+
+ @config.requirements.non_native_boolean
+ def test_rename_column_non_native_boolean_no_ck(self):
+ bar = Table(
+ 'bar', self.metadata,
+ Column('id', Integer, primary_key=True),
+ Column('flag', Boolean(create_constraint=False)),
+ mysql_engine='InnoDB'
+ )
+ bar.create(self.conn)
+ self.conn.execute(bar.insert(), {'id': 1, 'flag': True})
+ self.conn.execute(bar.insert(), {'id': 2, 'flag': False})
+ self.conn.execute(bar.insert(), {'id': 3, 'flag': 5})
+
+ with self.op.batch_alter_table(
+ "bar",
+ reflect_args=[Column('flag', Boolean(create_constraint=False))]
+ ) as batch_op:
+ batch_op.alter_column(
+ 'flag', new_column_name='bflag', existing_type=Boolean)
+
+ self._assert_data([
+ {"id": 1, 'bflag': True},
+ {"id": 2, 'bflag': False},
+ {'id': 3, 'bflag': 5}
+ ], 'bar')
+
def test_drop_column_pk(self):
with self.op.batch_alter_table("foo") as batch_op:
batch_op.drop_column('id')