]> git.ipfire.org Git - thirdparty/sqlalchemy/alembic.git/commitdiff
- Merge branch 'add_fk_check' of https://bitbucket.org/akamyshnikova/alembic into...
authorMike Bayer <mike_mp@zzzcomputing.com>
Sun, 30 Nov 2014 19:20:40 +0000 (14:20 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 30 Nov 2014 19:20:40 +0000 (14:20 -0500)
- complete merge, get all tests passing
- use 'foreignkey' literal

Conflicts:
alembic/autogenerate/compare.py
tests/test_autogenerate.py

1  2 
alembic/autogenerate/api.py
alembic/autogenerate/compare.py
alembic/autogenerate/render.py
tests/test_autogenerate.py

index 4539a9c77286af065f6f38ccb4983e49f999084b,5a5e75aadf59a7d987dbbdd5e73c2c2d66f30db1..60e2100c4c5cc12f744187e2aed966a3f7e417a9
@@@ -336,23 -305,3 +338,25 @@@ def _invoke_modify_command(updown, args
      if "server_default" in kw:
          kw.pop("existing_server_default", None)
      return _modify_col(tname, cname, autogen_context, schema=sname, **kw)
-         "constraint": lambda diff: (diff[0].table.schema, diff[0].table.name)
 +
 +
 +def _group_diffs_by_table(diffs):
 +    _adddrop = {
 +        "table": lambda diff: (None, None),
 +        "column": lambda diff: (diff[0], diff[1]),
 +        "index": lambda diff: (diff[0].table.schema, diff[0].table.name),
++        "constraint": lambda diff: (diff[0].table.schema, diff[0].table.name),
++        "fk": lambda diff:
++        (diff[0].parent.table.schema, diff[0].parent.table.name)
 +    }
 +
 +    def _derive_table(diff):
 +        if isinstance(diff, tuple):
 +            cmd_type = diff[0]
 +            adddrop, cmd_type = cmd_type.split("_")
 +            return _adddrop[cmd_type](diff[1:])
 +        else:
 +            sname, tname = diff[0][1:3]
 +            return sname, tname
 +
 +    return itertools.groupby(diffs, _derive_table)
index 3a352082db494111eb908134ff913ab7a919266c,db2e9c5578b69956efc038d871abd96b84f71d8a..eb649ac8d7be7918af07ca32ab265b94acab3e1e
@@@ -103,15 -103,17 +104,18 @@@ def _compare_tables(conn_table_names, m
          if _run_filters(
                  metadata_table, tname, "table", False,
                  conn_table, object_filters):
 -            _compare_columns(s, tname, object_filters,
 -                             conn_table,
 -                             metadata_table,
 -                             diffs, autogen_context, inspector)
 -            _compare_indexes_and_uniques(s, tname, object_filters,
 -                                         conn_table,
 -                                         metadata_table,
 -                                         diffs, autogen_context, inspector)
 -            _compare_foreign_keys(s, tname, object_filters, conn_table,
 -                                  metadata_table, diffs, autogen_context,
 -                                  inspector)
 +            with _compare_columns(
 +                s, tname, object_filters,
 +                conn_table,
 +                metadata_table,
 +                    diffs, autogen_context, inspector):
 +                _compare_indexes_and_uniques(s, tname, object_filters,
 +                                             conn_table,
 +                                             metadata_table,
 +                                             diffs, autogen_context, inspector)
++                _compare_foreign_keys(s, tname, object_filters, conn_table,
++                                      metadata_table, diffs, autogen_context,
++                                      inspector)
  
      # TODO:
      # table constraints
@@@ -566,3 -562,64 +570,64 @@@ def _compare_server_default(schema, tna
                   tname,
                   cname
                   )
 -        tuple(k.column.name for k in fk.constraint._elements.values()))
+ FKInfo = collections.namedtuple('fk_info', ['constrained_columns',
+                                             'referred_table',
+                                             'referred_columns'])
+ def _compare_foreign_keys(schema, tname, object_filters, conn_table,
+                           metadata_table, diffs, autogen_context, inspector):
+     # This methods checks foreign keys that tables contain in models with
+     # foreign keys that are in db.
+     # Get all necessary information about key of current table from db
+     if conn_table is None:
+         return
+     fk_db = {}
+     if hasattr(inspector, "get_foreign_keys"):
+         try:
+             fk_db = dict((_get_fk_info_from_db(i), i['name']) for i in
+                          inspector.get_foreign_keys(tname, schema=schema))
+         except NotImplementedError:
+             pass
+     # Get all necessary information about key of current table from
+     # models
+     fk_models = dict((_get_fk_info_from_model(fk), fk) for fk in
+                      metadata_table.foreign_keys)
+     fk_models_set = set(fk_models.keys())
+     fk_db_set = set(fk_db.keys())
+     for key in (fk_db_set - fk_models_set):
+             diffs.append(('drop_fk', fk_db[key], conn_table, key))
+             log.info(("Detected removed foreign key %(fk)r on "
+                       "table %(table)r"), {'fk': fk_db[key],
+                                            'table': conn_table})
+     for key in (fk_models_set - fk_db_set):
+             diffs.append(('add_fk', fk_models[key], key))
+             log.info((
+                 "Detected added foreign key for column %(fk)r on table "
+                 "%(table)r"), {'fk': fk_models[key].column.name,
+                                'table': conn_table})
+     return diffs
+ def _get_fk_info_from_db(fk):
+     return FKInfo(tuple(fk['constrained_columns']),
+                   fk['referred_table'],
+                   tuple(fk['referred_columns']))
+ def _get_fk_info_from_model(fk):
+     constrained_columns = []
+     for column in fk.constraint.columns:
+         if not isinstance(column, basestring):
+             constrained_columns.append(column.name)
+         else:
+             constrained_columns.append(column)
+     return FKInfo(
+         tuple(constrained_columns),
+         fk.column.table.name,
++        tuple(k.column.name for k in fk.constraint._elements.values()))
index 3bfa9c02b56538b5126d864ad6e701514fc29e89,aa576778a9db51ff8146228cbded88ce788ff722..f6f7d9d39d23856d28d02e72648e42140c9fe79b
@@@ -241,8 -193,32 +241,32 @@@ def _uq_constraint(constraint, autogen_
          }
  
  
- def _add_fk_constraint(constraint, autogen_context):
-     raise NotImplementedError()
+ def _add_fk_constraint(constraint, fk_info, autogen_context):
+     args = [
+         repr(_render_gen_name(autogen_context, constraint.name)),
+         constraint.parent.table.name,
+         fk_info.referred_table,
+         str(list(fk_info.constrained_columns)),
+         str(list(fk_info.referred_columns)),
+         "%s=%r" % ('schema', constraint.parent.table.schema),
+     ]
+     if constraint.deferrable:
+         args.append("%s=%r" % ("deferrable", str(constraint.deferrable)))
+     if constraint.initially:
+         args.append("%s=%r" % ("initially", str(constraint.initially)))
+     return "%(prefix)screate_foreign_key(%(args)s)" % {
+         'prefix': _alembic_autogenerate_prefix(autogen_context),
+         'args': ", ".join(args)
+     }
+ def _drop_fk_constraint(constraint, fk_info, autogen_context):
+     args = [repr(_render_gen_name(autogen_context, constraint.name)),
 -            constraint.parent.table.name, 'type_=foreignkey']
++            constraint.parent.table.name, "type_='foreignkey'"]
+     return "%(prefix)sdrop_constraint(%(args)s)" % {
+         'prefix': _alembic_autogenerate_prefix(autogen_context),
+         'args': ", ".join(args)
+     }
  
  
  def _add_pk_constraint(constraint, autogen_context):
index eebaa12b75c4108f3221f7b999cb4a940eb37c36,19a0d1dc1d834db051b2e6a696629e9f77ca903f..7351f7907f93ff0ba6834ec3f0f9ba0f287e4576
@@@ -410,37 -402,37 +410,43 @@@ class AutogenerateDiffTest(ModelOne, Au
          eq_(diffs[2][2], "address")
          eq_(diffs[2][3], metadata.tables['address'].c.street)
  
 -        eq_(diffs[3][0], "add_column")
 -        eq_(diffs[3][1], None)
 -        eq_(diffs[3][2], "order")
 -        eq_(diffs[3][3], metadata.tables['order'].c.user_id)
 +        eq_(diffs[3][0], "add_constraint")
 +        eq_(diffs[3][1].name, "uq_email")
  
 -        eq_(diffs[4][0][0], "modify_type")
 -        eq_(diffs[4][0][1], None)
 -        eq_(diffs[4][0][2], "order")
 -        eq_(diffs[4][0][3], "amount")
 -        eq_(repr(diffs[4][0][5]), "NUMERIC(precision=8, scale=2)")
 -        eq_(repr(diffs[4][0][6]), "Numeric(precision=10, scale=2)")
 +        eq_(diffs[4][0], "add_column")
 +        eq_(diffs[4][1], None)
 +        eq_(diffs[4][2], "order")
 +        eq_(diffs[4][3], metadata.tables['order'].c.user_id)
  
 -        eq_(diffs[5][0], 'add_fk')
 -        eq_(diffs[5][1].column.name, 'id')
 -        eq_(diffs[5][1].parent.table.name, 'order')
 -        eq_(diffs[5][2].referred_table, 'user')
 -        eq_(diffs[5][2].constrained_columns, ('user_id',))
 +        eq_(diffs[5][0][0], "modify_type")
 +        eq_(diffs[5][0][1], None)
 +        eq_(diffs[5][0][2], "order")
 +        eq_(diffs[5][0][3], "amount")
 +        eq_(repr(diffs[5][0][5]), "NUMERIC(precision=8, scale=2)")
 +        eq_(repr(diffs[5][0][6]), "Numeric(precision=10, scale=2)")
  
-         eq_(diffs[6][0][0], "modify_default")
-         eq_(diffs[6][0][1], None)
-         eq_(diffs[6][0][2], "user")
-         eq_(diffs[6][0][3], "a1")
-         eq_(diffs[6][0][6].arg, "x")
 -        eq_(diffs[6][0], 'remove_column')
 -        eq_(diffs[6][3].name, 'pw')
++        eq_(diffs[6][0], 'add_fk')
++        eq_(diffs[6][1].column.name, 'id')
++        eq_(diffs[6][1].parent.table.name, 'order')
++        eq_(diffs[6][2].referred_table, 'user')
++        eq_(diffs[6][2].constrained_columns, ('user_id',))
  
-         eq_(diffs[7][0][0], 'modify_nullable')
-         eq_(diffs[7][0][5], True)
-         eq_(diffs[7][0][6], False)
+         eq_(diffs[7][0][0], "modify_default")
+         eq_(diffs[7][0][1], None)
+         eq_(diffs[7][0][2], "user")
+         eq_(diffs[7][0][3], "a1")
+         eq_(diffs[7][0][6].arg, "x")
  
-         eq_(diffs[8][0], 'remove_index')
-         eq_(diffs[8][1].name, 'pw_idx')
+         eq_(diffs[8][0][0], 'modify_nullable')
+         eq_(diffs[8][0][5], True)
+         eq_(diffs[8][0][6], False)
  
-         eq_(diffs[9][0], 'remove_column')
-         eq_(diffs[9][3].name, 'pw')
++        eq_(diffs[9][0], 'remove_index')
++        eq_(diffs[9][1].name, 'pw_idx')
++
++        eq_(diffs[10][0], 'remove_column')
++        eq_(diffs[10][3].name, 'pw')
 +
      def test_render_nothing(self):
          context = MigrationContext.configure(
              connection=self.bind.connect(),
@@@ -519,6 -481,8 +524,7 @@@ nullable=True)
                 type_=sa.Numeric(precision=10, scale=2),
                 nullable=True,
                 existing_server_default=sa.text('0'))
 -    op.drop_column('user', 'pw')
+     op.create_foreign_key(None, order, user, ['user_id'], ['id'], schema=None)
      op.alter_column('user', 'a1',
                 existing_type=sa.TEXT(),
                 server_default='x',
@@@ -542,6 -501,9 +548,7 @@@ nullable=True)
                 existing_type=sa.TEXT(),
                 server_default=None,
                 existing_nullable=True)
 -    op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), \
 -nullable=True))
 -    op.drop_constraint(None, order, type_=foreignkey)
