Correct Ecto_sql v3 migration lock function when using adapter for sqlite3

Hi,

What should the lock_for_migrations function be for sqlite3 when writing an adapter to use with ecto_sql?

Reference issue in ecto.

That’s pretty old comment and things have changed with ecto_sql v3 since then.

My current implementation:

def lock_for_migrations(meta, query, _opts, fun) do
    %{opts: adapter_opts, repo: repo} = meta

    if Keyword.fetch(adapter_opts, :pool_size) == {:ok, 1} do
      raise_pool_size_error()
    end

    query
    |> Map.put(:lock, nil)
    |> repo.all()
    |> fun.()
  end

When running tests this fails with

** (Protocol.UndefinedError) protocol Ecto.Queryable not implemented for [] of type List. This protocol is implemented for the following type(s): Tuple, BitString, Ecto.SubQuery, Atom, Ecto.Query
    (ecto 3.5.5) lib/ecto/queryable.ex:1: Ecto.Queryable.impl_for!/1
    (ecto 3.5.5) lib/ecto/queryable.ex:9: Ecto.Queryable.to_query/1
    (ecto 3.5.5) lib/ecto/repo/queryable.ex:14: Ecto.Repo.Queryable.all/3
    (ecto_sql 3.5.3) lib/ecto/migrator.ex:516: anonymous fn/6 in Ecto.Migrator.lock_for_migrations/4
    (ecto_sql 3.5.3) lib/ecto/migrator.ex:512: Ecto.Migrator.lock_for_migrations/4
    integration/sqlite/test_helper.exs:116: (file)

Looking at the code in question it seems that the locked_query variable is an empty list

_ = migration_repo.all(locked_query, all_opts)

If I don’t call repo.all() in the lock_for_migrations function then the code fails with

{"busy",0}
{"busy",1}
{"busy",2}
{"busy",3}
{"busy",4}
{"busy",5}

18:19:44.272 [error] GenServer #PID<0.402.0> terminating
** (stop) bad return value: :too_many_tries
Last message (from #PID<0.95.0>): {:query_rows, "INSERT INTO \"schema_migrations\" (\"version\",\"inserted_at\") VALUES (?1,?2)", [timeout: :infinity, decode: :manual, types: true, bind: [0, ~N[2020-12-12 16:19:30]]]}

18:19:44.281 [error] Could not update schema migrations. This error usually happens due to the following:

  * The database does not exist
  * The "schema_migrations" table, which Ecto uses for managing
    migrations, was defined by another library
  * There is a deadlock while migrating (such as using concurrent
    indexes with a migration_lock)

To fix the first issue, run "mix ecto.create".

To address the second, you can run "mix ecto.drop" followed by
"mix ecto.create". Alternatively you may configure Ecto to use
another table and/or repository for managing migrations:

    config :ecto_sql, Ecto.Integration.TestRepo,
      migration_source: "some_other_table_for_schema_migrations",
      migration_repo: AnotherRepoForSchemaMigrations

The full error report is shown below.


18:19:44.282 [error] Sqlite.DbConnection.Protocol (#PID<0.384.0>) disconnected: ** (DBConnection.ConnectionError) client #PID<0.426.0> exited
** (Sqlite.DbConnection.Error) {{:bad_return_value, :too_many_tries}, {GenServer, :call, [#PID<0.402.0>, {:query_rows, "INSERT INTO \"schema_migrations\" (\"version\",\"inserted_at\") VALUES (?1,?2)", [timeout: :infinity, decode: :manual, types: true, bind: [0, ~N[2020-12-12 16:19:30]]]}, :infinity]}}
    (ecto_sql 3.5.3) lib/ecto/adapters/sql.ex:751: Ecto.Adapters.SQL.raise_sql_call_error/1
    (ecto 3.5.5) lib/ecto/repo/schema.ex:649: Ecto.Repo.Schema.apply/4
    (ecto 3.5.5) lib/ecto/repo/schema.ex:262: anonymous fn/15 in Ecto.Repo.Schema.do_insert/4
    (ecto_sql 3.5.3) lib/ecto/migrator.ex:672: Ecto.Migrator.verbose_schema_migration/3
    (ecto_sql 3.5.3) lib/ecto/migrator.ex:300: Ecto.Migrator.async_migrate_maybe_in_transaction/7
    (ecto_sql 3.5.3) lib/ecto/migrator.ex:218: anonymous fn/6 in Ecto.Migrator.up/4
    (ecto_sql 3.5.3) lib/ecto/migrator.ex:512: Ecto.Migrator.lock_for_migrations/4
    integration/sqlite/test_helper.exs:116: (file)

So the question is: Should the repo.all() be called or not?