Feat: Add(sqlite)#147
Conversation
| .or_trap("lunatic::sqlite::execute")? | ||
| .lock() | ||
| .or_trap("lunatic::sqlite::execute")? | ||
| .execute(exec) |
There was a problem hiding this comment.
It would be nice to return here the error to the guest, instead of just trapping.
| data_ptr: u32, | ||
| data_len: u32, |
There was a problem hiding this comment.
Instead of query taking the data_*, I think it would be better if it returned just the length and buffered the data. There could be another function (query_result?) that just copies the data into the guest.
That way the guest has a chance to allocate the right space and doesn't need to hope that the data is going to fit into a pre-allocated buffer.
There was a problem hiding this comment.
I was originally for this idea. We can talk about it on Monday for sure.
| .or_trap("lunatic::sqlite::query_prepare::read_binary")?; | ||
|
|
||
| let len = bytes.len(); | ||
| let mut result = vec![ColumnType::Binary as u8]; |
There was a problem hiding this comment.
I think this also needs to be .to_le_bytes encoded.
| let mut result = vec![ColumnType::String as u8]; | ||
| result.append(&mut (len as u32).to_le_bytes().to_vec()); | ||
| result.append(&mut bytes.as_bytes().to_vec()); | ||
| result |
There was a problem hiding this comment.
Instead of appending to a temporary vector (result), we could here directly append to return_value.
| return_value.append(&mut bytes); | ||
| } | ||
|
|
||
| return_value.push(ColumnType::NewRow as u8); |
There was a problem hiding this comment.
This is going to leave a trailing NewRow marker, even there are no new rows. This is not a big issue, but the parser needs to be aware of it.
| let memory = get_memory(&mut caller)?; | ||
| let (memory_slice, state) = memory.data_and_store_mut(&mut caller); | ||
|
|
||
| // get the vevtor |
|
This approach looks good to me. Thanks @jtenner for pushing this effort forward! I'm not that familiar with SQLite and am curious how useful features like "named parameters" are? At the moment we don't support something like: let mut statement = connection.prepare("SELECT * FROM users WHERE name = :name")?;
statement.bind_by_name(":name", "Bob")?;Should this also be part of the interface, or are people just working around this? I assume that doing manual escaping of user input without it would be an error-prone process. Should we include it in our API? I'm also wondering about the security implications of the On a more future-proof note, tools like https://litestream.io that work together with SQLite often use the WAL (write ahead log) feature. This is also recommended for all production deployments of SQLite. If I'm not mistaken, this also allows for multithreaded writes. Should we maybe use it by default? This would allow for connections to be shared between processes running concurrently. |
|
Absolutely. I haven't followed along with this PR, but after looking into it, it seems like it just adds host functions and stores state? I actually wrote some host functions for using postgres for a personal project, and I'm planning to transition that code into a plugin too. |
|
Closed in favor of #160 |
4 participants
This pull request aims to add sqlite capabilities to the lunatic host. Any comments and help would be appreciated!