++    op.drop_constraint(None, order, type_='foreignkey')
      op.alter_column('order', 'amount',
                 existing_type=sa.Numeric(precision=10, scale=2),
                 type_=sa.NUMERIC(precision=8, scale=2),
      op.drop_table('item')
      ### end Alembic commands ###""")
  
 +    def test_render_diffs_batch(self):
 +        """test a full render in batch mode including indentation"""
 +
 +        template_args = {}
 +        self.context.opts['render_as_batch'] = True
 +        autogenerate._produce_migration_diffs(
 +            self.context, template_args, set())
 +
 +        eq_(re.sub(r"u'", "'", template_args['upgrades']),
 +            """### commands auto generated by Alembic - please adjust! ###
 +    op.create_table('item',
 +    sa.Column('id', sa.Integer(), nullable=False),
 +    sa.Column('description', sa.String(length=100), nullable=True),
 +    sa.Column('order_id', sa.Integer(), nullable=True),
 +    sa.CheckConstraint('len(description) > 5'),
 +    sa.ForeignKeyConstraint(['order_id'], ['order.order_id'], ),
 +    sa.PrimaryKeyConstraint('id')
 +    )
 +    op.drop_table('extra')
 +    with op.batch_alter_table('address', schema=None) as batch_op:
 +        batch_op.add_column(sa.Column('street', sa.String(length=50), nullable=True))
 +        batch_op.create_unique_constraint('uq_email', ['email_address'])
 +
 +    with op.batch_alter_table('order', schema=None) as batch_op:
 +        batch_op.add_column(sa.Column('user_id', sa.Integer(), nullable=True))
 +        batch_op.alter_column('amount',
 +               existing_type=sa.NUMERIC(precision=8, scale=2),
 +               type_=sa.Numeric(precision=10, scale=2),
 +               nullable=True,
 +               existing_server_default=sa.text('0'))
++        batch_op.create_foreign_key(None, order, user, ['user_id'], ['id'], schema=None)
 +
 +    with op.batch_alter_table('user', schema=None) as batch_op:
 +        batch_op.alter_column('a1',
 +               existing_type=sa.TEXT(),
 +               server_default='x',
 +               existing_nullable=True)
 +        batch_op.alter_column('name',
 +               existing_type=sa.VARCHAR(length=50),
 +               nullable=False)
 +        batch_op.drop_index('pw_idx')
 +        batch_op.drop_column('pw')
 +
 +    ### end Alembic commands ###""")
 +
 +        eq_(re.sub(r"u'", "'", template_args['downgrades']),
 +            """### commands auto generated by Alembic - please adjust! ###
 +    with op.batch_alter_table('user', schema=None) as batch_op:
 +        batch_op.add_column(sa.Column('pw', sa.VARCHAR(length=50), nullable=True))
 +        batch_op.create_index('pw_idx', ['pw'], unique=False)
 +        batch_op.alter_column('name',
 +               existing_type=sa.VARCHAR(length=50),
 +               nullable=True)
 +        batch_op.alter_column('a1',
 +               existing_type=sa.TEXT(),
 +               server_default=None,
 +               existing_nullable=True)
 +
 +    with op.batch_alter_table('order', schema=None) as batch_op:
++        batch_op.drop_constraint(None, order, type_='foreignkey')
 +        batch_op.alter_column('amount',
 +               existing_type=sa.Numeric(precision=10, scale=2),
 +               type_=sa.NUMERIC(precision=8, scale=2),
 +               nullable=False,
 +               existing_server_default=sa.text('0'))
 +        batch_op.drop_column('user_id')
 +
 +    with op.batch_alter_table('address', schema=None) as batch_op:
 +        batch_op.drop_constraint('uq_email')
 +        batch_op.drop_column('street')
 +
 +    op.create_table('extra',
 +    sa.Column('x', sa.CHAR(), nullable=True),
 +    sa.Column('uid', sa.INTEGER(), nullable=True),
 +    sa.ForeignKeyConstraint(['uid'], ['user.id'], )
 +    )
 +    op.drop_table('item')
 +    ### end Alembic commands ###""")
 +
      def test_include_symbol(self):
          context = MigrationContext.configure(
              connection=self.bind.connect(),
@@@ -791,36 -675,30 +800,43 @@@ class AutogenerateDiffTestWSchema(Model
          eq_(diffs[2][2], "address")
          eq_(diffs[2][3], metadata.tables['%s.address' % self.schema].c.street)
  
 -        eq_(diffs[3][0], "add_column")
 -        eq_(diffs[3][1], self.schema)
 -        eq_(diffs[3][2], "order")
 -        eq_(diffs[3][3], metadata.tables['%s.order' % self.schema].c.user_id)
 +        eq_(diffs[3][0], "add_constraint")
 +        eq_(diffs[3][1].name, "uq_email")
  
 -        eq_(diffs[4][0][0], "modify_type")
 -        eq_(diffs[4][0][1], self.schema)
 -        eq_(diffs[4][0][2], "order")
 -        eq_(diffs[4][0][3], "amount")
 -        eq_(repr(diffs[4][0][5]), "NUMERIC(precision=8, scale=2)")
 -        eq_(repr(diffs[4][0][6]), "Numeric(precision=10, scale=2)")
 +        eq_(diffs[4][0], "add_column")
 +        eq_(diffs[4][1], self.schema)
 +        eq_(diffs[4][2], "order")
 +        eq_(diffs[4][3], metadata.tables['%s.order' % self.schema].c.user_id)
  
 -        eq_(diffs[5][0], 'remove_column')
 -        eq_(diffs[5][3].name, 'pw')
 +        eq_(diffs[5][0][0], "modify_type")
 +        eq_(diffs[5][0][1], self.schema)
 +        eq_(diffs[5][0][2], "order")
 +        eq_(diffs[5][0][3], "amount")
 +        eq_(repr(diffs[5][0][5]), "NUMERIC(precision=8, scale=2)")
 +        eq_(repr(diffs[5][0][6]), "Numeric(precision=10, scale=2)")
  
--        eq_(diffs[6][0][0], "modify_default")
--        eq_(diffs[6][0][1], self.schema)
--        eq_(diffs[6][0][2], "user")
--        eq_(diffs[6][0][3], "a1")
--        eq_(diffs[6][0][6].arg, "x")
++        eq_(diffs[6][0], 'add_fk')
++        eq_(diffs[6][1].column.name, 'id')
++        eq_(diffs[6][1].parent.table.name, 'order')
++        eq_(diffs[6][1].parent.table.schema, config.test_schema)
++        eq_(diffs[6][2].referred_table, 'user')
++        eq_(diffs[6][2].constrained_columns, ('user_id',))
 -        eq_(diffs[7][0][0], 'modify_nullable')
 -        eq_(diffs[7][0][5], True)
 -        eq_(diffs[7][0][6], False)
++        eq_(diffs[7][0][0], "modify_default")
++        eq_(diffs[7][0][1], self.schema)
++        eq_(diffs[7][0][2], "user")
++        eq_(diffs[7][0][3], "a1")
++        eq_(diffs[7][0][6].arg, "x")
 +
-         eq_(diffs[7][0][0], 'modify_nullable')
-         eq_(diffs[7][0][5], True)
-         eq_(diffs[7][0][6], False)
++        eq_(diffs[8][0][0], 'modify_nullable')
++        eq_(diffs[8][0][5], True)
++        eq_(diffs[8][0][6], False)
 +
-         eq_(diffs[8][0], 'remove_index')
-         eq_(diffs[8][1].name, 'pw_idx')
++        eq_(diffs[9][0], 'remove_index')
++        eq_(diffs[9][1].name, 'pw_idx')
 +
-         eq_(diffs[9][0], 'remove_column')
-         eq_(diffs[9][3].name, 'pw')
++        eq_(diffs[10][0], 'remove_column')
++        eq_(diffs[10][3].name, 'pw')
  
      def test_render_nothing(self):
          context = MigrationContext.configure(
@@@ -883,6 -759,7 +899,8 @@@ schema='%(schema)s'
                 nullable=True,
                 existing_server_default=sa.text('0'),
                 schema='%(schema)s')
 -    op.drop_column('user', 'pw', schema='%(schema)s')
++    op.create_foreign_key(None, order, user, ['user_id'], ['id'], \
++schema='%(schema)s')
      op.alter_column('user', 'a1',
                 existing_type=sa.TEXT(),
                 server_default='x',
@@@ -910,6 -782,8 +928,7 @@@ autoincrement=False, nullable=True), sc
                 server_default=None,
                 existing_nullable=True,
                 schema='%(schema)s')
 -    op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), \
 -autoincrement=False, nullable=True), schema='%(schema)s')
