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

Add support for typing using sqlite RETURNING clause #1246

Closed
fabi321 opened this issue May 24, 2021 · 9 comments
Closed

Add support for typing using sqlite RETURNING clause #1246

fabi321 opened this issue May 24, 2021 · 9 comments
Labels
bug db:sqlite Related to SQLite macros

Comments

@fabi321
Copy link

fabi321 commented May 24, 2021

minimal steps to reproduce:

struct Rowid {
    rowid: i64,
}

#[async_std::main]
async fn main() {
    let pool = sqlx::SqlitePool::connect("file::memory:").await.unwrap();
    sqlx::query!("CREATE TABLE IF NOT EXISTS test (rowid INTEGER PRIMARY KEY);")
        .execute(&pool)
        .await
        .unwrap();

    sqlx::query_as!(
        Rowid,
        "INSERT INTO test (rowid) VALUES (42) RETURNING rowid"
    ).fetch_one(&pool).await.unwrap();
}

this with a test database with the first statement executed will yield
error: unsupported type NULL of column #1 ("rowid")

sqlx version: 0.5.5
libsqlite3 version: 3.35.4 (libsqlite3-sys 0.22.2)

@abonander
Copy link
Collaborator

This is likely an issue with our bytecode analysis pass, but you can use the type override syntax which should let you bypass this error:

    sqlx::query_as!(
        Rowid,
        r#"INSERT INTO test (rowid) VALUES (42) RETURNING rowid AS "rowid: i64""#
    ).fetch_one(&pool).await.unwrap();

@abonander abonander added bug db:sqlite Related to SQLite macros labels May 25, 2021
@fabi321
Copy link
Author

fabi321 commented May 25, 2021

This works in the above example, but not my intended (and probably still valid) case:
I actually wanted to do something similar and seemingly oversimplified the problem:

async fn main() {
    let pool = sqlx::SqlitePool::connect("file::memory:").await.unwrap();
    sqlx::query!("CREATE TABLE IF NOT EXISTS test (rowid INTEGER PRIMARY KEY, name TEXT);")
        .execute(&pool)
        .await
        .unwrap();

    sqlx::query_as!(
        Rowid,
        r#"INSERT INTO test (name) VALUES ('42') RETURNING rowid as "rowid: i64""#
    ).fetch_one(&pool).await.unwrap();
}

In this case, it expects a Option<i64> although the field is a primary key.

I'm not sure if I'm missing a scenario here.

@abonander
Copy link
Collaborator

It's likely the same issue, the column is assumed to be nullable unless we can prove that it won't be and there seems to be something confusing the heuristics here; if we can't even determine the type of the column, we certainly can't determine its nullability.

@happysalada
Copy link

Thank you for the workaround, here is what I ended up doing

struct Row {
    id: Option<String>,
}

let row = sqlx::query_as!(
Row,
 r#"INSERT INTO PLANS (id, title, agent_id) VALUES (?, ?, ?) RETURNING id AS "id: String""#,
 ulid,
 new_plan.title,
 new_plan.agent_id
)
.fetch_one(&context.pool)
.await?;
Ok(row.id.expect("Failed insertion"))

You do have to return a result in the end, but it seems to me that this can indicate a failure to insert for example.

@happysalada
Copy link

One question though, with your "trick" is there a way to get back the whole struct?
I guess the best thing to do in that case is to just do another query to get the newly inserted record, correct?

@fabi321
Copy link
Author

fabi321 commented Jul 7, 2021

.await?;

The ? here catches an failed insertion.

Ok(row.id.expect("Failed insertion"))

This is basically an unreachable panic as sqlx assumes that every return value is an option until it can prove it wrong. So in this case, this field will never be an None value, but sqlx wants to be on the safe side.

@fabi321
Copy link
Author

fabi321 commented Jul 7, 2021

As by the Sqlite reference, you can return all fields of the table you're inserting into.

@happysalada
Copy link

Thank you! unfortunately, you need to create a new struct with all the fields that are options and then you can return that. So in the end, rather than creating a new structs, making a new query seems a little simpler.

Thank you for the info about the unreacheable panic!

@abonander
Copy link
Collaborator

Closed by #1323

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug db:sqlite Related to SQLite macros
Projects
None yet
Development

No branches or pull requests

3 participants