New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check for multiple statements in prepare #1147
Comments
You may have false positive with some trailing comments "SELECT 1; /comments/;" |
Yeah. I think that's probably fine. |
I ran into this issue in my own code and worked up a crate to quickly split strings containing multiple sqlite statements and also to tell me when I'm dealing with multiple sql statements. It's standalone functionality, so I put it in its own crate, but I could easily turn it into a PR.
If this is useful to rusqlite, I'd be happy to turn it into a PR. |
|
I think we should take the current approach, where we used the tail returned by SQLite. This has downsides, but is much less error-prone than trying to parse it ourselves IMO. |
Yes, that's right. I thought it could be done simply, but after chasing down a bunch of edge cases and handling virtual tables, I think this "simple" bit of code will quickly get hairier than its worth. |
Would also be great if there was an equivalent method for multiple statements, like there is for |
We should be able to check at compile time that only a single statement is provided with sqlite3-parser if the SQL is a static string. |
There is a new let changes = conn.execute(single!("DELETE FROM my_table WHERE col = ?"), ...)?; which is not user-friendly... Edit: I am going to try conditional attribute-like macro => doesn't work #[inline]
#[cfg_attr(feature = "rusqlite-macros", single())]
pub fn execute<P: Params>(&self, sql: &str, params: P) -> Result<usize> { |
Consider checking by default if someone does something like
conn.prepare("<stmt 1>; <stmt 2>")
. This will silently only run stmt 1, which is probably not what the user wants.Right now, I believe we have some tail checking here, but I think it's only under extra-check feature. It also might be disabled then too. I think we should probably check this always, since I don't see a case where anybody wouldn't want this detected.
The text was updated successfully, but these errors were encountered: