Diesel crashes but should return db error

[Boscop]

In sqlite FTS, searching for a string that is empty or contains only punctuation returns an error in the sqlite3 cli like so:

sqlite> select * from midi_files where id in (select rowid from midi_files_fts('!'));
Error: fts5: syntax error near "!"

Btw, it works when the string contains alphanumeric chars:

sqlite> select * from midi_files where id in (select rowid from midi_files_fts('colou*'));
74964|Bud\Colour_Of_My_Love-Celine.Dion.mid|22996
78136|Siem\sash_feat_dr_alban_-_colour_the_world.mid|55853
78442|Thai\darius_colourblind.mid|47976

When I use this code

let query = sql::<midi_files::SqlType>("select * from midi_files where id in (select rowid from midi_files_fts(?))").bind::<Text, _>(s);
query.get_results::<Self>(&*db::conn())?

if the string is empty or only contains punctuation, instead of returning a db error, diesel crashes:

thread 'main' panicked at 'SQL error or missing database', C:\Users\me\.cargo\re
gistry\src\github.com-1ecc6299db9ec823\diesel-0.15.2\src\sqlite\connection\stmt.
rs:126:21
stack backtrace:
thread 'ui' panicked at 'cannot access stderr during shutdown', error: process d

I think diesel should return an error instead of crashing.

[killercup]

Thanks for the report! The error you quoted comes from this function:

diesel/diesel/src/sqlite/connection/stmt.rs

Lines 122 to 128 in bb6a35b

	fn step(&mut self) -> Option<SqliteRow> {
	match unsafe { ffi::sqlite3_step(self.inner_statement) } {
	ffi::SQLITE_DONE => None,
	ffi::SQLITE_ROW => Some(SqliteRow::new(self.inner_statement)),
	error => panic!("{}", super::error_message(error)),
	}
	}

I agree, panic does see very harsh here and I too would expect a Result here. I twas introduced in 17a3034, the first commit to add SQLite support. Is there any scenario where the sqlite connection is essentially irreparably broken when such an error occurs? I.e., can we always return Err in the last match arm?

[Boscop]

I'm not an sqlite expert but I tested it in the REPL and I don't see a reason why the connection wouldn't be perfectly usable afterwards.

[sgrif]

The main question isn't whether the connection is useable, but whether the statement itself is useable. I suspect we need to remove that statement from the cache if an error occurs here.

[sgrif]

This no longer panics as of 1.0.0-rc1. I still have concerns about whether the statement cache is poisoned by this case or not. If anyone is able to create a repro script that shows a poisoned prepared statement from this, I would appreciate an issue report.