feat: add after_tables to custom_statements so raw SQL can depend on another table#799
Open
jechol wants to merge 2 commits into
Open
feat: add after_tables to custom_statements so raw SQL can depend on another table#799jechol wants to merge 2 commits into
jechol wants to merge 2 commits into
Conversation
jechol
force-pushed
the
migration-ordering-and-after-tables
branch
from
July 18, 2026 16:54
bbd0d0a to
5a883ea
Compare
jechol
marked this pull request as draft
July 18, 2026 17:20
jechol
force-pushed
the
migration-ordering-and-after-tables
branch
5 times, most recently
from
July 19, 2026 09:33
8933fa9 to
28ccc51
Compare
…aph topological sort Replaced the ~69-clause pairwise after?/2 insertion sort in MigrationGenerator.sort_operations/2 with an explicit dependency-graph model: each operation provides/requires facts (operation_deps.ex), and toposort_operations/1 runs Kahn's algorithm over the resulting graph, raising OperationCycleError on a genuine cycle instead of silently falling back to an arbitrary order. The old insertion sort had two structural problems: it's non-transitive and clause-order-dependent (two pairs of clauses were found to be dead code — a broader/earlier clause always shadowed a more specific/later one for the same struct pair), and it had no general notion of an operation declaring a dependency on a fact, which the old global "always last"/"always first" tiers for AddCheckConstraint, AlterDeferrability, RemovePrimaryKeyDown, AddPrimaryKey papered over with hardcoded ad-hoc rules rather than real DDL requirements. streamline/2, group_into_phases/3, clean_phases/1 are unchanged; they only consume the final order. RenameTable's struct is normalized to use the same generic :table field every other operation struct uses (previously :old_table/:new_table), since the new ordering can place it next to code that reads op.table directly without special-casing rename operations. Verified against a large real-world Ash application (~150 resources) via 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. This 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 clause 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, creating a genuine two-way cycle between that attribute and the index. test/migration_generator_test.exs: 102/102 passing (two tests fixed for pre-existing fragility exposed by valid reordering). New test/migration_generator/operation_deps_test.exs unit-tests the provides/requires model directly. Full mix test: 860 passed.
jechol
force-pushed
the
migration-ordering-and-after-tables
branch
from
July 19, 2026 10:05
28ccc51 to
bd9b300
Compare
jechol
marked this pull request as ready for review
July 19, 2026 10:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
custom_statementshad no way to declare "this depends on another table's structure being finalized first." The only rule was "adds always run last, removes always run first." That breaks for a raw-SQL statement that needs to run after some other table is fully set up — e.g. a composite foreign key referencing a unique index that isn't guaranteed to exist yet.What changed
AshPostgres.Statementgets a newafter_tablesoption (a list of table name strings) so a statement can declare that dependency explicitly:Internally this required splitting the single
:table_finalizedfact into two::table_structure_ready— this table's structural operations (columns, indexes, constraints, etc.) are done.:table_finalized— the table is truly done, including anycustom_statementsdeclared on it.A statement's own implicit "wait for my own table" requirement uses the narrower
:table_structure_ready, so two statements on the same table never end up requiring each other's completion (a guaranteed cycle if both used the broader fact). A declaredafter_tablesentry uses the broader:table_finalized, so it also waits for the referenced table's own custom statements — not just its structure.Statements with no
after_tableskeep the previous default behavior exactly (no declared dependency → best-effort last), so this is fully backwards compatible.Testing
Builds on the dependency-graph rewrite in the #798 . New tests in
test/migration_generator/operation_deps_test.exscover: a statement's own-table requirement being satisfied by structural ops, anafter_tablesrequirement being satisfied by another table's custom statement (the actual new capability), and two same-table statements not creating a cycle. Fullmix test: 858 passed.Contributor checklist
Leave anything that you believe does not apply unchecked.