From 0a1822ab990a8d79d8cc11fa513f2be5494bc290 Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Wed, 30 Sep 2026 11:49:46 -0600 Subject: [PATCH] Skips V03's index rename on SQLite StatifierRouter.Migrations.V03 renamed the subscription index with ALTER INDEX IF EXISTS on every adapter. SQLite has no ALTER INDEX, so a SQLite host whose router migration walks through V03 failed with a syntax error near "INDEX" from 0.8.0 on. On Ecto.Adapters.SQLite3 both directions now do nothing: SQLite keeps an identifier whole, so V02's index there already holds its whole name and there is no cut name to repair. Every other adapter runs the rename as before; the Postgres migration tests pass unchanged. Adds ecto_sqlite3 as a test-only dependency, a SQLite test repo and StatifierRouter.SQLiteMigrationsTest, which walks V01 to V03 up and down on SQLite and reads the index name back. ADR-0007 gets a dated foot Note; a changelog fragment records the fix. Refs: sr-zb9m --- changelog.d/sr-zb9m.md | 6 + docs/adr/0007-the-source-invoke.md | 23 +++ lib/statifier_router/migrations.ex | 4 +- lib/statifier_router/migrations/v03.ex | 17 +- mix.exs | 5 +- mix.lock | 4 + .../sqlite_migrations_test.exs | 151 ++++++++++++++++++ test/support/sqlite_repo.ex | 12 ++ 8 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 changelog.d/sr-zb9m.md create mode 100644 test/statifier_router/sqlite_migrations_test.exs create mode 100644 test/support/sqlite_repo.ex diff --git a/changelog.d/sr-zb9m.md b/changelog.d/sr-zb9m.md new file mode 100644 index 0000000..35c3811 --- /dev/null +++ b/changelog.d/sr-zb9m.md @@ -0,0 +1,6 @@ +### Fixed + +- Migration V03 runs on SQLite, where it now does nothing: SQLite never + shortened the subscription index name V03 renames on Postgres. V03's + `ALTER INDEX`, which SQLite does not have, failed the migration of a + SQLite host on 0.8.0 and later. diff --git a/docs/adr/0007-the-source-invoke.md b/docs/adr/0007-the-source-invoke.md index 1c17e8a..b7b2520 100644 --- a/docs/adr/0007-the-source-invoke.md +++ b/docs/adr/0007-the-source-invoke.md @@ -375,3 +375,26 @@ Nothing section 6 fixes changes: V01's `_execution_id_index` (`StatifierRouter.Migrations.V01`), the index section 6 names for reading the execution's address row by execution id alone. + +## Note (2026-09-30, sr-zb9m): V03 does nothing on SQLite + +The Note of 2026-09-27 (sr-lyym) says what V03 does, read at `4f76546`: +it renames the subscription index with `ALTER INDEX IF EXISTS`, in both +directions. That was true on every adapter up to `127f2e7`, and on +SQLite the statement is a syntax error, so a host on SQLite whose router +migration walks through V03 failed to migrate from statifier_router +0.8.0 on. This Note records the change that answers it. It decides +nothing, and no line above it was edited. + +On `Ecto.Adapters.SQLite3`, `StatifierRouter.Migrations.V03.up/1` and +`down/1` now do nothing (the private `rename_index/3`). SQLite keeps an +identifier whole, so there V02 already left the index under the whole +name it gave it, with nothing cut to repair; the index keeps that name. +On every other adapter V03 runs the rename exactly as before. The test +`StatifierRouter.SQLiteMigrationsTest` walks the versions on SQLite and +reads the index name back. + +Nothing section 6 fixes changes, for the reasons the Note of 2026-09-27 +gives: the index's columns, their order and its uniqueness are V02's on +every adapter, and `StatifierRouter.subscribe/3` names the triple by its +columns, not by the index's name. diff --git a/lib/statifier_router/migrations.ex b/lib/statifier_router/migrations.ex index bf8d472..5151e17 100644 --- a/lib/statifier_router/migrations.ex +++ b/lib/statifier_router/migrations.ex @@ -131,7 +131,9 @@ defmodule StatifierRouter.Migrations do migration calls `up/1` with no `version:` gets V03 from it on a fresh database, and still writes the migration above for the databases that ran the first one before V03 existed: on a fresh database the second - run finds the index already renamed and does nothing. + run finds the index already renamed and does nothing. On SQLite, which + never cut V02's name, V03 does nothing in either direction and the index + keeps V02's name (`StatifierRouter.Migrations.V03`). ## The location table, V04, is opt-in diff --git a/lib/statifier_router/migrations/v03.ex b/lib/statifier_router/migrations/v03.ex index b538db0..4dc09be 100644 --- a/lib/statifier_router/migrations/v03.ex +++ b/lib/statifier_router/migrations/v03.ex @@ -32,6 +32,13 @@ defmodule StatifierRouter.Migrations.V03 do one that already walked through V03 on a fresh database: the second run finds the index already renamed and leaves it. + On SQLite (`Ecto.Adapters.SQLite3`) both directions do nothing. SQLite + keeps an identifier whole, so V02's index there already holds the name + V02 gave it, with nothing cut to repair, and SQLite has no + `ALTER INDEX`. The index keeps V02's name on SQLite; the package's + queries name its columns, never its name. Every other adapter runs the + rename above. + A host already running V02 reaches this version with `StatifierRouter.Migrations.up(from: 3)`: `from:` names the first version the host has **not** run and the walk includes it. @@ -51,14 +58,14 @@ defmodule StatifierRouter.Migrations.V03 do optional(atom()) => term() } - @doc "Renames V02's subscription index to `
_invocation_index`." + @doc "Renames V02's subscription index to `
_invocation_index`; on SQLite, nothing." @spec up(storage()) :: :ok def up(storage) do {v02_name, v03_name} = names(storage) rename_index(storage, v02_name, v03_name) end - @doc "Renames the subscription index back to the name V02 left it under." + @doc "Renames the subscription index back to the name V02 left it under; on SQLite, nothing." @spec down(storage()) :: :ok def down(storage) do {v02_name, v03_name} = names(storage) @@ -74,8 +81,12 @@ defmodule StatifierRouter.Migrations.V03 do as_stored("#{subscriptions}_invocation_index")} end + # SQLite never cut V02's name, so there is nothing to rename, and it has + # no ALTER INDEX to rename with. defp rename_index(%{prefix: prefix}, from, to) do - execute("ALTER INDEX IF EXISTS #{qualified(prefix, from)} RENAME TO #{quoted(to)}") + if repo().__adapter__() != Ecto.Adapters.SQLite3 do + execute("ALTER INDEX IF EXISTS #{qualified(prefix, from)} RENAME TO #{quoted(to)}") + end :ok end diff --git a/mix.exs b/mix.exs index e8c4044..b118fcd 100644 --- a/mix.exs +++ b/mix.exs @@ -119,7 +119,10 @@ defmodule StatifierRouter.MixProject do # Test-only: a host brings its own database driver, and this package # needs one only to test itself (the rule sp-ADR-0005 records for # statifier_persistence). - {:postgrex, "~> 0.22", only: :test} + {:postgrex, "~> 0.22", only: :test}, + # Test-only, like postgrex: the SQLite repo the migration tests run + # the versions against, since a host may migrate on SQLite. + {:ecto_sqlite3, "~> 0.22", only: :test} ] end end diff --git a/mix.lock b/mix.lock index b0750a2..a66c63f 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,7 @@ %{ "broadway": {:hex, :broadway, "1.3.0", "f75f6376159b74f55c5ba2629dac613e4fd79d9e71148ab5fbac8fdd7c999d2a", [:mix], [{:gen_stage, "~> 1.0", [hex: :gen_stage, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.3.7 or ~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "bef3b4c5512d0072917b70239cbecf8f76a2587465a5b7c3e2b9ae18b4bc405b"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, + "cc_precompiler": {:hex, :cc_precompiler, "0.1.11", "8c844d0b9fb98a3edea067f94f616b3f6b29b959b6b3bf25fee94ffe34364768", [:mix], [{:elixir_make, "~> 0.7", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "3427232caf0835f94680e5bcf082408a70b48ad68a5f5c0b02a3bea9f3a075b9"}, "credo": {:hex, :credo, "1.7.19", "cc52129665fc7c15143d47838fda0f9cd6dac9ceced7bf4da6f85fcbfe64b12a", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2d8bc95d5a7bb99dd2613621d4f08c6a3575c3fd4b62e6a2b48a100352a557b8"}, "db_connection": {:hex, :db_connection, "2.10.2", "ae391e803a5adff104da913c2fc1c0c14a37f8b10001dcef568796e1fb7bf95c", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "510b14482330f1af6490a2fa0efd8d4f1435d1529b165647df22ac0f2df0fa93"}, "decimal": {:hex, :decimal, "3.1.1", "430d87b04011ce6cbd4fd205be758311a81f87d552d40904abd00f015935b1d0", [:mix], [], "hexpm", "c5f25f2ced74a0587d03e6023f595db8e924c9d3922c8c8ffd9edfc4498cf1f6"}, @@ -8,10 +9,13 @@ "earmark_parser": {:hex, :earmark_parser, "1.4.46", "67607a0532e810c6f630a515c548d0b24949643f168cc556303bee4cf96105c7", [:mix], [], "hexpm", "9c44636e8a1c68c62f526b2dcd85d941dbbcee7ab82cf64ba06ce28bef8e89f5"}, "ecto": {:hex, :ecto, "3.14.2", "99db28a864293a789c970651de711e3cae184291e0e7ea1166c54055ac41c1f3", [:mix], [{:decimal, "~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "25d60b8c816a07d19d85b80bdf60978bd8b102209dda198d768cd7c6745339a6"}, "ecto_sql": {:hex, :ecto_sql, "3.14.0", "06446ab8410d2f85bfbb80857ee224ab3b693700cbb38f6535d507449a627b2e", [:mix], [{:db_connection, "~> 2.9", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.14.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.8", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.19 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:tds, "~> 2.1.1 or ~> 2.2", [hex: :tds, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "f4d8d36faf294c9417b5a37ec7ac8217ee2abdef5fcf197ba690f361548d3949"}, + "ecto_sqlite3": {:hex, :ecto_sqlite3, "0.25.0", "309898d694b17a8ca8cd88d648d3a7fe8479b04ea6831a18d97d56ed3541031b", [:mix], [{:decimal, "~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.14", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.14", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:exqlite, "~> 0.22", [hex: :exqlite, repo: "hexpm", optional: false]}], "hexpm", "7da65c7af38dccf228320db32f93ae49650b0afdd850a09fd2fb191554b3faf5"}, + "elixir_make": {:hex, :elixir_make, "0.10.0", "16577e2583a79bb79237bbff349619ef5d80afffc07eac6e4faf0d00e2ddaf7d", [:mix], [], "hexpm", "dc1f09fb7fa68866b886abd5f0f3c83553b1a19a52359a899e92af1bb3b31982"}, "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"}, "ex_doc": {:hex, :ex_doc, "0.40.4", "66f2e42bf588594d5a8aab31cad87f2ddad09d0da1b1a2f379340ec2c2e497cb", [:mix], [{:earmark_parser, "~> 1.4.46", [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", "6222b9e423d76584ee34df2c82a5ed72c2d53dc153f7f483ad28b378694186cc"}, "ex_quality": {:hex, :ex_quality, "0.15.1", "238c222406693e483ff56a56fc602146e297f30b4abae7cd5db49f76e402c8e6", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "1d59cd18f14f7849bcefc991717992a35b6bcc4c1bea5165198719db770cfdde"}, "excoveralls": {:hex, :excoveralls, "0.18.5", "e229d0a65982613332ec30f07940038fe451a2e5b29bce2a5022165f0c9b157e", [:mix], [{:castore, "~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "523fe8a15603f86d64852aab2abe8ddbd78e68579c8525ae765facc5eae01562"}, + "exqlite": {:hex, :exqlite, "0.41.0", "f7b6d9730d19efd8a2c9d4624172e82d4f98f2094743429c111972994967b4e2", [:make, :mix], [{:cc_precompiler, "~> 0.1", [hex: :cc_precompiler, repo: "hexpm", optional: false]}, {:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.8", [hex: :elixir_make, repo: "hexpm", optional: false]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "a7e9b6bed529ab72aa07ed2a925ac109c27e6877a7a8af252361c396a4192855"}, "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, "gen_stage": {:hex, :gen_stage, "1.3.2", "7c77e5d1e97de2c6c2f78f306f463bca64bf2f4c3cdd606affc0100b89743b7b", [:mix], [], "hexpm", "0ffae547fa777b3ed889a6b9e1e64566217413d018cabd825f786e843ffe63e7"}, "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"}, diff --git a/test/statifier_router/sqlite_migrations_test.exs b/test/statifier_router/sqlite_migrations_test.exs new file mode 100644 index 0000000..16c180f --- /dev/null +++ b/test/statifier_router/sqlite_migrations_test.exs @@ -0,0 +1,151 @@ +defmodule StatifierRouter.SQLiteMigrationsTest do + # The version walk on SQLite, through ecto_sqlite3, against a database + # file of each test's own: no Postgres, no SQL sandbox, nothing shared + # with the rest of the suite. + use ExUnit.Case, async: true + + alias Ecto.Adapters.SQL + alias Ecto.Migrator + alias StatifierRouter.SQLiteRepo + + # A host on SQLite at the default table prefix: its migration for V01 and + # V02, written before V03 existed, and the one it adds for V03. + defmodule MigrateThroughV02 do + @moduledoc false + use Ecto.Migration + + def up, do: StatifierRouter.Migrations.up(version: 2) + def down, do: StatifierRouter.Migrations.down(from: 2) + end + + defmodule MigrateV03 do + @moduledoc false + use Ecto.Migration + + def up, do: StatifierRouter.Migrations.up(from: 3) + def down, do: StatifierRouter.Migrations.down(from: 3, version: 3) + end + + # A host whose one migration calls up/1 and down/1 with no version. + defmodule MigrateAll do + @moduledoc false + use Ecto.Migration + + def up, do: StatifierRouter.Migrations.up() + def down, do: StatifierRouter.Migrations.down() + end + + @through_v02_version 20_260_930_000_401 + @v03_version 20_260_930_000_402 + @all_version 20_260_930_000_403 + + @subscriptions "statifier_router_subscriptions" + @v02_spelling "statifier_router_subscriptions_execution_id_binding_id_invoke_id_index" + + setup do + database = + Path.join( + System.tmp_dir!(), + "statifier_router_sqlite_#{System.unique_integer([:positive])}.db" + ) + + start_supervised!({SQLiteRepo, database: database, pool_size: 1}) + + on_exit(fn -> + for suffix <- ["", "-wal", "-shm"], do: File.rm(database <> suffix) + end) + + :ok + end + + # The migration's result, or the message of what it raised, so a + # version the adapter refuses fails the assertion that names it. + defp migrate(direction, version, module) do + apply(Migrator, direction, [SQLiteRepo, version, module, [log: false]]) + rescue + error -> {:raised, Exception.message(error)} + end + + # name => {unique?, columns in index order}, for every index SQLite + # holds on the table, the ones it makes for itself left out. + defp indexes(table) do + %{rows: rows} = + SQL.query!( + SQLiteRepo, + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = ?1 " <> + "AND name NOT LIKE 'sqlite_autoindex_%'", + [table] + ) + + Map.new(rows, fn [name] -> {name, index(name)} end) + end + + defp index(name) do + %{rows: [[unique]]} = + SQL.query!(SQLiteRepo, "SELECT \"unique\" FROM pragma_index_list(?1) WHERE name = ?2", [ + @subscriptions, + name + ]) + + %{rows: columns} = + SQL.query!(SQLiteRepo, "SELECT name FROM pragma_index_info(?1) ORDER BY seqno", [name]) + + {unique == 1, List.flatten(columns)} + end + + defp router_tables do + %{rows: rows} = + SQL.query!( + SQLiteRepo, + "SELECT name FROM sqlite_master WHERE type = 'table' AND name LIKE 'statifier_router_%'", + [] + ) + + rows |> List.flatten() |> Enum.sort() + end + + describe "V03 on SQLite" do + # sabotage: made V03 run its ALTER INDEX on every adapter, as it did + # before it skipped SQLite -> red, the V03 up raised a syntax error + # near "INDEX". + test "leaves V02's index under the whole name SQLite gave it, up and down" do + assert migrate(:up, @through_v02_version, MigrateThroughV02) == :ok + + # SQLite keeps a name whole: V02's 70 bytes, which Postgres would cut + # to 63 and which V03 renames there. + assert byte_size(@v02_spelling) == 70 + v02_indexes = indexes(@subscriptions) + + assert %{@v02_spelling => {true, ["execution_id", "binding_id", "invoke_id"]}} = + v02_indexes + + assert migrate(:up, @v03_version, MigrateV03) == :ok + assert indexes(@subscriptions) == v02_indexes + + assert migrate(:down, @v03_version, MigrateV03) == :ok + assert indexes(@subscriptions) == v02_indexes + + assert migrate(:down, @through_v02_version, MigrateThroughV02) == :ok + assert router_tables() == [] + end + + # sabotage: made V03's down/1 run its ALTER INDEX on every adapter + # -> red, the rollback raised a syntax error near "INDEX". + test "is walked by up/1 and down/1 with no version, as a host's first migration" do + assert migrate(:up, @all_version, MigrateAll) == :ok + + assert router_tables() == [ + "statifier_router_addresses", + "statifier_router_dedupe", + "statifier_router_routing_ledger", + @subscriptions + ] + + assert %{@v02_spelling => {true, ["execution_id", "binding_id", "invoke_id"]}} = + indexes(@subscriptions) + + assert migrate(:down, @all_version, MigrateAll) == :ok + assert router_tables() == [] + end + end +end diff --git a/test/support/sqlite_repo.ex b/test/support/sqlite_repo.ex new file mode 100644 index 0000000..a7d4382 --- /dev/null +++ b/test/support/sqlite_repo.ex @@ -0,0 +1,12 @@ +defmodule StatifierRouter.SQLiteRepo do + @moduledoc """ + An Ecto repo on SQLite, for the tests that run the package's migration + versions against an adapter other than Postgres. It has no config of its + own: each test starts it with the database file it migrates. Test-only + support code, not part of the package's public API. + """ + + use Ecto.Repo, + otp_app: :statifier_router, + adapter: Ecto.Adapters.SQLite3 +end