Diesel: Sqlite: There should be a way to obtain an id of last insert

[krdln]

It is a really crucial feature to be able to obtain an id an inserted object. Postgres backend allows to do that using RETURING clause via the get_result method. Sqlite exposes last_insert_rowid but it's not reexported by diesel. Without that feature, the sqlite backend is quite unusable for me.

I see a few possible solutions here:

1. Just reexport the last_insert_rowid for SqliteConnection. The connection is not Sync so it shouldn't be a problem.
2. Create a trait method for getting the id of the insert/update and implement it for all the backends. Postgres backend could just call get_result. This solution is nice as it allows writing backend-agnostic code.
3. Make the get_result work on sqlite only on structs containing just the id. I actually don't know diesel well enough to understand whether this is possible by appropriate implementations of Queryable.
4. Make the get_result work for sqlite by pulling data from the structs that are inserted. It won't work for UPDATE though. I guess that would require some architectural changes in diesel, but on the other hand, it's the simplest solution wrt porting code between backends.
5. I've just learned that you can execute the raw query SELECT last_insert_rowid();. Maybe this workarkound could be exposed somewhere in the docs (for LoadDsl or SqliteConnection)? Also, I have to admit, I don't know how how to execute raw sql query in diesel, searching for raw yields no results.

[sgrif]

You can get the result of last_insert_rowid in your application using no_arg_sql_function! and select. We may eventually try to implement get_result for inserts on SQLite, but last_insert_rowid has a ton of caveats and gotchas that we've opted to avoid for the time being. table.order(id.desc()).first(&conn) inside of a transaction makes the tradeoffs much more apparent.

[victorhqc]

@sgrif I'm sorry, I'm a bit new in the rust ecosystem and I'm not fluent in reading the documentation provided. I'm having the same problem of not being able to retrieve the last element after an insert as well.

Could you provide some example of now to call something like SELECT last_insert_rowid()? 🙏
it's not clear to me how no_arg_sql_function! is used, and then I found that it will get deprecated. So maybe an example with sql_function will be appreciated 😸

My (of course) wrong approach of this doesn't work 😅

diesel::insert_into(my_table::table).values(&new_value);
let last_id = sql_function!("SELECT last_insert_rowid()");

_Update:_
After reading a bit more thoroughly I found this nice example in the repository. It basically solves my issue, so I'll leave it here for reference 😄

[emmanueltouzery]

I hit that issue now and went with the last_insert_rowid approach, here's my code if that'll help anybody:

no_arg_sql_function!(
    last_insert_rowid,
    diesel::sql_types::Integer,
    "Represents the SQL last_insert_row() function"
);

// ...

diesel::select(last_insert_rowid)
     .get_result::<i32>(sql_conn); // <--- returns Result<i32, Error>

[vmasdani]

I hit that issue now and went with the last_insert_rowid approach, here's my code if that'll help anybody:

no_arg_sql_function!(
    last_insert_rowid,
    diesel::sql_types::Integer,
    "Represents the SQL last_insert_row() function"
);

// ...

diesel::select(last_insert_rowid)
     .get_result::<i32>(sql_conn); // <--- returns Result<i32, Error>

this helped me so much. thank you

[FelixHenninger]

Hi everyone, thanks to all of you for the workarounds you've provided!

It looks like SQLite is going to support RETURNING with the upcoming version 3.35.0; their implementation is designed to follow Postgres. In light of this change, would it be possible to revisit the issue discussed above?

I'm personally not super-familiar with diesel internals so I'm not sure what a change like this would entail (it might be as easy as adding impl SupportsReturningClause for Sqlite {}), but if this were possible it would make my life a lot easier as a user, and I would be glad to help e.g. testing and revising examples.

[weiznich]

@FelixHenninger Adding support for get_result on sqlite based on returning clauses is definitively something that seems reasonable. Contributions are welcome.

I did not look at the code yet, but adding this impl is certainly a good starting point. Quite likely that's all that is required to make this work. Additionally the corresponding tests should also be run against the sqlite backend.
The biggest piece of work is likely to make this work on our CI + find some way to make it really clear in our documentation that this feature requires at leat sqlite 3.35.

[sprhawk]

You can get the result of last_insert_rowid in your application using no_arg_sql_function! and select. We may eventually try to implement get_result for inserts on SQLite, but last_insert_rowid has a _ton_ of caveats and gotchas that we've opted to avoid for the time being. table.order(id.desc()).first(&conn) inside of a transaction makes the tradeoffs much more apparent.

according to sqlite doc, the largest is not always the last inserted rowid.

[weiznich]

@sprhawk That's exactly the reason why we do not provide a get_result() function on this basis. At least for sqlite < 3.35 there is no correct way to get the latest auto incremented id. It will always depend on your use case.

[sprhawk]

@weiznich but sqlite function last_insert_rowid will return the rowid, isn't it?

[weiznich]

@sprhawk Not necessarily, this is really tricky. Again: There is no general solution for this problem and this old issue is certainly not the right place to discuss such things.

[FelixHenninger]

