Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encodes nil blobs for updates #140

Merged
merged 1 commit into from
Feb 4, 2024
Merged

Conversation

JesseStimpson
Copy link
Contributor

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
Copy link
Member

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

@JesseStimpson
Copy link
Contributor Author

Thanks @warmwaffles

@warmwaffles warmwaffles merged commit 14f1f76 into elixir-sqlite:main Feb 4, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants