launchbadge / sqlx

🧰 The Rust SQL Toolkit. An async, pure Rust SQL crate featuring compile-time checked queries without a DSL. Supports PostgreSQL, MySQL, and SQLite.
Apache License 2.0
13k stars 1.24k forks source link

sqlite: query! borrows Copy parameters #1151

Open NyxCode opened 3 years ago

NyxCode commented 3 years ago

I've struck a strange issue with sqlite. This code

fn query<'a, 'c: 'a>(
    con: impl Executor<'c, Database = Sqlite> + 'a,
    limit: i64,
    offset: i64,
) -> BoxStream<'a, sqlx::Result<i64>> {
    sqlx::query_scalar!("SELECT id FROM users LIMIT ? OFFSET ?", limit, offset).fetch(con)
}

fails to compile with sqlite with this error:

error[E0515]: cannot return value referencing function parameter `limit`
  --> example-sqlite/src/main.rs:23:5
   |
23 |     sqlx::query_scalar!("SELECT id FROM users LIMIT ? OFFSET ?", limit, offset).fetch(con)
   |     ---------------------------------------------------------------------------^^^^^^^^^^^
   |     |
   |     returns a value referencing data owned by the current function
   |     `limit` is borrowed here

error[E0515]: cannot return value referencing function parameter `offset`
  --> example-sqlite/src/main.rs:23:5
   |
23 |     sqlx::query_scalar!("SELECT id FROM users LIMIT ? OFFSET ?", limit, offset).fetch(con)
   |     ---------------------------------------------------------------------------^^^^^^^^^^^
   |     |
   |     returns a value referencing data owned by the current function
   |     `offset` is borrowed here

error: aborting due to 2 previous errors; 2 warnings emitted

The same happens with query! and query_as!. The equivalent code compiles fine on MySql and Postgres. It's only on sqlite that the stream borrows the parameters.

breakpointninja commented 2 years ago

I see that this error is raised because a local value here is borrowed. The local value itself is borrowed from the argument bindings. This means that even if a reference with static lifetime is used for argument binding, we will still see an error.

let arg_bindings = quote! {
    #(let #arg_name = &(#arg_expr);)*
};

IMHO, passing by reference should be the library users responsibility. But we can't break this syntax now. Maybe this should be part of #875.

I have a more backward compatible solution in mind. Would it make sense to allow prepending & before the argument to signal that its already a reference? So if arg_expr is already a reference then we don't prepend &. This would allow the following code to work

fn query<'a, 'c: 'a>(
    con: impl Executor<'c, Database = Sqlite> + 'a,
    limit: &'x i64,
    offset: &'x i64,
) -> BoxStream<'x, sqlx::Result<i64>>
  where
    'a: 'x
{
    sqlx::query_scalar!("SELECT id FROM users LIMIT ? OFFSET ?", 
       &limit,  // <---- `&` here signals this is already a reference
       &offset, // <---- `&` here signals this is already a reference
    ).fetch(con)
}

@abonander I think this is simple enough and I can give a PR. Do you think its worth spending time on this?

Crazytieguy commented 2 months ago

I just ran into this as well, is there any way to work around this?