Expose last insert row id from database_binder

[bjoernpollex]

Since database_binder::chain_type is essentially a prepared statement, it would be useful to have access to the last insert row id from such an object, for instance when storing it on an object that is used to write data.

[Killili]

As this is saved on the DB Connection not the statement, this would make it necessary to call last_insert_row after every statement execution and cache its value. Because we dont know what querys will actually insert something. And we dont know when the user decides to use this value. I think thats some overhead we should avoid.

[bjoernpollex]

But database_binder has a reference to database, so couldn't it just have a member function like this:

sqlite_int64 last_insert_row_id() const {
    return db.last_insert_row_id();
}

This would not add any overhead, because the caller would explicitly call it when needed.

[Killili]

Nope, consider having 2 querys A and B.

A is run -> last_insert_row = 5 B is run -> last_insert_row = 8 A is asked for last_insert_row and returns 8 because it asks the DB con both query's run on.

[bjoernpollex]

Ah, now I understand. Here's an alternative proposal. You could provide an alternative to the void execute() function that returns the row id (e.g. sqlite_int64 update()). Do you think this would be doable?

[Killili]

Actually thinking about my example got me worried about some multi threaded stuff i use. There it could happen that thread1 has inserted something while thread2 was getting his last_insert_row. I think we need to support some atomic way to execute and return the last_insert_row_id. Something like you proposed but with a bit different intention.

[falemagn]

Sqlite docs explicitly state that in case of multiple threads doing inserts on the same connection the result of last_insert_row_id() is unpredictable, so the user is already warned of the consequences, or am I missing something?

[Killili]

You're totally right, but if this is something i did never think about maybe some other poor people didn't, too. So it would be a nice thing to have. And i need it now that i'm aware of a potential bug in my stuff.

[aminroosta]

I was reading discussions on sqlite api. this and this

It seems to me that there is no safe way of using last_insert_rowid() with multiple db connections. Whats the point of warning users if they can't use this function in concurrent applications?

I kept reading the comments and found a workaround that's an acceptable solution to me. We can use transactions to ensure that the correct values are returned even with competing threads.

We can add this sample code to Readme and recommend it as an acceptable solution.

int row_id = 0;
db << "begin;"
      "insert into my_table (filed) values (1);"
      "select last_insert_rowid();"
      "commit;"  >>  row_id; 

And the same thing for sqlite3_changes() function which has the same problem.

int changes = 0;
db << "begin;"
      "update my_table set age=? where name=?"
      "select changes();"
      "commit;"  << 18 << "bob"
      >>  changes; 

What do you think?

[Killili]

This will work but i dont like it. Because you have to alter you're querys. And this runs the SQL parser and interpreter for the second query thats then calling last_insert_row() like we could do without the overhead. But i see that threading and SQLite is a complex thing because of the compile time selection of the threading model. I'm not sure that there is a right way to have this.

My solution so far: I have a method database_binder::execute_and_get_last_insert_row() It does the same thing sqlite3_exec() makes (in mode 1 and 2) it gets the connection mutex locks it executes the whole query (this is the reason why youre the SQL approche works) and and instead of releasing it, it the gets the last_row_id() and releases the mutex then.

This works but only in one threading mode "multithreaded" "mode = 1" which is "serealized execution of querys" because thats when there are mutexs. In the two other modes stuff is either thread local (but not the last_insert_row() facepalm) or no thread safety at all.

So i guess mode=1 will be the most used one, but we cant make everyone happy without adding our own mutex to the db connection. And i guess thats something people would not like given the overhead of 2 locks protecting the same code section if the mode is 1.

[bjoernpollex]

The way I see this library it is a modern, thin wrapper around SQLite APIs. Since these APIs suffer from that multi-threading issue, I don't see why this wrapper should try to fix it. I think it is perfectly acceptable to tell users that this only works in a specific mode.

[polesapart]

I agree with @bjoernpollex on the matter, and database_binder::execute_and_get_last_insert_row() seems enough, provided the documentation is clear on the limitations. There could be a debug build switch or at least a separate test case file that checks at runtime if sqlite is using the appropriate thread support and if it's enabled by default, and bark otherwise.