improvement: replace pairwise operation ordering with a dependency-graph topological sort

[jechol]

Motivation

Migration operations were ordered by a ~69-clause pairwise after?/2 comparator feeding an insertion sort. It's non-transitive and clause-order-dependent — two clause pairs turned out to be dead code, where a broader/earlier clause always shadowed a more specific one for the same struct pair.

It also had no way for an operation to declare "this depends on some fact being true yet." Global "always last" / "always first" tiers for things like AddCheckConstraint, AlterDeferrability, RemovePrimaryKeyDown, and AddPrimaryKey papered over what are really specific DDL ordering requirements with ad-hoc rules.

What changed

Each operation now declares facts it provides and facts it requires (operation_deps.ex), and toposort_operations/1 runs Kahn's algorithm over the resulting dependency graph.

A genuine cycle raises OperationCycleError instead of silently falling back to an arbitrary order — a later PR builds on this to let custom_statements opt into cross-table ordering, and a cycle there should fail loudly rather than produce a migration that runs in the wrong order.

group_into_phases/3 and clean_phases/1 are unchanged — they only consume the final order. streamline/2 is nearly unchanged: its custom-statement guard (from 12cf97d) is removed, since the structs it protected now carry :schema and the toposort can't place a same-table custom statement inside an add/alter window. RenameTable's struct now uses the same generic :table field every other operation struct uses (previously :old_table/:new_table), and RemovePrimaryKey now carries its :keys so its FK-ordering requirement can be column-scoped.

Bugs found during verification

Verified against a large real-world Ash application (~150 resources): a from-scratch schema regen applied to a real Postgres database, plus applying the same diff against a database restored from a production-sized backup, confirming the resulting schema matches in both cases. That surfaced two real bugs, both fixed here:

• A resource's own schema option defaults to nil, but attribute.references.schema is loaded as an explicit "public" string — building fact keys from these without normalizing silently dropped real cross-table FK ordering constraints.
• A unique index with a where/base filter can reference columns that aren't part of its keys (e.g. a soft-delete-scoped identity whose where checks archived_at, a column outside the index's own keys), with no structured way to know which ones without parsing raw SQL. AddUniqueIndex now conservatively requires every attribute added to the table first, but only when such a filter is actually present: applying that margin unconditionally would also force a self-referencing attribute (one whose own FK targets this same index, e.g. a self-referential belongs_to) to finish before the index it depends on — a genuine two-way cycle between that attribute and the index.

Tracing each fact's provider/consumer pairs also surfaced two pre-existing ordering bugs, fixed here with regression tests:

• Removing a check constraint and its column together could generate a down that recreates the constraint before re-adding the column it references. The dependency direction is now reversed: column removal/rename waits for the constraint's removal.
• Dropping a unique index still referenced by another table's foreign key was not ordered after the FK drop — Postgres blocks that drop for any unique constraint, not just the primary key.

Testing

test/migration_generator_test.exs: all passing (two tests fixed for pre-existing whitespace fragility the new ordering exposed), plus new integration tests pinning the generated up/down order for the bugs above. New test/migration_generator/operation_deps_test.exs unit-tests the provides/requires model directly. Full mix test: 858 passed.

Writing up what each fact means (in response to review) surfaced facts that were carried over from the old after?/2 rules but had no remaining consumer once traced through — each was removed individually and verified against the full suite. Every remaining fact has a documented provider/consumer pair and test coverage.

Contributor checklist

Leave anything that you believe does not apply unchecked.

• I accept the AI Policy, or AI was not used in the creation of this PR.
• Bug fixes include regression tests
• Chores
• Documentation changes
• Features include unit/acceptance tests
• Refactoring
• Update dependencies

[zachdaniel]

This is very cool and I was always aware that the pairwise "after" was a hack, glad that you've invested the energy to create a better way 🤩

Have some comments/questions and will need to do a deeper review soon of the specific fact emissions etc.

[jechol]

Addressed all review comments (replies inline). The branch has changed quite a bit since your review — easier to re-review fresh than against the old diff. Two pre-existing ordering bugs were also found and fixed along the way (details in code comments and tests).

One removal worth flagging: the custom-statement guard in streamline/2 (from 12cf97d) is gone — it guarded against op.schema raising on structs that had no :schema field, which they now have, and the toposort can't place a same-table custom statement inside an add/alter window anyway.

Ready for another look.

[jechol]

Ready for another pass — some context on what's changed since the last review, and why I think the dependency-graph model keeps paying off.

New ordering facts found by real-world migrations since the last review

Beyond translating the existing pairwise rules into the graph, running our production app's migrations against this branch kept surfacing orderings the pairwise model couldn't express. Each landed as a precise fact with a regression test:

1. A foreign key must run after the unique index covering its referenced columns (6431ffb). Found while converting hand-written composite-FK custom statements to declarative match_with references: the FK was emitted before the (org_id, id) unique custom index it targets, failing with "there is no unique constraint matching given keys". Since Postgres requires one unique index on exactly the referenced column set, this is modeled as a single column-set fact (unique_index_created) — built from the columns the rendered index actually covers (including the tenant attribute that attribute multitenancy prefixes at render time) and required for destination_attribute plus all match_with destinations. Two separate single-column unique indexes correctly do not satisfy a composite FK.
2. Custom indexes with plain field lists now wait only on their own columns (same commit). Previously a custom index waited for every column operation on its table. Combined with fact 1 that's a deadlock on the same table: the FK alter must wait for the unique index, but the unique index waited for the whole table to settle — which includes that very FK alter. Now an index whose fields are plain columns depends only on those columns; raw where/expression indexes keep the conservative whole-table wait.
3. Rewriting a reference index must drop the old index before creating the new one (1f4148c), since both derive the same auto-generated name — and a reference changing to index? false must not emit a fresh AddReferenceIndex (50acf60). Both found the same way.

Locally everything is green: the full suite plus the OperationDeps unit tests covering each fact above.

[zachdaniel]

🚀 Thank you for your contribution! 🚀