On Thu, Nov 14, 2024 at 8:19 PM Дмитрий Архипов via Boost <boost@lists.boost.org> wrote:
ср, 13 нояб. 2024 г. в 15:31, Richard Hodges via Boost <boost@lists.boost.org>:
The Boost formal review of the Boost SQLITE library starts *TODAY*
I have a few questions
1) I do not understand the boost_sqlite vs. boost_sqlite_ext situation. Should I always use the second one if I want to create an extension? If I want to both create an extension and use SQLite from the same binary, should I link to both?
If you create an extension, you'll need to call into the database differently using the indirections in sqlite3ext.h. In order to avoid conflicts, this library puts everything you include into an inline namespace (sqlite::ext), to avoid linker issues. That way you get access to the host db with the same API, but the internal calls are different, so you'll need to link to sqlite3-ext. You can then use a regular sqlite3 library from the same binary, but not the same TU. If you accidentally include boost/sqlite in both modes, but only link one you'll get linker errors.
2) The library can deduce struct composition in C++20. Have you considered using Boost.PFR to also be able to do it in C++14?
I don't exactly know what you mean here. You can use pfr in C++20 for a static_result_set (and describe for earlier standards). struct author { std::string last_name; std::string first_name; }; for (auto author : conn.query<author>("select first_name, last_name from author"));
3) Regarding custom functions.Have you considered using multiple arguments for the callback rather than span<value, N>? I have overall weird feelings about the aggregate function API. When the passed callable is of a class type it should have particularly named member functions. That gives the impression that the state that those member functions use is supposed to be stored in the callable object. But instead it should be stored in the object of the type of the first parameter, and the callable object itself is used as a sort of database-local global. As I've said this is kind of confusing. Also, SQLite allows overloading based on the number of arguments. The current interface requires having a separate callable type for each such overload, because the special members cannot be overloaded or be templates, otherwise the deduction would not work. There also doesn't seem to be a way to remove functions.
I don't entirely disagree, but this behaviour is directly inherited from sqlite3. That is: every overload of a function has a state like this. And the most likely use is that you can pass state into the function Sqlite also guarantees that final will be called whenever the function is used, so this state can be used to share data between the invocations of step.
I am thinking of an interface like this:
template <std::size_t N = boost::dynamic_extent, class F, class Ctx = std::nullptr_t> void create_aggregate_function(F f, Ctx ctx = Ctx{})
And then the callables that need the semi-global context would be called as f.step(ctx, value_0, ... value_n), and if they do not need that context, they'd be called as f.step(value_0, ... value_n).
I don't think I fully understand. What would `f`/`this` be pointing to when `step` gets called?
4) (not a question) You use core::string_view in some APIs. Due to a (very unfortunate IMO) decision by the community that type is not public. So, instead you need to create an alias in your library and reference that alias in the docs. You can document the alias to have a compatible API to std::string_view.
Unfortunate, indeed.