++    op.drop_constraint(None, order, type_='foreignkey')
      op.alter_column('order', 'amount',
                 existing_type=sa.Numeric(precision=10, scale=2),
                 type_=sa.NUMERIC(precision=8, scale=2),
@@@ -1192,37 -1065,37 +1211,43 @@@ class CompareMetadataTest(ModelOne, Aut
          eq_(diffs[2][2], "address")
          eq_(diffs[2][3], metadata.tables['address'].c.street)
  
 -        eq_(diffs[3][0], "add_column")
 -        eq_(diffs[3][1], None)
 -        eq_(diffs[3][2], "order")
 -        eq_(diffs[3][3], metadata.tables['order'].c.user_id)
 +        eq_(diffs[3][0], "add_constraint")
 +        eq_(diffs[3][1].name, "uq_email")
  
 -        eq_(diffs[4][0][0], "modify_type")
 -        eq_(diffs[4][0][1], None)
 -        eq_(diffs[4][0][2], "order")
 -        eq_(diffs[4][0][3], "amount")
 -        eq_(repr(diffs[4][0][5]), "NUMERIC(precision=8, scale=2)")
 -        eq_(repr(diffs[4][0][6]), "Numeric(precision=10, scale=2)")
 +        eq_(diffs[4][0], "add_column")
 +        eq_(diffs[4][1], None)
 +        eq_(diffs[4][2], "order")
 +        eq_(diffs[4][3], metadata.tables['order'].c.user_id)
  
 -        eq_(diffs[5][0], 'add_fk')
 -        eq_(diffs[5][1].column.name, 'id')
 -        eq_(diffs[5][1].parent.table.name, 'order')
 -        eq_(diffs[5][2].referred_table, 'user')
 -        eq_(diffs[5][2].constrained_columns, ('user_id',))
 +        eq_(diffs[5][0][0], "modify_type")
 +        eq_(diffs[5][0][1], None)
 +        eq_(diffs[5][0][2], "order")
 +        eq_(diffs[5][0][3], "amount")
 +        eq_(repr(diffs[5][0][5]), "NUMERIC(precision=8, scale=2)")
 +        eq_(repr(diffs[5][0][6]), "Numeric(precision=10, scale=2)")
  
-         eq_(diffs[6][0][0], "modify_default")
-         eq_(diffs[6][0][1], None)
-         eq_(diffs[6][0][2], "user")
-         eq_(diffs[6][0][3], "a1")
-         eq_(diffs[6][0][6].arg, "x")
 -        eq_(diffs[6][0], 'remove_column')
 -        eq_(diffs[6][3].name, 'pw')
++        eq_(diffs[6][0], 'add_fk')
++        eq_(diffs[6][1].column.name, 'id')
++        eq_(diffs[6][1].parent.table.name, 'order')
++        eq_(diffs[6][2].referred_table, 'user')
++        eq_(diffs[6][2].constrained_columns, ('user_id',))
+         eq_(diffs[7][0][0], "modify_default")
+         eq_(diffs[7][0][1], None)
+         eq_(diffs[7][0][2], "user")
+         eq_(diffs[7][0][3], "a1")
+         eq_(diffs[7][0][6].arg, "x")
  
-         eq_(diffs[7][0][0], 'modify_nullable')
-         eq_(diffs[7][0][5], True)
-         eq_(diffs[7][0][6], False)
+         eq_(diffs[8][0][0], 'modify_nullable')
+         eq_(diffs[8][0][5], True)
+         eq_(diffs[8][0][6], False)
  
-         eq_(diffs[8][0], 'remove_index')
-         eq_(diffs[8][1].name, 'pw_idx')
++        eq_(diffs[9][0], 'remove_index')
++        eq_(diffs[9][1].name, 'pw_idx')
 +
-         eq_(diffs[9][0], 'remove_column')
-         eq_(diffs[9][3].name, 'pw')
++        eq_(diffs[10][0], 'remove_column')
++        eq_(diffs[10][3].name, 'pw')
 +
      def test_compare_metadata_include_object(self):
          metadata = self.m2