Skip to content

Commit

Permalink
differentiate CreateTableOp from model vs. op.create_table()
Browse files Browse the repository at this point in the history
Fixed regression caused by just fixed 🎫`844` that scaled back the
filter for ``unique=True/index=True`` too far such that these directives no
longer worked for the ``op.create_table()`` op, this has been fixed.

Change-Id: Idbed889ef4fbb7b3a944d3f4631f1cae53030316
Fixes: #848
  • Loading branch information
zzzeek committed May 24, 2021
1 parent ffc6904 commit 19e1f6c
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 3 deletions.
21 changes: 18 additions & 3 deletions alembic/operations/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,13 @@ class CreateTableOp(MigrateOperation):
"""Represent a create table operation."""

def __init__(
self, table_name, columns, schema=None, _namespace_metadata=None, **kw
self,
table_name,
columns,
schema=None,
_namespace_metadata=None,
_constraints_included=False,
**kw
):
self.table_name = table_name
self.columns = columns
Expand All @@ -954,6 +960,7 @@ def __init__(
self.prefixes = kw.pop("prefixes", None)
self.kw = kw
self._namespace_metadata = _namespace_metadata
self._constraints_included = _constraints_included

def reverse(self):
return DropTableOp.from_table(
Expand All @@ -973,6 +980,13 @@ def from_table(cls, table, _namespace_metadata=None):
list(table.c) + list(table.constraints),
schema=table.schema,
_namespace_metadata=_namespace_metadata,
# given a Table() object, this Table will contain full Index()
# and UniqueConstraint objects already constructed in response to
# each unique=True / index=True flag on a Column. Carry this
# state along so that when we re-convert back into a Table, we
# skip unique=True/index=True so that these constraints are
# not doubled up. see #844 #848
_constraints_included=True,
comment=table.comment,
prefixes=table._prefixes,
**table.kwargs
Expand All @@ -987,7 +1001,7 @@ def to_table(self, migration_context=None):
schema=self.schema,
prefixes=self.prefixes,
comment=self.comment,
_constraints_included=True,
_constraints_included=self._constraints_included,
**self.kw
)

Expand Down Expand Up @@ -1111,7 +1125,8 @@ def to_table(self, migration_context=None):
self.table_name,
*cols_and_constraints,
schema=self.schema,
_constraints_included=True,
_constraints_included=bool(self._reverse)
and self._reverse._constraints_included,
**self.table_kw
)
return t
Expand Down
7 changes: 7 additions & 0 deletions docs/build/unreleased/848.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.. change::
:tags: bug, regression, op directives
:tickets: 848

Fixed regression caused by just fixed :ticket:`844` that scaled back the
filter for ``unique=True/index=True`` too far such that these directives no
longer worked for the ``op.create_table()`` op, this has been fixed.
30 changes: 30 additions & 0 deletions tests/test_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,36 @@ def test_create_table_unique_constraint(self):
uq = [c for c in t1.constraints if isinstance(c, UniqueConstraint)]
eq_(uq[0].name, "uq_1")

def test_create_table_unique_flag(self):
context = op_fixture()
t1 = op.create_table(
"some_table",
Column("id", Integer, primary_key=True),
Column("foo_id", Integer, unique=True),
)
context.assert_(
"CREATE TABLE some_table (id INTEGER NOT NULL, foo_id INTEGER, "
"PRIMARY KEY (id), UNIQUE (foo_id))"
)

uq = [c for c in t1.constraints if isinstance(c, UniqueConstraint)]
assert uq

def test_create_table_index_flag(self):
context = op_fixture()
t1 = op.create_table(
"some_table",
Column("id", Integer, primary_key=True),
Column("foo_id", Integer, index=True),
)
context.assert_(
"CREATE TABLE some_table (id INTEGER NOT NULL, foo_id INTEGER, "
"PRIMARY KEY (id))",
"CREATE INDEX ix_some_table_foo_id ON some_table (foo_id)",
)

assert t1.indexes

def test_create_table_index(self):
context = op_fixture()
t1 = op.create_table(
Expand Down

0 comments on commit 19e1f6c

Please sign in to comment.