improvement: add rebuild_table, a migration helper that rebuilds a table

[a-nassim]

Closes #239.

Adds opt-in table rebuilds for changes sqlite can't make in place (--rebuild-tables, or rebuild_tables: true in the repo config).
The generator then writes a rebuild_table migration (create, copy, drop, rename, as in the sqlite docs) instead of a failing statement.

By default, generated migrations are unchanged and only a hint says what could not be done.

It needs before_transaction/1 and after_transaction/1 from elixir-ecto/ecto_sql#758 (merged, not released yet).

I suggest reading by commit, but it's still quite large. The last 3 could be separate if you prefer.

Manual testing

The steps below were drafted with AI help. I ran them myself against a fresh Ash project.

Setup

mix igniter.new support_desk --install ash,ash_sqlite --example --extend sqlite --yes
cd support_desk
perl -pi -e 's|\.\./path/to/your\.db|support_desk_dev.db|' config/dev.exs
perl -0pi -e 's|\{:ash_sqlite, "~> 0.2"\},|{:ash_sqlite, github: "a-nassim/ash_sqlite", branch: "improvement/rebuild-table"},\n      {:ecto_sql, github: "elixir-ecto/ecto_sql", ref: "bf126d6b", override: true},\n      {:ecto, "~> 3.13", override: true},|' mix.exs
rm -rf _build/dev/lib/support_desk/priv && mkdir priv
mix deps.get && mix compile --force && mix ash.codegen create_tables && mix ash.setup
sqlite3 support_desk_dev.db "insert into tickets (id, subject, status) values ('a', 'Printer', 'open'), ('b', 'Login', 'open')"

(The ecto_sql and ecto lines are only needed until elixir-ecto/ecto_sql#758 is released; Mix only honors override in your own mix.exs, not the one in this branch. A min_sqlite_version warning is printed when compiling; it comes from upstream.)

1. Make an attribute optional. In lib/support_desk/support/ticket.ex, replace the subject attribute with:

    attribute :subject, :string do
      public? true
    end

mix ash.codegen subject_optional --dry-run

Prints (in yellow) what SQLite can't do in place, then a modify :subject, :text, null: true migration that would fail.

mix ash.codegen subject_optional --rebuild-tables && mix ash.migrate
sqlite3 support_desk_dev.db "select id, subject from tickets; select sql from sqlite_master where name='tickets'"

The migration is a rebuild_table :tickets with a comment saying why, subject has no NOT NULL any more, and both tickets are still there. (It also lists modify :status ... default: "open": a plain default: becomes the column default.)

2. Add a required attribute without a default. In the same file, add above the status attribute:

    attribute :priority, :integer do
      allow_nil? false
      public? true
    end

mix ash.codegen add_priority --rebuild-tables && mix ash.migrate

The migration has a REVIEW: note, and mix ash.migrate stops with NOT NULL constraint failed: tickets_rebuild.priority, changing nothing. Give the existing rows a value, as the note says:

perl -pi -e 's/rebuild_table :tickets do/rebuild_table :tickets, copy: [priority: "1"] do/' priv/repo/migrations/*_add_priority.exs
mix ash.migrate
sqlite3 support_desk_dev.db "select id, subject, priority from tickets"

Both rows have priority = 1.

3. Change a type. Change priority to a string:

    attribute :priority, :string do
      allow_nil? false
      public? true
    end

mix ash.codegen priority_text --rebuild-tables && mix ash.migrate
sqlite3 support_desk_dev.db "select id, priority, typeof(priority) from tickets"

typeof is now text. The migration comment warns that a value that doesn't convert would be copied as it is.

4. Drop a foreign key. First give a ticket a representative, while the foreign key still exists:

sqlite3 support_desk_dev.db "insert into representatives (id, name) values ('r', 'Ada'); update tickets set representative_id = 'r' where id = 'a'"

In lib/support_desk/support/ticket.ex, delete this block:

  relationships do
    belongs_to :representative, SupportDesk.Support.Representative do
      public? true
    end
  end

and add this attribute above status:

    attribute :representative_id, :uuid do
      public? true
    end

mix ash.codegen drop_fk --rebuild-tables && mix ash.migrate
sqlite3 support_desk_dev.db "select id, representative_id from tickets; select count(*) from pragma_foreign_key_list('tickets')"

The migration comment says dropping the foreign key tickets_representative_id_fkey. The query shows a|r, b| and 0: the rows keep their value and the table has no foreign key any more. Without --rebuild-tables, the generated migration raises "SQLite does not support dropping foreign key constraints".

5. Roll back.

mix ash_sqlite.rollback -n 1
sqlite3 support_desk_dev.db "select id, representative_id from tickets; select \"table\" from pragma_foreign_key_list('tickets')"

The foreign key to representatives is back (representatives) and the rows are still there (a|r, b|).

Rebuild cases

Behaviour on ash_sqlite main, and with rebuild_tables

AI drafted, checked manually

Change	ecto_sqlite3, ash_sqlite main	ecto_libsql, ash_sqlite main	ecto_sqlite3 and ecto_libsql, with rebuild_tables (this branch)
Column becomes nullable	up fails: ecto_sqlite3 raises ALTER COLUMN not supported by SQLite3 on the modify that ash_sqlite generated.	up and down run without error.	up and down run and the rows are kept. up drops NOT NULL and down restores it.
Column becomes required	up fails: ecto_sqlite3 raises ALTER COLUMN not supported by SQLite3 on the modify that ash_sqlite generated.	up and down run without error. A row with NULL in the column keeps the NULL while the column is now NOT NULL, with no error.	up and down run and the rows are kept. up makes the column NOT NULL and down makes it optional again. If a row has NULL in the column, up fails in SQLite itself (NOT NULL constraint failed) and nothing is changed. The migration has a REVIEW comment that says so, with a copy: example.
Column changes type	up fails: ecto_sqlite3 raises ALTER COLUMN not supported by SQLite3 on the modify that ash_sqlite generated.	up and down run without error. The declared type changes, but existing values are not converted (a text '1' stays text in a BIGINT column).	up and down run and the rows are kept. Existing values are converted (a text '1' becomes the integer 1), and down converts them back to text.
Column gets a default	up fails: ecto_sqlite3 raises ALTER COLUMN not supported by SQLite3 on the modify that ash_sqlite generated.	up and down run without error.	up and down run and the rows are kept. up adds the default and down removes it. Existing rows with NULL stay NULL.
Primary key becomes composite	up fails: ecto_sqlite3 raises SQLite3 does not support ALTER TABLE DROP CONSTRAINT on the drop constraint that ash_sqlite generated. The migration also only marks the changed column as primary key, so the key would not be composite even if it ran.	up fails: ecto_libsql raises LibSQL/SQLite does not support ALTER TABLE DROP CONSTRAINT on the drop constraint that ash_sqlite generated. The migration also only marks the changed column as primary key, so the key would not be composite even if it ran.	up and down run and the rows are kept. up makes the primary key (id, code) and down makes it id again.
Foreign key dropped, on_delete changed, or made deferrable	up fails: the migration ash_sqlite generated contains raise "SQLite does not support dropping foreign key constraints…".	up fails: the migration ash_sqlite generated contains raise "SQLite does not support dropping foreign key constraints…".	up and down run and the rows are kept. up changes ON DELETE or removes the foreign key, and down restores it. A change to deferrable alone is also rebuilt, and the table stays the same (see the trade-offs).
New foreign key column	up runs. down fails: the migration ash_sqlite generated contains raise "SQLite does not support dropping foreign key constraints…".	up runs. down fails: the migration ash_sqlite generated contains raise "SQLite does not support dropping foreign key constraints…".	up and down run and the rows are kept. up adds the column with its foreign key, and down rebuilds the table without the column.
New NOT NULL column without a default	Table with rows: up fails in SQLite itself, Cannot add a NOT NULL column with default value NULL. Empty table: up and down run.	Table with rows: up fails in SQLite itself, Cannot add a NOT NULL column with default value NULL. Empty table: up runs, and down fails because of a bug in ecto_libsql (no function clause matching in Ecto.Adapters.LibSql.Connection.execute_ddl/1), which it hits for any added column.	Table with rows: up fails in SQLite itself (NOT NULL constraint failed) because there is no value to copy, and nothing is changed. The migration has a REVIEW comment that says how to give the rows a value (copy: or a default). Empty table, on both: up and down run, including down on ecto_libsql, which a plain drop column rollback does not.
Column renamed and changed together	up and down run. The migration ash_sqlite generated only renames the column. The change is not in it.	up and down run. The migration ash_sqlite generated only renames the column. The change is not in it.	up and down run and the rows are kept. up renames the column and applies the change, and down renames it back and restores the old definition.
Table becomes STRICT or stops being	ash_sqlite generates no migration.	ash_sqlite generates no migration.	On ecto_sqlite3, up and down run and the rows are kept: the table becomes STRICT, or stops being. On ecto_libsql, up fails with no function clause matching in Keyword.get/3

Tradeoffs

• Tried to maintain existing behaviour when the option is off, apart from hints
• ecto_libsql is not distinguished: changes it runs natively are rebuilt too
• Adding a new foreign key column: only the way down fails, but both directions generate a rebuild, to keep it simple
• A new NOT NULL column without a default: rebuild even when the table is empty, because the generator does not know whether the table has rows
• Deferrable foreign keys are not handled: ecto adapters cannot create deferrable constraints (plain sqlite can). Setting deferrable on an existing reference still generates a rebuild (unnecessary, keep the table identical unless there are other changes). Not rebuilding when deferrability is the only foreign key change would fix that, but this PR is already large

Contributor checklist

Leave anything that you believe does not apply unchecked.

• I accept the AI Policy
• Bug fixes include regression tests
• Chores
• Documentation changes
• Features include unit/acceptance tests
• Refactoring
• Update dependencies

[zachdaniel]

Given something like a rolling deploy, does this do anything to prevent writes to the table while we're undergoing this process?

[zachdaniel]

wonder if we should error instead, and then have them pass like an explicit --no-rebuild if they want to skip it, and we just ignore it then and don't even try to generate migrations that we know won't work.

[a-nassim]

I would say eventually yes, but initially i didn't want to force it on users since it's quite a big change, and users that do want that behaviour can easily flip the config at the repo level, to not have to think about it

It also depends on whether you want to support multiple ecto_sql versions. Since older ones lack the required callbacks they cannot have this default rebuild behaviour.

[zachdaniel]

Right but at the moment don't the migratiosn that emit alter statements just straight up fail?

[a-nassim]

True, in most cases it fails with ecto_sqlite3 and ecto_libsql runs some of them, I listed every case in the updated description

Also reading again I think I misunderstood the first comment 😅

Is this what you meant?

• the task should raise when rebuild_tables isn't set and a change needs a rebuild
• --no-rebuild should leave failing changes out of the migration
• --rebuild-tables is what this PR does

[zachdaniel]

Had some comments, in general this looks good but we can't merge it until we could reasonably deploy it unless you can do something to make it support both old and new versions of ecto_sql when it releases.

[a-nassim]

Given something like a rolling deploy, does this do anything to prevent writes to the table while we're undergoing this process?

I actually didn't know it was possible to have a rolling deploy with sqlite. On a single host, it seems possible to have overlapping containers sharing one db. The current PR on its own doesn't do anything to prevent writes during the migration, the guarantee comes from sqlite itself and its single writer model. So if the migration transaction acquires the write lock, concurrent writes either wait or fail.

EDIT: added a guard to ensure the migration runs inside a transaction f652cd3

[a-nassim]

Had some comments, in general this looks good but we can't merge it until we could reasonably deploy it unless you can do something to make it support both old and new versions of ecto_sql when it releases.

Yes of course, I am fine with that! I thought it could be useful to already open the PR even if it's not merged anytime soon

I didn't think about supporting multiple ecto_sql versions, I explored this in 104a56a and 39b2fa0 does that look reasonable? Currently the CI would only run against a single ecto_sql version, the rebuild table part is not covered unless we change the workflow. So up to you, maybe waiting for the ecto_sql release is simpler