SQLite: Deserialize database from in-memory buffer #3792
Conversation
diesel/src/sqlite/connection/mod.rs
Outdated
| let data_ptr = data.as_ptr() as *mut u8; | ||
| let data_len = data.len() as i64; | ||
|
|
||
| unsafe { |
There was a problem hiding this comment.
This unsafe block should be in sqlite/connection/raw.rs instead. So we don't need to allow unsafe_code in the main connection module as well.
diesel/src/sqlite/connection/mod.rs
Outdated
| /// | ||
| /// # Errors | ||
| /// | ||
| /// This function will return `Err` if the deserialization fails. |
There was a problem hiding this comment.
I would prefer having a example on this function, even if is "only" a no_run example.
| @@ -406,6 +407,41 @@ impl SqliteConnection { | |||
| .register_collation_function(collation_name, collation) | |||
| } | |||
|
|
|||
| /// Deserialize an SQLite database from a byte buffer. | |||
| /// | |||
| /// This function takes a byte slice and attempts to deserialize it into a SQLite database. | |||
There was a problem hiding this comment.
The documentation needs to mention that it will open the database in read only mode. It might be even a good idea to change the method name to deserialize_read_only_database_from_buffer()
diesel/src/sqlite/connection/mod.rs
Outdated
| #[test] | ||
| fn database_deserializes_successfully() { | ||
| let connection = &mut SqliteConnection::establish(":memory:").unwrap(); | ||
| let data = include_bytes!("sqlite_test.db"); |
There was a problem hiding this comment.
It would be nice to not include an actual database in the git repository. We could just add support for sqlite3_serialize and do a roundtrip here instead (so first serialize then deserialize).
… round-trip, and move the unsafe code into raw.rs
Thanks very much for the guidance. I believe I implemented all of your suggestions - if you could take a look again and provide some further feedback that would be great :) |
There was a problem hiding this comment.
Thanks for the update. It now looks mostly good. I would like to see a small comment on one of the unsafe blocks for some invariant that needs to be upheld. The Deref<Target = [u8]> impl might be nice, but that's not strictly required for merging.
In addition it might be good to add an entry to the Changlog.md file for this feature.
| } | ||
|
|
||
| /// Returns a slice of the serialized database. | ||
| pub fn as_slice(&self) -> &[u8] { |
There was a problem hiding this comment.
I would like to see an additional comment on this unsafe block that the pointer is never null because we don't pass the NO_COPY flag.
| /// `SerializedDatabase` is a wrapper for a serialized database that is dynamically allocated by calling `sqlite3_serialize`. | ||
| /// This RAII wrapper is necessary to deallocate the memory when it goes out of scope with `sqlite3_free`. | ||
| #[derive(Debug)] | ||
| pub struct SerializedDatabase { |
There was a problem hiding this comment.
I think it might be meaningful to implement Deref<Target = [u8]> for this wrapper type so that it can be used as &[u8] directly.
I am creating a project that reads data from an sqlite database. I am simply using sqlite as a storage medium for a large amount of static, readonly data that my application needs to query.
For this type of use-case, it can be advantageous to embed the database file directly into the executable with the include_bytes! macro. Then the byte buffer can be deserialized into an in-memory sqlite database. This approach means the program can be distributed more easily as a single executable.
This PR adds support for doing that, using the "sqlite_deserialize" function from the sqlite C library.