From: Mike Bayer Date: Sun, 30 Nov 2014 19:20:40 +0000 (-0500) Subject: - Merge branch 'add_fk_check' of https://bitbucket.org/akamyshnikova/alembic into... X-Git-Tag: rel_0_7_1~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c4409d67985fca04efdad02da8d3a7b8bc7cdc6f;p=thirdparty%2Fsqlalchemy%2Falembic.git - Merge branch 'add_fk_check' of https://bitbucket.org/akamyshnikova/alembic into pr32 - complete merge, get all tests passing - use 'foreignkey' literal Conflicts: alembic/autogenerate/compare.py tests/test_autogenerate.py --- c4409d67985fca04efdad02da8d3a7b8bc7cdc6f diff --cc alembic/autogenerate/api.py index 4539a9c7,5a5e75aa..60e2100c --- a/alembic/autogenerate/api.py +++ b/alembic/autogenerate/api.py @@@ -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) + + +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) ++ "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) diff --cc alembic/autogenerate/compare.py index 3a352082,db2e9c55..eb649ac8 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@@ -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 ) + + 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())) ++ tuple(k.column.name for k in fk.constraint._elements.values())) diff --cc alembic/autogenerate/render.py index 3bfa9c02,aa576778..f6f7d9d3 --- a/alembic/autogenerate/render.py +++ b/alembic/autogenerate/render.py @@@ -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): diff --cc tests/test_autogenerate.py index eebaa12b,19a0d1dc..7351f790 --- a/tests/test_autogenerate.py +++ b/tests/test_autogenerate.py @@@ -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.create_foreign_key(None, order, user, ['user_id'], ['id'], schema=None) - op.drop_column('user', 'pw') 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), @@@ -558,83 -519,6 +565,85 @@@ 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