Skip to content

feat: add after_tables to custom_statements so raw SQL can depend on another table#799

Open
jechol wants to merge 2 commits into
ash-project:mainfrom
jechol:migration-ordering-and-after-tables
Open

feat: add after_tables to custom_statements so raw SQL can depend on another table#799
jechol wants to merge 2 commits into
ash-project:mainfrom
jechol:migration-ordering-and-after-tables

Conversation

@jechol

@jechol jechol commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Motivation

custom_statements had 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.Statement gets a new after_tables option (a list of table name strings) so a statement can declare that dependency explicitly:

custom_statements do
  statement :children_parent_composite_fk do
    after_tables ["parents"]
    up "ALTER TABLE children ADD CONSTRAINT children_parent_fk FOREIGN KEY (region_id, parent_id) REFERENCES parents (region_id, id);"
    down "ALTER TABLE children DROP CONSTRAINT children_parent_fk;"
  end
end

Internally this required splitting the single :table_finalized fact into two:

  • :table_structure_ready — this table's structural operations (columns, indexes, constraints, etc.) are done.
  • :table_finalized — the table is truly done, including any custom_statements declared 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 declared after_tables entry 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_tables keep 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.exs cover: a statement's own-table requirement being satisfied by structural ops, an after_tables requirement being satisfied by another table's custom statement (the actual new capability), and two same-table statements not creating a cycle. Full mix test: 858 passed.

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

@jechol
jechol force-pushed the migration-ordering-and-after-tables branch from bbd0d0a to 5a883ea Compare July 18, 2026 16:54
@jechol
jechol marked this pull request as draft July 18, 2026 17:20
@jechol
jechol force-pushed the migration-ordering-and-after-tables branch 5 times, most recently from 8933fa9 to 28ccc51 Compare July 19, 2026 09:33
jechol added 2 commits July 19, 2026 19:03
…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
jechol force-pushed the migration-ordering-and-after-tables branch from 28ccc51 to bd9b300 Compare July 19, 2026 10:05
@jechol
jechol marked this pull request as ready for review July 19, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant