1. I've found myself running into lifetime issues with code that looked well-formed. At one point, I had the following:
sqlite::static_resultset
get_order_with_items(std::int64_t order_id) { sqlite::statement stmt = conn.prepare("SELECT ..."); return conn.execute ({order_id}); } This is undefined behavior (as stated in the docs), and needs to be rewritten as:
sqlite::static_resultset
get_order_with_items(std::int64_t order_id) { return conn.prepare("SELECT ...") .execute ({order_id}); } The problem being that both sqlite::static_resultset and sqlite::statement are implemented in terms of a sqlite3_statement*. The second snippet makes the static_resultset own the statement handle, while the first version makes it a view. This problem will probably appear much more frequently if you're using error codes instead of exceptions.
IMO the problem arises due to a mismatch between the SQLite and the Boost.Sqlite object model regarding statements and resultsets. Resultsets don't exist in the C API, and thus may become problematic in the C++ API. Making a type sometimes owning and sometimes non-owning can be misleading.
Do you think this could be solved with ref-qualified overloads, i.e. .execute() && would transfer ownership, while .execute() & would not? That would seem intuitive to me.
Isn't this what you have right now?
4. In the static interface, type mismatches are silent. If one of your field types doesn't match what your query returns, the library doesn't emit any error and silently sets the field to its default value. While this matches what the sqlite3_column_xxxx functions do, I think it'd be more helpful to emit an error. Such type mismatches are likely due to programming errors, and erroring helps to detect them soon. For instance, Boost.MySQL does this by checking metadata (you can probably use sqlite3_column_type to perform the check).
Note that this reflects sqlite's behaviour, its tables aren't strictly typed. That is, a column might be defined as INT, but a user could insert a "foobar" in it. And then it's going to be a string. While odd, that's not a programming, but user error.
I think adding a strict mode for the static_resultset would be the way to go, i.e. an opt in similar to strict tables in sqlite.
sqlite only added type checking as an option in 2021 with strict tables.
You got me in this one - I didn't know this was legal. I still think this should be an error. If you actually expect different types in a single field, an appropriate representation (like sqlite::field or a variant) should be used.
2. sqlite::transaction features a throwing destructor, which I find surprising.
That's for backwards compatibility, ROLLBACK won't fail these days, but it did in older versions. So in practice it won't throw, unless you're using some ancient sqlite instance.
If that's the case, I think it's best to just don't use a throwing destructor, and don't communicate the error anyway.
2. The virtual table infrastructure may be invoking undefined behavior by violating strict aliasing, casting sqlite3_value* into sqlite::value via a reinterpret_cast. Some conversations have happened in the Slack workspace about it, with conflicting opinions whether this is actual undefined behavior or not.
It's not, the sqlite::value class has a standard-layout class and the element it's cast from is the first in the class.
I know the cast is well-formed, but the target object doesn't technically exist (as opposed to a member subobject, which starts existing when the parent object is created). Thanks, Ruben.