Re: [review] [sqlite]

From: Дмитрий Архипов (grisumbras_at_[hidden])
Date: 2024-11-14 12:18:36

ср, 13 нояб. 2024 г. в 15:31, Richard Hodges via Boost <boost_at_[hidden]>:
> 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?

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?

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 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).

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.