Bind named query parameter

[rhcarvalho]

In addition to positional params, SQLite supports named query parameters. I tend to prefer named params when writing queries by hand as it is more descriptive and less error prone than positional params.

sqlite> .parameter set :name John
sqlite> SELECT upper(:name);
┌──────────────┐
│ upper(:name) │
├──────────────┤
│ 'JOHN'       │
└──────────────┘

Is it possible to achieve something similar with Ecto/Exqlite?

[rhcarvalho]

I tried digging a bit to find answers.

The definition of Ecto.Adapters.SQL.query_params/0 suggests it should be possible to pass a map of parameter => value:

@type query_params() :: [term()] | %{required(atom() | String.t()) => term()}

However, when I try that I get an error:

iex> MyApp.Repo.query("select upper(:name)", %{name: "hello"})
[debug] QUERY ERROR db=0.1ms idle=1627.9ms
select upper(:name) %{name: "hello"}
{:error,
 %Exqlite.Error{
   message: "errors were found at the given arguments:\n\n  * 1st argument: not a list\n",
   statement: "select upper(:name)"
 }}

I traced the error back to this code:

exqlite/lib/exqlite/sqlite3.ex

Lines 177 to 182 in b8035f2

	@spec bind(statement, [bind_value] | nil) :: :ok
	def bind(stmt, nil), do: bind(stmt, [])
	def bind(stmt, args) do
	params_count = bind_parameter_count(stmt)
	args_count = length(args)

Calling length/1 on the map raises the "not a list" error. The code expects a list of params, not a map.

I suspect the functionality is not supported, but maybe I'm writing the query wrong?

[warmwaffles]

@rhcarvalho this is a limitation of Ecto itself. We used numbered bindings because as these queries grow in complexity beyond your simple example, it gets more and more difficult to track down the issue.

In your example, you should be using Ecto to compose the query and avoid using fragments unless absolutely necessary. Using ecto in the way you are, it won't be possible to use named bindings. I am not going down that path with the adapter right now and I don't really want to support that there.

However, within Exqlite itself, we can potentially alter Sqlite3.bind_all/3 to try and support named parameters to call sqlite3_bind_parameter_index in the NIF code. Once that is in place I may be open to potentially supporting it in the Ecto adapter.

I am going to suggest, setting up your example to query a repo using a postgres database. I am fairly certain it does not allow you to pass named parameters.

I do see the appeal to using them in order to reduce excess binding on larger queries and being more efficient.

[rhcarvalho]

@warmwaffles thanks for the clear and quick reply 💜

I agree with you re: Ecto when working with defined Schema structs directly -- just let Ecto figure out how to get that into a SQL query that works.

My example was just an oversimplification of a more complex query. A more realistic example is converting queries from a Go code base using named params into Elixir.

Go:

        // additional fields omitted
	db.ExecContext(ctx, `
		INSERT INTO comments (
			created, id, tenancy_id, comment, ...
		) VALUES (
			julianday(:created), :comment_id, :tenancy_id, :comment, ...
		)
	`,
		sql.Named("created", comment.Created),
		sql.Named("comment_id", comment.ID),
		sql.Named("tenancy_id", comment.Tenancy),
		sql.Named("comment", commentData.Comment),
                // ...
	)

Elixir:

      # additional fields omitted
      """
      INSERT INTO "comments" ("id","comment","created","deleted","tenancy_id", ...)
      VALUES (?1,?2,julianday(?3),julianday(?4),?5, ...) ON CONFLICT DO NOTHING
      """
      |> MyApp.Repo.query!([
        comment.id,
        comment.comment,
        comment.created,
        comment.deleted,
        comment.tenancy_id,
        # ...
      ])

FWIW, the original Go code base contains more complex queries where a given parameter is used more than once, making named params safer to use than repeating positional args in the right places.

One of the problems with this language conversion Go -> Elixir is that, in this particular case, dates were stored in the database using Julian day, but Exqlite only deals with dates in textual formats (fair, no complaint from my side -- if anything, this would be a separate issue/feature request). The way forward other than transforming the existing data in the database was to use a fragment when reading existing data:

    from(c in "comments",
      where: c.id == ^id,
      select: %{
        c
        | created: type(fragment("datetime(?)", c.created), :utc_datetime),
          deleted: type(fragment("datetime(?)", c.deleted), :utc_datetime)
      }
    )
    |> MyApp.Repo.one()

But I could not get the same to work on the write path. I tried a custom Type, but realized I'd have to do the Julian day conversion in Elixir, but I'd rather delegate that to SQLite.

So next was writing the queries by hand, which could be easily copy-pasted from the existing code base, thus the realization that I could not use named params.

I am going to suggest, setting up your example to query a repo using a postgres database. I am fairly certain it does not allow you to pass named parameters.

I've debated multiple times moving over to Postgres, but the operational simplicity of SQLite and fit for this current use case makes it the unbeatable choice, limitations and all considered.

---

PS: if this turns out to merit a code change, I am up for contributing code and/or code review/test.

[rhcarvalho]

BTW, for the example in the earlier comment, the Ecto.Schema definition being:

  @primary_key {:id, :binary_id, autogenerate: false}
  schema "comments" do
    field :created, :utc_datetime
    field :deleted, :utc_datetime
    field :tenancy_id, :string
    field :comment, :string
    # other fields omitted...
  end

[ruslandoga]

👋

I think it should be possible to do in Ecto (but only in Repo.query)!

%{required(atom() | String.t()) => term()} typespec (Ecto didn't actually enforce the params format) was added for a ClickHouse adapter to avoid Dialyzer warnings and it seems (?) to work.

Internally, when params arrive as a map Exqlite could do something like this but probably more ergonomic.
Ah, it was already suggested in #330 (comment)

[warmwaffles]

I'm open to the idea honestly.

[warmwaffles]

@rhcarvalho this is being released under v0.32.0

[rhcarvalho]

Thank you for this and the ecto_sqlite3 improvements! Upgrading today 💜