Skip to content
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

execute() may return stale number of changed rows #565

Closed
fzgregor opened this issue Aug 26, 2019 · 1 comment · Fixed by #567
Closed

execute() may return stale number of changed rows #565

fzgregor opened this issue Aug 26, 2019 · 1 comment · Fixed by #567

Comments

@fzgregor
Copy link

fzgregor commented Aug 26, 2019

Using execute() with a select query returns the number of rows changed in the last database altering query. It would be better to return 0.

I have a select query for which I'm only interested in the number of returned rows. https://docs.rs/diesel/1.4.2/diesel/query_dsl/trait.RunQueryDsl.html#method.execute only spoke about returning the number of "affected rows". Though the description still made me wonder whether it is the correct function to use. After testing, everything seemed to work as expected (due to the above issue of returning a stale number of changed rows from a previous insert). Now I spent way too long to figure out what was wrong with the application.

rusqlite could use https://www.sqlite.org/c3ref/total_changes.html before and after the query instead of sqlite3_changes to prevent the return of a stale number of affected rows.

@gwenn
Copy link
Collaborator

gwenn commented Aug 26, 2019

We can check that sqlite3_column_count returns 0. But it doesn't work with some PRAGMA.
We can also check that sqlite3_stmt_readonly returns false.
There is already a check here. But it does not work if your SELECT returns nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants