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|).

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?

[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