elixir-sqlite / ecto_sqlite3

An Ecto SQLite3 adapter.
https://hexdocs.pm/ecto_sqlite3
MIT License
300 stars 45 forks source link

Encodes nil blobs for updates #140

Closed JesseStimpson closed 9 months ago

JesseStimpson commented 9 months ago

I recently upgraded some dependencies and found what appears to be a regression in ecto_sqlite3's ability to update blob/binary fields into NULL (nil).

This PR contains a new test case for this purpose. Excerpt:

    assert %Setting{checksum: nil} = setting
             |> Setting.changeset(%{checksum: nil})
             |> TestRepo.update!()

On the latest main, the following error is generated:

10:48:30.087 [error] ** State machine #PID<0.326.0> terminating
** Reason for termination ==
** (DBConnection.ConnectionError) client #PID<0.942.0> stopped: ** (Protocol.UndefinedError) protocol String.Chars not implemented for {:wrong_type, {:blob, nil}} of type Tuple. This protocol is implemented for the following type(s): Atom, BitString, Date, DateTime, Decimal, Exqlite.Query, Float, Integer, List, NaiveDateTime, Time, URI, Version, Version.Requirement
    (elixir 1.15.7) lib/string/chars.ex:3: String.Chars.impl_for!/1
    (elixir 1.15.7) lib/string/chars.ex:22: String.Chars.to_string/1
    (exqlite 0.19.0) lib/exqlite/connection.ex:657: Exqlite.Connection.bind_params/3
    (exqlite 0.19.0) lib/exqlite/connection.ex:616: Exqlite.Connection.execute/4
    (ecto_sql 3.11.1) lib/ecto/adapters/sql/sandbox.ex:384: Ecto.Adapters.SQL.Sandbox.Connection.proxy/3
    (db_connection 2.6.0) lib/db_connection/holder.ex:354: DBConnection.Holder.holder_apply/4
    (db_connection 2.6.0) lib/db_connection.ex:1512: DBConnection.run_execute/5
    (db_connection 2.6.0) lib/db_connection.ex:1607: DBConnection.run/6
    (db_connection 2.6.0) lib/db_connection.ex:800: DBConnection.execute/4
    (ecto_sqlite3 0.15.0) lib/ecto/adapters/sqlite3/connection.ex:89: Ecto.Adapters.SQLite3.Connection.query/4
    (ecto_sql 3.11.1) lib/ecto/adapters/sql.ex:1108: Ecto.Adapters.SQL.struct/10
    (ecto 3.11.1) lib/ecto/repo/schema.ex:775: Ecto.Repo.Schema.apply/4
    (ecto 3.11.1) lib/ecto/repo/schema.ex:467: anonymous fn/15 in Ecto.Repo.Schema.do_update/4
    (ecto 3.11.1) lib/ecto/repo/schema.ex:286: Ecto.Repo.Schema.update!/4
    test/ecto/integration/blob_test.exs:22: Ecto.Integration.BlobTest."test updates blob to nil"/1
    (ex_unit 1.15.7) lib/ex_unit/runner.ex:463: ExUnit.Runner.exec_test/2
    (stdlib 5.1.1) timer.erl:270: :timer.tc/2
    (ex_unit 1.15.7) lib/ex_unit/runner.ex:385: anonymous fn/5 in ExUnit.Runner.spawn_test_monitor/4

Including my small change to blob_encode/1 allows all tests to pass:

Finished in 0.3 seconds (0.3s async, 0.04s sync)
314 tests, 0 failures

Please let me know what you think!

-Jesse

warmwaffles commented 9 months ago

Just need to get the linting issue sorted and I'll merge it in. Thank you for fixing this!

JesseStimpson commented 9 months ago

Thanks @warmwaffles