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

lib/data_layer.ex

@@ -98,9 +98,10 @@ defmodule AshPostgres.DataLayer do
     describe: """
     A section for configuring custom statements to be added to migrations.
 
-    Changing custom statements may require manual intervention, because Ash can't determine what order they should run
-    in (i.e if they depend on table structure that you've added, or vice versa). As such, any `down` statements we run
-    for custom statements happen first, and any `up` statements happen last.
+    By default, a statement has no declared dependency on other tables, so `down` statements run before any other
+    operation and `up` statements run after all other operations for that statement's table. If your statement's `up`
+    depends on structure from another table (e.g. a foreign key referencing a unique index defined via `identities`),
+    declare it with `after_tables` so the migration generator orders it correctly relative to that table's operations.
 
     Additionally, when changing a custom statement, we must make some assumptions, i.e that we should migrate
     the old structure down using the previously configured `down` and recreate it.
@@ -116,6 +117,13 @@ defmodule AshPostgres.DataLayer do
           up "CREATE INDEX pgweb_idx ON pgweb USING GIN (to_tsvector('english', title || ' ' || body));"
           down "DROP INDEX pgweb_idx;"
         end
+
+        statement :children_parent_composite_fk do
+          # ensures this runs after `parents`'s columns and unique indexes are finalized
+          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
       """
     ],

lib/migration_generator/operation_deps.ex

@@ -21,11 +21,11 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
 
   Requiring a fact waits on *every* operation that provides it, not just one
   — see `toposort_operations/1`'s `provides_index`. That's what makes
-  `:table_structure_ready` work as a catch-all: many operation types provide
-  it, so an op that requires it (currently only `AddCustomStatement`'s
-  own-table requirement) transparently waits for all of that table's
-  structural work, without needing to enumerate every attribute/index/
-  constraint by hand.
+  `:table_structure_ready`/`:table_finalized` work as catch-alls: many
+  operation types provide them, so an op that requires one (e.g.
+  `AddCustomStatement`'s own-table or `after_tables` requirement)
+  transparently waits for all of that table's work, without needing to
+  enumerate every attribute/index/constraint by hand.
 
   Requiring a fact that nothing in the current batch provides is vacuously
   satisfied — it adds no dependency edge at all, rather than blocking or
@@ -64,13 +64,13 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
 
   Table-scoped (`key = {schema, table}`):
 
-  These first three track "how done is this table", but not as one single
+  These first four track "how done is this table", but not as one single
   chain — `:table_ready` is provided by a disjoint set of operations
   (`CreateTable`/`RenameTable`/`MoveTableSchema`) from `:table_columns_settled`
   (`AddAttribute`/`RenameAttribute`/`AlterAttribute`/`RemoveAttribute`), so
   requiring both together is *not* redundant — neither subsumes the other.
-  Both converge at `:table_structure_ready`, which every structural operation
-  provides:
+  Both converge at `:table_structure_ready` and `:table_finalized`, which
+  every structural operation provides:
 
   - `:table_ready` — the table exists (`CreateTable`/`RenameTable`/
     `MoveTableSchema`).
@@ -80,10 +80,20 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
     a check constraint's `check:`) might reference a column that's about to
     be added, altered, renamed, or removed, and can't be parsed to know
     which columns it actually touches.
-  - `:table_structure_ready` — provided by every structural operation on this
-    table (including `:table_ready`'s and `:table_columns_settled`'s
-    providers); a consumer requires it once to mean "wait for all of this
-    table's structural work".
+  - `:table_structure_ready` — this table's structural (DDL) work is done:
+    provided by every structural operation on this table (including
+    `:table_ready`'s and `:table_columns_settled`'s providers).
+  - `:table_finalized` — this table is *truly* done, including any
+    `custom_statements` declared on it: provided by everything that provides
+    `:table_structure_ready`, plus each `AddCustomStatement` on the table (a
+    table with no custom statements is finalized as soon as its structure is
+    ready). Kept separate from `:table_structure_ready` because
+    `AddCustomStatement`'s own implicit "wait for my own table" requirement
+    must use the narrower fact — were it to require `:table_finalized`, two
+    custom statements on the same table would each provide and require the
+    same fact, a guaranteed cycle. Only the explicit, opt-in `after_tables`
+    cross-table reference requires `:table_finalized`, so it also waits for
+    the target table's own custom statements.
 
   Column-scoped (`key = {schema, table, column}`):
 
@@ -154,20 +164,19 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
   def provides(op) do
     case op do
       %Operation.CreateTable{table: table, schema: schema} ->
-        [{:table_ready, key(table, schema)}, {:table_structure_ready, key(table, schema)}]
+        [{:table_ready, key(table, schema)}] ++ structure_ready_facts(table, schema)
 
       %Operation.RenameTable{table: table, schema: schema} ->
-        [{:table_ready, key(table, schema)}, {:table_structure_ready, key(table, schema)}]
+        [{:table_ready, key(table, schema)}] ++ structure_ready_facts(table, schema)
 
       %Operation.MoveTableSchema{table: table, new_schema: schema} ->
-        [{:table_ready, key(table, schema)}, {:table_structure_ready, key(table, schema)}]
+        [{:table_ready, key(table, schema)}] ++ structure_ready_facts(table, schema)
 
       %Operation.AddAttribute{table: table, schema: schema, attribute: attribute} ->
         [
           {:column_ready, key(table, schema, attribute.source)},
-          {:table_columns_settled, key(table, schema)},
-          {:table_structure_ready, key(table, schema)}
-        ]
+          {:table_columns_settled, key(table, schema)}
+        ] ++ structure_ready_facts(table, schema)
 
       %Operation.RenameAttribute{
         table: table,
@@ -176,21 +185,18 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
       } ->
         [
           {:column_ready, key(table, schema, new_attribute.source)},
-          {:table_columns_settled, key(table, schema)},
-          {:table_structure_ready, key(table, schema)}
-        ]
+          {:table_columns_settled, key(table, schema)}
+        ] ++ structure_ready_facts(table, schema)
 
       %Operation.AlterAttribute{table: table, schema: schema} ->
         [
-          {:table_columns_settled, key(table, schema)},
-          {:table_structure_ready, key(table, schema)}
-        ]
+          {:table_columns_settled, key(table, schema)}
+        ] ++ structure_ready_facts(table, schema)
 
       %Operation.RemoveAttribute{table: table, schema: schema} ->
         [
-          {:table_columns_settled, key(table, schema)},
-          {:table_structure_ready, key(table, schema)}
-        ]
+          {:table_columns_settled, key(table, schema)}
+        ] ++ structure_ready_facts(table, schema)
 
       %Operation.AddUniqueIndex{table: table, schema: schema, identity: identity} = op ->
         columns =
