Boost

boost@lists.boost.org

September 2025

47 participants
26 discussions

Re: [sqlite] Formal review of Boost.SQLite begins
by Maximilian Riemensberger 01 Sep '25

01 Sep '25

On 9/1/25 16:16, Klemens Morgenstern wrote: > > > On Sun, Aug 31, 2025 at 3:24 PM Maximilian Riemensberger <riemaxi(a)gmx.net <mailto:riemaxi@gmx.net>> wrote: > > > >> > >> * The connection type can be owning or non-owning. Why wouldn't one > >> instead have distinct connection (owning) and connection_ref (non-owning) > >> types? > >> > > Because otherwise the complexity goes up unnecessarily. A user can write a > > function just taking `connection&` and not worry about this. > > In the library I have the `transaction` type for example. It holds a > > reference to the connection - if there was a second type like > > `connection_ref`, now I would have a second type of a transaction. > > > > I see this point. However, a user that writes a function that takes `connection&` as an argument likely does not care about whether the connection is owning since the general expection would be that the connection is connected and live before the function is called and remains so until the function returns. All those functions can just take `connection_ref` by value (just like functions take a string_view or span or function_ref) and no duplication is necessary, assuming connection_ref is implicitly constructible from `connection`. > > For example, the transaction: > > class transaction { > connection_ref conn_; > > public: > explicit transaction(connection_ref conn); > > // Many more things > } > > And I would then expect both of the following to compile just fine: > > // 1 > connection c{":memory:"}; > transaction t{c}; > > // 2 > connection c{":memory:"}; > connection_ref cr{c}; > transaction t{cr}; > > and mean the same thing. > > Fair, but I might have the same piece of code that needs to guarantee that the `connection` is alive while it holds the handle. If this is part of a vtable impl (where it's non owning) that'll be the case automatically, > whereas it would take ownership otherwise. Using a connection_ref introduces an additional level of complexity, when the non-owning connection is just used in functions & vtables. I'm not sure I understand. Isn't this the same problem with the current design? If you have a `connection&`, it does not guarantee anything about the validity of the `sqlite*` since it might not be owning and whoever owns it closed it already. So I would think there are few to no functions that care about the ownership. I haven't really looked into the vtable stuff before, but since you mention it in this context, I noticed the following piece of code: ``` template<typename Table> struct module { using table_type = Table; /// @brief Creates the instance /// The instance_type gets used & managed by value, OR a pointer to a class that inherits sqlite3_vtab. /// instance_type must have a member `declaration` that returns a `const char *` for the declaration. BOOST_SQLITE_VIRTUAL result<table_type> create(sqlite::connection db, int argc, const char * const argv[]) BOOST_SQLITE_PURE; /// @brief Create a table /// The table_type gets used & managed by value, OR a pointer to a class that inherits sqlite3_vtab. /// table_type must have a member `declaration` that returns a `const char *` for the declaration. BOOST_SQLITE_VIRTUAL result<table_type> connect(sqlite::connection db, int argc, const char * const argv[]) BOOST_SQLITE_PURE; }; ``` As far as I understand, I as a user would derive from this struct and implement these two functions. Both receive a `sqlite::connection` by value. Is it owning? Does it need to be owning? What happens if I move and store the connection somewhere private? Am I actually allowed to do that, or will it break some lifetime assumptions by the library or sqlite3? Or if it is indeed owning, do I need to store it and keep it alive to avoid the database being closed and pulled away under my feet? Neither docs, tests, or examples use this parameter as far I can see. Digging throught the detail/vtable.hpp header shows it is non-owning in both cases. And now I think I understand your comment, from the implementation point of view since the vtable/module lifetime is managed by sqlite itself and thus tied to the connection on which it was registered (by the user or sqlite?) it behaves kind of like an owner even if it isn't. I still find it both confusing to the user (mostly myself) when starting to learn and understand the library. And I think there is a better way to do that, that clearly communicates ownership. Basically, similar to executor and execution_context in asio. Because if I look again at the above module class, I think it should just receive a non-owing type (like executor, connection_ref, ...) because ownership is clearly managed externally from the modules perspective and this should also be documented. By the way, this is unfortunately an issue I have run into during this review: The primary two pieces of documentation I look at are the boost_sqlite source code and the sqlite3 documentation. The boost_sqlite documentation is not that helpful beyond the simple examples. But regarding the connection type, I still think it would be much better to have a strictly owning type (like execution_context) and a strictly non-owning type (like executor) and almost exclusively use the non-owing type except for where ownership is necessary. And I'm pretty sure ownership is really only necessary on the application level when opening the database. Sorry to be a bit obnoxious about this, but when I look at the public headers and the docs only with a fairly regular C++ type design expectation, it's just confusing me. Best regards, Max

1 0