Rows cannot implement Drop about rusqlite HOT 10 CLOSED

See gwenn@d8d220e

from rusqlite.

Ick. Great catch. I would like for that to not compile, but AFAICT an Iterator impl can't return references to itself from next, which is what I believe we'd need.

Options:

• Leave it as is and document it. This is related to the already-documented issue that if you call next twice, you'll get errors trying to access the first row which is now stale.
• Don't reset when Rows is dropped, and go back to reset_if_needed. We can keep the reset on error / SQLITE_DONE to still reset ASAP when possible.

Thoughts? Other options?

from rusqlite.

Another option is to deprecate/remove query. We already recommend people use query_map and/or query_and_then instead, neither of which expose this problem.

from rusqlite.

Could you please confirm that the problem is related to streaming iterators ?

from rusqlite.

Yes, that's the exact problem.

from rusqlite.

I think #153 is a decent compromise. Keep Rows but change next() to return a Row<'a, 'stmt> where 'a is tied to the vending Rows handle. This means Rows no longer implements Iterator, but it's at least still usable, and this change doesn't affect anyone using query_map/query_and_then.

from rusqlite.

This change breaks the current API (when users iterate on Rows), no ?

from rusqlite.

It does, yeah.

We're still in 0.x versioning, so breaking changes are allowed between minor versions. I was okay with runtime checks to let Rows implement Iterator when the only thing that could go wrong was if you used a row after calling next() again, but adding more runtime checks (or disabling resetting the statement when a Rows is dropped) seems bad.

from rusqlite.

Ok.

from rusqlite.

Closed with #153.

from rusqlite.