@@ -200,15 +206,13 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
             Map.get(op, :multitenancy)
           )
 
-        [
-          {:table_structure_ready, key(table, schema)},
-          {:unique_index_created, key(table, schema, columns)}
-        ]
+        structure_ready_facts(table, schema) ++
+          [{:unique_index_created, key(table, schema, columns)}]
 
       %Operation.RemoveUniqueIndex{table: table, schema: schema, identity: identity} ->
         keys = List.wrap(identity.keys)
 
-        [{:table_structure_ready, key(table, schema)}] ++
+        structure_ready_facts(table, schema) ++
           Enum.map(keys, &{:column_unique_index_removed, key(table, schema, &1)})
 
       %Operation.AddCustomIndex{table: table, schema: schema, index: index} = op ->
@@ -226,11 +230,11 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
             []
           end
 
-        [{:table_structure_ready, key(table, schema)}] ++ unique_facts
+        structure_ready_facts(table, schema) ++ unique_facts
 
       %Operation.RemoveCustomIndex{table: table, schema: schema, index: index} ->
         base =
-          [{:table_structure_ready, key(table, schema)}] ++
+          structure_ready_facts(table, schema) ++
             Enum.map(index.fields, fn field ->
               {:column_custom_index_removed,
                key(table, schema, AshPostgres.CustomIndex.column_name(field))}
@@ -248,16 +252,14 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
         end
 
       %Operation.AddReferenceIndex{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.RemoveReferenceIndex{table: table, schema: schema, source: source} ->
-        [
-          {:table_structure_ready, key(table, schema)},
-          {:reference_index_removed, key(table, schema, source)}
-        ]
+        structure_ready_facts(table, schema) ++
+          [{:reference_index_removed, key(table, schema, source)}]
 
       %Operation.AddCheckConstraint{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.RemoveCheckConstraint{
         table: table,
@@ -267,7 +269,7 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
       } ->
         cols = check_constraint_columns(constraint, multitenancy)
 
-        [{:table_structure_ready, key(table, schema)}] ++
+        structure_ready_facts(table, schema) ++
           Enum.map(cols, &{:column_check_constraint_removed, key(table, schema, &1)})
 
       %Operation.DropForeignKey{
@@ -281,33 +283,39 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
         dest_schema = Map.get(reference, :schema)
 
         [
-          {:column_fk_dropped, key(dest_table, dest_schema, dest_column)},
-          {:table_structure_ready, key(table, schema)}
-        ]
+          {:column_fk_dropped, key(dest_table, dest_schema, dest_column)}
+        ] ++ structure_ready_facts(table, schema)
 
       %Operation.DropForeignKey{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.AlterDeferrability{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.AddPrimaryKey{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.AddPrimaryKeyDown{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.RemovePrimaryKey{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
 
       %Operation.RemovePrimaryKeyDown{table: table, schema: schema} ->
-        [{:table_structure_ready, key(table, schema)}]
+        structure_ready_facts(table, schema)
+
+      %Operation.AddCustomStatement{table: own_table, schema: schema} ->
+        [{:table_finalized, key(own_table, schema)}]
 
       _ ->
         []
     end
   end
 
+  defp structure_ready_facts(table, schema) do
+    [{:table_structure_ready, key(table, schema)}, {:table_finalized, key(table, schema)}]
+  end
+
   @doc "Facts that must already be provided (by some other operation) before `op` can run."
   def requires(op) do
     case op do
@@ -461,8 +469,18 @@ defmodule AshPostgres.MigrationGenerator.OperationDeps do
       %Operation.RemovePrimaryKey{table: table, schema: schema, keys: keys} ->
         Enum.map(List.wrap(keys), &{:column_fk_dropped, key(table, schema, &1)})
 
-      %Operation.AddCustomStatement{table: own_table, schema: schema} ->
-        [{:table_structure_ready, key(own_table, schema)}]
+      %Operation.AddCustomStatement{table: own_table, schema: schema, statement: statement} ->
+        after_tables = statement |> Map.get(:after_tables) |> List.wrap()
+
+        # Own table: the narrower `:table_structure_ready` (not
+        # `:table_finalized`) — using the broader fact here would make two
+        # custom statements on the same table each require the other's
+        # `:table_finalized` (each provides it too), a guaranteed cycle.
+        # Declared `after_tables` targets: the broader `:table_finalized`,
+        # so this statement also waits for *that* table's own custom
+        # statements, not just its structure.
+        [{:table_structure_ready, key(own_table, schema)}] ++
+          Enum.map(after_tables, &{:table_finalized, key(&1, schema)})
 
       _ ->
         []

lib/statement.ex

@@ -10,7 +10,8 @@ defmodule AshPostgres.Statement do
     :up,
     :down,
     :code?,
-    :global?
+    :global?,
+    :after_tables
   ]
 
   defstruct @fields ++ [:__spark_metadata__]
@@ -50,6 +51,13 @@ defmodule AshPostgres.Statement do
       type: :string,
       doc: "How to tear down the structure of the statement",
       required: true
+    ],
+    after_tables: [
+      type: {:list, :string},
+      default: [],
+      doc: """
+      Table names that this statement's `up` depends on being fully finalized (including their columns and indexes) before it runs. Use this when a raw SQL statement references structure (e.g. a foreign key referencing a unique index) on another table so the migration generator can order it correctly.
+      """
     ]
   ]
 

mix.lock

@@ -16,7 +16,7 @@
   "eflame": {:hex, :eflame, "1.0.1", "0664d287e39eef3c413749254b3af5f4f8b00be71c1af67d325331c4890be0fc", [:mix], [], "hexpm", "e0b08854a66f9013129de0b008488f3411ae9b69b902187837f994d7a99cf04e"},
   "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"},
   "ets": {:hex, :ets, "0.9.0", "79c6a6c205436780486f72d84230c6cba2f8a9920456750ddd1e47389107d5fd", [:mix], [], "hexpm", "2861fdfb04bcaeff370f1a5904eec864f0a56dcfebe5921ea9aadf2a481c822b"},
-  "ex_ast": {:hex, :ex_ast, "0.12.10", "1126eb2afe3218e7a8a53ce85adf1056c528c88fbb5adcbe0191b647b842b377", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.7", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "e03f668c4354e3a1382c3d762c0fcc82ca9670f6f37e62b9097ce752be6adf39"},
+  "ex_ast": {:hex, :ex_ast, "0.13.0", "5e5453e2ad73c26e6c8137ea977cce1d010159099a9f257f122db1a821a49f86", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.7", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "bb94a77d056e0c61e8aa65ab7807bea8405b290eb853fba17d09685c232fd5bc"},
   "ex_check": {:hex, :ex_check, "0.16.0", "07615bef493c5b8d12d5119de3914274277299c6483989e52b0f6b8358a26b5f", [:mix], [], "hexpm", "4d809b72a18d405514dda4809257d8e665ae7cf37a7aee3be6b74a34dec310f5"},
   "ex_doc": {:hex, :ex_doc, "0.40.3", "4a972ffe64bc07dc605af487e98fc19b72a4185f55ca031b94c0552d6071c1d9", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "2756e357742fecd9749b489b85d67c9ce99c465f2e75728d9e6dc8d704b973de"},
   "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"},