Ok, thanks @weiznich for the kind encouragement! I'm very much a rust beginner, so please let me know if I'm causing more trouble than it's worth, but as noted above I would love to help get this running. I've added the line we discussed above (impl SupportsReturningClause for Sqlite {}) to diesel/src/sqlite/backend.rs, and as best as I can tell, it mostly works! 🎉

So here's what I think needs to be done (based on my very cursory understanding of your repo, apologies in advance 😬)

• You'll want the impl in your code, naturally (maybe behind some feature flag?)
• The tests concerning RETURNING for insert appear to rest on support for the DEFAULT keyword, and fail with error[E0277]: the trait bound 'Sqlite: SupportsDefaultKeyword' is not satisfied. I couldn't figure out what was going on here.
• The tests for RETURNING in update and delete should no longer skip sqlite -- they (appear to) work out of the box.
• The CI linux installation needs to be updated to SQLite 3.35 -- right now it's running 3.31. As best as I can tell, the Windows and macOS installations via chocolaty and brew are already up-to-date.
• Several compile tests are now unnecessary, I think these can be deleted (?)
  
  
  
  • delete_statement_does_not_support_returning_methods_on_sqlite.rs
  
  
  • insert_statement_does_not_support_returning_methods_on_sqlite.rs
  
  
  • update_statement_does_not_support_returning_methods_on_sqlite.rs
  
  

Alright, that's what I came up with -- if you can spare a moment, I'd love to hear your feedback: What am I missing? Could you give me a hint regarding the failing test for INSERT above? Would you like me to put together a PR for this? (which would be a starting point for docs, of course)

Many thanks again for your efforts, patience, and kind words!

[sgrif]

Please take my opinion with a grain of salt, I have not been super involved
in Diesel for a while now, and I haven't fully read all the arguments made.
Georg should make the final decision on this. I would just like to mention
that Diesel has chosen a minimum supported SQLite version in the past, and
taking advantage of returning clause support seems like it'd be useful. I
think this might be worth treating similar to how we support Rust versions
-- especially since this wouldn't involve dropping support for older
versions, just allowing code to compile and result in a runtime error.
Feature flags might also be something to consider for this.

On Sun, Mar 28, 2021 at 4:52 PM Felix Henninger @.*>
wrote:

Ok, thanks @weiznich https://github.com/weiznich for the kind
encouragement! I'm very much a rust beginner, so please let me know if I'm
causing more trouble than it's worth, but as noted above I would love to
help get this running. I've added the line we discussed above (impl
SupportsReturningClause for Sqlite {}) to diesel/src/sqlite/backend.rs,
and as best as I can tell, it mostly works! 🎉

So here's what I think needs to be done (based on my very cursory
understanding of your repo, apologies in advance 😬)

• You'll want the impl in your code, naturally (maybe behind some
  feature flag?)
• The tests concerning RETURNING for insert
  https://github.com/diesel-rs/diesel/blob/a4b8031f6a096c8ea4032e7d8f05711d7f0dea1a/diesel_tests/tests/insert.rs#L65-L116
  appear to rest on support for the DEFAULT keyword, and fail with error[E0277]:
  the trait bound 'Sqlite: SupportsDefaultKeyword' is not satisfied. I
  couldn't figure out what was going on here.
• The tests for RETURNING in update
  https://github.com/diesel-rs/diesel/blob/a4b8031f6a096c8ea4032e7d8f05711d7f0dea1a/diesel_tests/tests/update.rs#L85-L113
  and delete
  https://github.com/diesel-rs/diesel/blob/a4b8031f6a096c8ea4032e7d8f05711d7f0dea1a/diesel_tests/tests/delete.rs#L32-L44
  should no longer skip sqlite -- they (appear to) work out of the box.
• The CI linux installation needs to be updated to SQLite 3.35 --
  right now it's running 3.31
  https://github.com/diesel-rs/diesel/blob/master/.github/workflows/ci.yml#L61.
  As best as I can tell, the Windows and macOS installations via chocolaty
  and brew are already up-to-date.
• Several compile tests are now unnecessary, I think these can be
  deleted (?)
  
  
  
  • delete_statement_does_not_support_returning_methods_on_sqlite.rs
  
  
  • insert_statement_does_not_support_returning_methods_on_sqlite.rs
  
  
  • update_statement_does_not_support_returning_methods_on_sqlite.rs
  
  

Alright, that's what I came up with -- if you can spare a moment, I'd love
to hear your feedback: What am I missing? Could you give me a hint
regarding the failing test for INSERT above? Would you like me to put
together a PR for this?

Many thanks again for your efforts, patience, and kind words!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/diesel-rs/diesel/issues/771#issuecomment-808972754,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AALVMK7P4D4XKDEJAKLVZTLTF66J7ANCNFSM4DB3FJKQ
.

--

Thanks,
Siân Griffin

[sgrif]

Also, Georg is right that this belongs in a new issue. (Sorry for posting my opinion here)

[weiznich]

@sgrif @FelixHenninger https://github.com/diesel-rs/diesel/discussions/2684 Would be a better place to discuss the implementation of this feature.