feat(sqlite): add preupdate hook#3625
Conversation
d9eb625 to
5264ada
Compare
436694b to
03d9a3f
Compare
03d9a3f to
bcdb609
Compare
bcdb609 to
7f1fdd9
Compare
| pub fn get_old_column_value(&self, i: i32) -> Result<SqliteValue, Error> { | ||
| let mut p_value: *mut sqlite3_value = ptr::null_mut(); | ||
| unsafe { | ||
| let ret = sqlite3_preupdate_old(self.db, i, &mut p_value); |
There was a problem hiding this comment.
The sqlite3_value returned by this call and sqlite3_preupdate_new has weird semantics. It's a "protected" value so it's thread-safe, but at the same time the documentation also specifies:
The sqlite3_value that P points to will be destroyed when the preupdate callback returns.
We should handle this as a SqliteValueRef instead, using the same lifetime. The user can then use .to_owned() to get a fully independent value if they need it.
You'll need to add a new case here:
Line 19 in 1678b19
There was a problem hiding this comment.
This should also check i against sqlite3_preupdate_count because the result is undefined if the index is out of bounds.
There was a problem hiding this comment.
We should handle this as a SqliteValueRef instead, using the same lifetime. The user can then use .to_owned() to get a fully independent value if they need it.
SqliteValue::new calls sqlite3_value_dup which creates a copy of the sqlite3_value. I tested this out by storing the returned SqliteValue in a mutex and ensuring the returned value could still be decoded properly after the callback was completed.
I can add something to SqliteValueRef for this to avoid the call to sqlite3_value_dup, but I think that would require re-implementing a lot of the logic in SqliteValue or modifying it so that it can operate on both an owned or borrowed value. Let me know if I'm missing something.
EDIT: added this in a separate commit. I can revert it if this isn't what you had in mind.
There was a problem hiding this comment.
This should also check i against sqlite3_preupdate_count because the result is undefined if the index is out of bounds.
I think the documentation might be a bit conservative here (or they don't want to make any guarantees) because it does check for these conditions and return an error, but I went ahead and added an explicit check here too.
d1a3a97 to
c90b843
Compare
c90b843 to
8da5e6b
Compare
Adds bindings for SQLite's preupdate hook.
This is exposed as a separate feature because the system SQLite version generally does not have this flag enabled, so using it with
sqlite-unbundledmay cause linker errors.If we don't want to create a new feature flag in the main crate just for this, we could enable it by default with the
sqlite(bundled) feature only. That would make the configuration a little simpler.