sqlalchemy
take use_alter=True, other FK / foreign key / foreignkey constraint rules into account when autogenerating create_table / drop_table
Migrated issue, originally created by Adrian Vogelsgesang (@vogelsgesang)
If the schema contains circular dependencies, the order of the CREATE TABLE statements is incorrect.
This is because metadata.sorted_tables (used here) returns the tables ordered alphabetically if there are circular dependencies.
Michael Bayer (@zzzeek) wrote:
well there is no "correct" order if the tables have a cycle. The constraints have to be added with ALTER after the fact.
Michael Bayer (@zzzeek) wrote:
We should be able to leverage sort_tables_and_constraints in order to break out CreateTable ops vs. individual create constraint ops; however those constraints would need to be removed from the rendered version of CreateTable. This can be applied right now as an end-user autogen filter so we could even make a recipe to start with. It probably needs to be done in a post-filtering style on the inside as well and since it breaks up the create table into components likely needs to remain a pretty clear switchable/optional thing, though a clean autogen should have it on by default. But of course it'll be wrong on SQLite. might need to be a flag, perhaps we can warn if cycles are detected and the flag is off.
tomkcook (@tomkcook) wrote:
Moving discussion here from #387. I've had a go at implementing a simple version of this, by just ignoring foreign keys while comparing tables (nobbling _render_foreign_keys) and then comparing all the foreign keys on all the tables at the end - it works, but it feels like there should be pitfalls I haven't seen. I'll get it up on a bitbucket fork for review tomorrow.
tomkcook (@tomkcook) wrote:
See https://bitbucket.org/tomkcook/alembic. 16 tests fail at present; I'm not at all familiar with the tests, but as far as I can tell the failing cases all work as intended. I'm not familiar enough with the inner workings of alembic to fix the tests so they actually test creation of foreign keys; if you give me a pointer on how to do it, I'll go through and fix them.
tomkcook (@tomkcook) wrote:
I've got a vague feeling that this will do odd things if you drop a table that has foreign key constraints - it might well attempt to drop the table and then drop the constraint, which won't work. I guess the right way of doing this would be to replace _compare_tables from compare.py with something like _compare_tables_and_constraints and pass it the result of sqlalchemy.schema.sort_tables_and_constraints around compare.py:76.
Any comments?
Michael Bayer (@zzzeek) wrote:
the mutually-dependent constraints problem also has to be addressed on the drop side, yes.
per my comment at https://bitbucket.org/zzzeek/alembic/issues/326/take-use_alter-true-other-fk-foreign-key#comment-21818371, I had envisioned the problem here being approached using a post-filtering approach. A filter like this might go into compare.py -> _populate_migration_script. I would extract ForeignKeyConstraint objects that need to have use_alter going on, and then put them into their own create_foreign_key ops down below the createtable / addcolumn calls they are associated with.
this is not an easy change because it has to fit cleanly into the plugin-oriented autogenerate architecture, and I had planned to get into this architecture again at some point to figure out the best way sorting filters like these can be applied internally. I don't have a decision on the best place to do it and I'd have to spend a bunch of time getting into that.
If you want to just play around on the outside, work with the process_revision_directives hook: http://alembic.zzzcomputing.com/en/latest/api/autogenerate.html#customizing-revision-generation. A recipe here can even go into the "recipes" section in the interim and/or form the basis of the built-in behavior.
Changes by Michael Bayer (@zzzeek):
- changed title from "incorrect migration script generated if schema con" to "take use_alter=True other FK constraint rules into"
Changes by Michael Bayer (@zzzeek):
- changed title from "take use_alter=True other FK constraint rules into" to "take use_alter=True other FK / foreign key / forei"
the idea would be render it as:
op.create_table()
op.create_table()
op.add_fk(...)
...
yeah. we know how to determine that using what is now called sort_tables_and_constraints so this would need to be run and rearrange the tables somewhere between the compare and render phases. like perhaps sort and rearrange when we have the migration structure built.
Hello, this issue is relevant to our work organization. I will try to tackle this in an upcoming merge request using the sqlalchemy sort_tables_and_constraints
