elixir-sqlite / ecto_sqlite3

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

Using placeholders results in `(Exqlite.Error) expected 6 arguments, got 5` #152

Closed axelson closed 3 weeks ago

axelson commented 4 weeks ago

Using ecto's placeholder feature results in an unexpected error:

placeholders = %{timestamp: ~N[2024-10-26 16:40:06]}

attrs = [
  %{
    id: 12911,
    json: %{a: 42},
    author_login: "jason",
    committed_date: ~N[2024-10-25 17:57:25],

    # Inserting with placeholders results in `(Exqlite.Error) expected 6 arguments, got 5`
    inserted_at: {:placeholder, :timestamp},
    updated_at: {:placeholder, :timestamp},

    # Inserting directly successfully inserts the row
    # inserted_at: placeholders.timestamp,
    # updated_at: placeholders.timestamp,
  }
]

Sqlite3Repro.Repo.insert_all(
  Sqlite3Repro.MergedPullRequest,
  attrs,
  placeholders: placeholders
)

Results in:

iex(1)> Sqlite3Repro.run_and_save_merged_prs_query ** (Exqlite.Error) expected 6 arguments, got 5 INSERT INTO "merged_pull_requests" ("id","json","author_login","committed_date","inserted_at","updated_at") VALUES (?,?,?,?,?,?) (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1096: Ecto.Adapters.SQL.raise_sql_call_error/1 (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:967: Ecto.Adapters.SQL.insert_all/9 (ecto 3.12.4) lib/ecto/repo/schema.ex:59: Ecto.Repo.Schema.do_insert_all/7 (sqlite3_repro 0.1.0) lib/sqlite3_repro.ex:60: Sqlite3Repro.run_and_save_merged_prs_query/0 iex:1: (file)

Full repro is at https://github.com/axelson/sqlite3_repro

warmwaffles commented 3 weeks ago

I don't think I implemented placeholders 😞

warmwaffles commented 3 weeks ago

@axelson I have a fix to support {:placeholder, :placeholderkey} the problem is I'm now getting a ** (Exqlite.Error) datatype mismatch. Working through that now.

warmwaffles commented 3 weeks ago

@axelson published under v0.17.3

axelson commented 3 weeks ago

@warmwaffles wow, that was fast! Thank you so much! I just tested it in my full app and it's now working! 🎉

warmwaffles commented 3 weeks ago

No problem. I was meaning to implement cell wise placeholders. This is just the excuse I needed.