On Wed, Nov 13, 2024 at 4:31 AM Richard Hodges via Boost < boost@lists.boost.org> wrote:

The library is authored by Klemens Morgenstern (@klemens-morgenstern in the CppLang slack).

This review is already off to a poor start. A valid complaint from the last review is that discussion took place off-list, on Slack. I suggested that future announcements might remind reviewers to have discussions here instead of elsewhere. I see no such reminder here, and instead I see you are making it easier to contact the library submitter on Slack. At least you didn't also provide the link to the workspace invitation page (which is https://cpp.al/slack) Here are the things being said in the #boost channel regarding this library submission: "404 not found in review email link https://anarthal.github.io/mysql/index.html" "is the reference doc supposed to be complete? E.g. https://klemens.dev/sqlite/structboost_1_1sqlite_1_1connection.html is just mostly empty, except for decorations. For what is worth, I wanted to know how to open the DB read-only, or using URIs, and was thus looking at the doc, and I find none." "these docs are so complete one gets tired of completeness when reading them" "Should these questions be asked on the list instead of here?" "dunno. I'm asking a simple question so far, on expectations I should have" "Expect what you would expect from a complete Boost library." "I was expecting at least the signatures of functions (Ctors in this instance), at least. thus I'm suspecting a ref doc (doxygen based?) generation issue, and was inquiring" "Fwiw, I don't think it's inappropriate to ask about the docs like that here because yeah, that seems kind of just like a simple configuration error I'm guessing" "these are good things to ask on the list and you should state your documentation expectations on the list" "I intend to embed the sources in my code base, instead of compiling Boost. Will Boost.SQLite be compatible with my existing Boost 1.84, for its dependencies (if any)?" "darn, all the classes docs aren't there." "explicit connection(handle_type handle, bool take_ownership = true) that reads inscrutably at the call site. I've taken the habit of using e.g. enum class HasOwnership : bool; inline constexpr HasOwnership cTakeOwnership{ true }; ... so the call site reads well. Bad idea? Is that something you'd consider?" "I think these are great topics for the mailing list" "I confess I was for the ML as well, but in practice, for small things like that, I prefer the interaction here..." "regarding the enum-class-bool idiom, I guess there's .take_ownership = true to make it readable, but C++ doesn't have that for args, does it? Only starting with C++20 for structs, no?" "Well I don't know, introducing a type for a single parameter seems a bit overkill. You usually onl nee the take_ownership = false from plugin code" "I see. In a similar vein, you've decided not to make flags type-safe? E.g. int flags=SQLITE_OPEN_READWRITE|SQLITE_OPEN_CREATE" "right, because the sqlite flags are good enough. i didn't want to wrap everything and duplicate a bunch of code for little gain. the idea is more that the library is an extension of sqlite into C++, not a 100% wrapper" "What about opening a connection with a custom VFS? I've seen that in our code-base. Did you get into that? Both allowing using a custom VFS, and wrappers for VFSs similar to your vtable one?" "nope. I thought about it but couldn't come up with an example that was contrived. I might add it later if someone comes with a use-case" "The designated initializer doesn't work." "Requiring people to move all discussions to the ML is unnecessary friction that benefit a few at the inconvenience of many." "The point is to create an auditable paper trail so people can see why a library was accepted But for something like this where it's: hey, I think you messed up a path, it's not v. pertinent to the quality of the library being reviewed" "I think its very pertinent. The review evaluates the author as much as the library if not more so, since basically Boost is getting married to the author. A consistent pattern of paying great attention to small details such as paths is meaningful." "docs are updated" "The fact is that the previous review was stained by significant discussion taking place off-list. This was discussed after the fact, and now we are repeating this behavior. I agree there is less friction on Slack but I also like having robust reviews of the kind that can be referred to in the future " "Q&A should only be on the list if it's of value to other list members. if a reviewer asks questions for his own education so that he can determine whether to accept the library, these questions do not have to be on the list, although it can be useful for them to be if other reviewers are likely to have the same questions" "connection::prepare has two overloads, and I supposed the 1-arg one throws? The doc doesn't say. (it's kinda obvious, given the 3-args one, but still). Or perhaps there's a global mention in the doc somewhere about the throw-vs-nothrow overloads? (didn't read the manual yet)" "interactive Q&A here is less intimidating than writing to the ML, and more "instant", especially for small questions like mine so far." "nothrow for those overloads is not entirely correct, because the error-code overloads could still throw things like ENOMEM. it basically means that the failure of the main operation is not thrown" Should these discussions stay on Slack or should they be brought to the list? Thanks

vtable.hpp contains the following piece of code: auto r = mod.insert(value{argv[1]}, boost::span<value>{reinterpret_cast(argv + 2), static_caststd::size_t(argc - 2)}, sqlite3_vtab_on_conflict(db)); Where argv is a sqlite3_value **, and sqlite3::value is a wrapper class roughly looking like this: struct value { // More member functions explicit value(sqlite3_value * value_) noexcept : value_(value_) {} private: sqlite3_value * value_ = nullptr; }; I had the impression that such a cast is UB (although it seems to work and seems to be accepted by ubsan). Could someone with more knowledge about the language point out whether this is allowed or not (and why) Thanks, Ruben.

**Boost.Sqlite review** It is more than ten years ago that I did a Boost review, but seeing Boost.Sqlite entering the stage I thought it good to evaluate it, because I’m a sqlite fan and used some other wrappers in the past. My compliments to Klemens to propose this awesome library. Below you will see that I vote for acceptance. Also I will answer the suggested questions, but first I’ll write my findings here. **Installation** I used the CMake flavour. That didn’t go smoothly, among others because Doxygen is required and not in my standard path, and because I’ve two or more locations with Boost. Anyway, that all is personal and not really relevant because in the end it will be shipped with Boost. And I managed to solve it quickly by using an adapted CMake file. **Documentation** I started by going through the documentation. The documentation looks good and the left side menu is convenient. However, it is a bit concise and goes too early into the tiny details, while skipping the happy path workflow. Describing the sqlite connection first is fine of course. But then it inserts 4 rows immediately, goes to a transaction where rows are inserted differently by an unnecessary construction and then goes to a custom aggregate option right away, in the first query - without having presented a “normal” query…. It looks more like a showcase than a quick start. It would IMO be good to split it into a Tutorial part and a Functionality (or so) part (the showcase part). This could also be an Advanced section in the tutorial. Please remove the names of Boost Authors from the documentation, and all unit tests, as this might raise eyebrows. It is better to take a more neutral example (Chinook is often used for sample databases). **Step 1** My step 1 would probably be a candidate for a slow-paced introductory start for a tutorial - going through the standard options slowly. Just by making a connection and doing some basic inserts and queries in different ways. All I tried worked, either immediately, or by trial and error or consulting source code Creating a connection Inserting Using indexed parameters Using named parameters Entering fields one by one (not documented! Please add it!) Either using a transaction, or without Querying Simple using row.at Prepared with a named parameter (not documented for queries! Please add it!) With tuples (awesome!) With a struct in C++ (this is really cool!) (it would also be cool if a struct could be inserted!) My code is attached. **Step 2** In my step 2 I used a blob, trying to insert an image and get it back. Using blobs should go into the documentation as well, not just in the reference part, but how to insert them, how to get them, what is the difference between a blob_view and a blob / when to use what, … Anyway, it’s simple. The next line is all needed (where get_blob is a local lambda reading it from a file). conn.prepare("insert into italy (regio_name, image) values (?1, ?2)").execute({"Veneto", get_blob("veneto.png")}); This “normal” workflow works conveniently and correctly, I can insert the image, get it back, and using an SQLite viewer in VS Code I can see the inserted image there. Cool. Then I try to use the tuple approach (fields one by one). This does not work here. I’m not sure if it’s not supported, or the way I do it is wrong. The main issue is that a blob is not constructible without parameters. Therefore you need to enter it where you declare it. See below. After that I query for the blob. Walking manually as per documentation, this works and I get the blobs using row.at(2u).get_blob() But using the query_result (described query), it does not work. According to the documentation, it should work, blob and blob_view are explicitly mentioned there in the bullet list. But a compiler error stops me here… “note: default constructor of 'query_result' is implicitly deleted because field 'image' has no default constructor boost::sqlite::blob_view image;” Maybe I need tag_invoke, also listed in the compiler errors. But the documentation does not mention it for that purpose. I didn’t try it. So I now have: struct query_result { sqlite_int64 id; boost::sqlite::string_view regio_name; boost::sqlite::string_view image; // COMPILES, but wrong result // boost::sqlite::blob_view image; // DOES NOT COMPILE // boost::sqlite::blob image; // DOES NOT COMPILE // boost::sqlite::value image; // DOES NOT COMPILE }; Anyway, that was the second option. The first option works correctly so I can do what I want to do. **Step 3** In this step I evaluated column meta information, and I added a custom SQL function (cool!). For this effort I used an existing database, a geopackage, which is a geospatial database using an sqlite3 database as its storage medium. One of its columns is typed geometry, which is not listed in this sqlite documentation (that’s OK), but apparently it can be handled as a blob by the library. Awesome. Because of this geometry-typed column, I verified the column meta information (please add to the quick start how to do these things! Neither is it in the reference). const auto meta = table_column_meta_data(conn, "limits_IT_regions", "geom"); std::cout << "Column: " << meta.data_type << " " << meta.collation << " " << meta.not_null << " " << meta.primary_key << " " << meta.auto_increment << std::endl; This works easily, all suggested by copilot, and gives me: Column: GEOMETRY BINARY 0 0 0 By the way, how can I get table names, and the whole schema? (Didn’t look it up). Then I added a custom sql scalar function my_blob_size boost::sqlite::create_scalar_function(conn, "my_blob_size", [](boost::sqlite::contextstd::size_t ctx, boost::span args) -> std::size_t { return args[0].get_blob().size(); }); This is awesome functionality and works simple and as expected. My query is now select fid,geom,my_blob_size(geom) as blob_size,reg_name from limits_IT_regions And it gives, obviously, the size of the blob. The method get_blob returns a blob_view, maybe this might need a renaming. As mentioned before, creating a function is not documented in the quick start (it starts with aggregate right away - but this simpler scalar function with a lambda misses). The reference has an example how to create it - but a small example how to use it would be nice. **Implementation** I was very happy to evaluate the library from a user perspective in some simple steps. I only glanced through the implementation, stepping through it with the debugger. It looks clear, and like a small-enough wrapper around the sqlite code. Also, I had to look some things up because they were not documented. **Typos** thos associated → those “// a case insensitive string omparison” → comparison The statemens need to be kept alive. → statements (various places) This can be a map, a vector or a stuple → tuple for the exception less overload. → “exceptionless” or “overload with error code” (it’s actually clear enough without) “The following types are allowed in a static query result:” → static_result_set It's interfaces -> Its interfaces or include boost/sqlite/extensions.hpp first → called `extension.hpp` auto r = q.current();’’ → the two quotes should be removed sqlite::create_function(conn, "my_sum",, → the double comma should go (In source): Perform a query without parametert, It execute a multiple statement. → parameter, executes (probably meant here is: “It can execute multiple statements”) The handle is shared between the statement & resultset. The statemens need to be kept alive. → statements (as mentioned above) and the & looks to drafty. The last two typos indicate that the sample code is not compiled! That would be very welcome. I noticed some other errors as well. **Errors in documentation** 1: sqlite::connection conn{":memory:"}; { sqlite::connection read{"./read_only_db.db", SQLITE_READONLY}; backup(read, target); } → it doesn’t use conn - it doesn’t define target, I don’t think it’s right 2: sqlite::create_function(conn, "win_char_counter", aggregate_func{}); It’s incorrect, because right above, the window_func is defined. Also it’s not called create_function, it’s called create_window_function 3: sqlite::create_function( conn, "to_upper", [](sqlite::context<> ctx, boost::span args) -> std::string { std::string res; auto txt = val[0].get_text(); res.resize(txt.size()); std::transform(txt.begin(), txt.end(), res.begin(), [](char c){return std::toupper(c);}); return value; }); args → val (or vice versa) return value → return res; ::create_function → create_scalar_function This clearly indicates that the documentation samples should be compiled, automatically (as done for example in Boost.Geometry). **Unclarity in documentation** 1: The documentation should make clear that these lines struct query_result { std::string first_name, lib_name;}; BOOST_DESCRIBE_STRUCT(query_result, (), (first_name, lib_name)); // this can be omitted with C++20. should be declared OUTSIDE the scope where the next lines go. It does not compile, the way it is presented in the documentation. 2: Samples how to use JSON. Do we really need it in this library? Isn’t it an extension that can be documented, but not included? That would also avoid a dependency. 3: Samples how to use BLOB I had to inspect the unit test for it What is open_blob ? 4: About ”supports variants & json”(mentioned in the library comparison) → Where is the variant in the documentation? How to do this? What does it? → Isn’t tuple a better argument for this comparison? 5: The tag_invoke should be explained better. Why do we need it? How do we define it? A sample would be very welcome. I ignored it further, so maybe I don’t need it. Or did I need it for described structures with blobs? **Other topics** Documentation: I think the library comparison can be omitted in the end, but during the review phase, it is fine. CPP code lines: 614. HPP code lines: 4179. If it is like this, can’t we make it header only? On the other hand - many structures are not templated. Shouldn’t their implementations then be moved to the sources? There is some unused functionality, for example bool glob. I can’t find any usage or mentioning of it Because there is a bind implementation anyway - can that be exposed? I think it is common practice to bind parameter by parameter, at least I missed it here (therefore I concentrated on the tuple-way, which is a nice alternative). **Other answers**

Will the library bring additional out-of-the-box utility to Boost? Yes, sqlite is tremendously popular and used by nearly every programmer. It might become one of the most used Boost libraries.

- What is your evaluation of the implementation? I only glanced through the implementation. It looks clean. I did the review mostly from a user perspective. I judged the API which looks very simple, still powerful, and useful to me. - What is your evaluation of the documentation? The layout looks very good. The typesetting is very good. There are some typos, errors and there are some improvements possible, as described above. - Will the choice of API abstraction model ease the development of software that must talk to a SQLITE database? For sure it will make it much easier. - Are there any immediate improvements that could be made after acceptance, if acceptance should happen? The errors in the documentation should be fixed. I also encountered some unclarities in behaviour, that might be addressed, either by fixing them, or by pointing me out what I did wrong and/or improving the documentation. - Did you try to use the library? With which compiler(s)? Did you have any problems? I made 3 small test programs, all attached, on MacOS using clang, with C++14 and C++20. There were no real problems. Details are described above. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent more than a full day on this review. - Are you knowledgeable about the problem domain? Yes, I know SQL databases and I have used sqlite a lot. Also I normally use geospatial databases a lot. As also tried in my review research, step 3. And I’m a Boost author (Boost.Geometry).

Please provide in your review information you think is valuable to explain your choice to ACCEPT or REJECT including SQLITE as a Boost library. Please be explicit about your decision (ACCEPT or REJECT).

**Summary** Certainly I ACCEPT the library. Boost.Sqlite is a great addition to Boost, it is easy to use, it is powerful, the API makes sense, and it looks better to me than any other SQL API I used before (but I didn’t compare it during this review). There are some errors and inconveniences in the documentation, but it is relatively minor and I expect that this will all be handled. The library is already very useful right away, as is. This acceptance is therefore UNCONDITIONAL. Thanks for submitting the library, and thanks for managing the review. Kind regards, Barend Gehrels

What's the best way to specify a nullable field in the static interface? std::optional<T> and boost::optional<T> don't seem to work, nor does having a sqlite::field member. Thanks, Ruben.

Hi all, This is my review of the proposed Boost.Sqlite. First of all, thanks Klemens for submitting the library, and Richard for managing the review.

- Will the library bring additional out-of-the-box utility to Boost?

I think it will. SQLite is very popular, and many people will likely benefit from the library.

- Did you try to use the library? With which compiler(s)? Did you have any problems?

I had two use cases in mind: regular SQL use, and virtual tables as a way to expose in-memory data structures. I've implemented both. My experiences with them follow. To try regular SQL use, I adapted an existing example I used to have in Boost.MySQL. It simulates an order management system for an online store, with a command-line interface. It's intentionally simplistic. I know that SQLite is more targeted towards resource-constrained systems, rather than web applications, but it's been enough to have a taste of how using the API looks like. For reference, the resulting code is here: https://gist.github.com/anarthal/40c018cc4d133d0c9a082814d99d2a7a I've used connection::prepare, connection::query, connection::execute, statement, static_resultset and transaction. I've found the API mostly satisfactory. Some gotchas: 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. As a quick idea (I haven't implemented it), you may have a reader class, with a similar interface to the current resultset, that allows iterating over the rows, but never takes ownership of the statement. For example: auto stmt = conn.prepare("SELECT ..."); stmt.execute({order_id}); for (const order& ord: stmt.reader<order>()) { // } You can probably make it a one-liner if you make execute return a reference to the statement. The main point would be making statement always owning, and reader/resultset always a view. 2. I've found it impossible to use the static interface with fields that may be NULL. I've tried the following: a. Using std::optional<T> and boost::optional<T> - these are not implemented yet. b. Using sqlite::value is documented to work, but doesn't. The author has stated that this is a bug. c. Using sqlite::field doesn't work either. I've hot-patched the library to include the relevant tag_invoke overload to make sqlite::field work. The tag_invoke mechanism looks like a private interface, so as a regular user I would have not been able to make use of it. Nullable fields are very common, and should likely be supported by the static interface. 3. I tried using std::int64_t for my 64-bit integer fields and it didn't work. You need to use sqlite3_int64. In my machine, sqlite3_int64 is defined as a long long, while std::int64_t is defined to be a long. I found this annoying, since it makes database implementation types leak into business logic types. The compiler errors when using the static interface are not as informative as I'd have liked. When using a struct with an unsupported type, it'd be very helpful if the offending type name appeared first in the compiler message - diagnosing the sqlite3_int64 problem would have been much easier. For example, these are the first lines that get emitted when trying to use a struct with a wchar_t field (unsupported) in the Boost.MySQL static interface: /opt/boost-1.87.0-b1-rc2/include/boost/mysql/detail/typing/row_traits.hpp:172:13: error: static assertion failed due to requirement 'is_readable_field::value': You're trying to use an unsupported field type in a row type. Review your row type definitions. 172 | is_readable_field<T>::value, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ 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). My second use case involves exposing in-memory data structures to foreign processes. In one of my previous jobs, we had a high-severity bug due to one of such tables getting stuck with incorrect data. Diagnosing the problem was quite involved. A tool that made it easy to inspect the data in such tables could have been a reasonable measure after the bugfix. I have a similar data structure in my servertech chat project (https://github.com/anarthal/servertech-chat/blob/master/server/src/services/...), so I thought on adapting the multi_index.cpp example to suit my needs. I have no prior experience with SQLite virtual tables, but I've been able to learn about them using SQLite's official documentation. Adapting multi_index.cpp has been challenging, though, because: a. Virtual tables are inherently complex. b. There is no discussion page on virtual tables in the proposed library. c. The provided examples are inherently complex and don't have many comments. As a result, I've found myself reverse-engineering the library and examples, which is not very rewarding. I was under the wrong impression that loading the module in one process would make it immediately available to other processes loading the database file onto which it was loaded. This is my fault, as there is no way for SQLite to do this. This implies that, once I have my module, I need to write a protocol to accept and execute SQLite queries (e.g. by using a UNIX socket). Unfortunately, the overall architecture is too complex and has security implications that need to be addressed, and thus would have never gone into production in the company I used to work for. This left me questioning whether using virtual tables to expose in-memory data structures to SQLite is really strong. If I'm not mistaken, the library has 3 examples on this, so I'm likely missing something. I think a good motivation for these three examples would be very valuable - that is, some comments stating what real world situations would require me to write code like the one in the examples. I'm actually not convinced of the value added by the library in terms of virtual tables. My impression is that providing tested, robust virtual table implementations for concrete use cases would add more value than what the current infrastructure offers. In other words, is writing virtual table implementations really something that happens in day to day development? I feel that the answer to this question is no, so I find it surprising that all examples except one are about virtual tables. Take this with perspective, though, as I'm no expert in this topic.

- Will the choice of API abstraction model ease the development of software that must talk to a SQLITE database?

The API simplifies regular SQL usage, as explained above. A couple of comments: 1. The virtual table infrastructure uses a macro (BOOST_SQLITE_NO_VIRTUAL) to conditionally make functions virtual. I think this is a potential problem in the long-run. If virtualization is not required, appropriate compile-time checks should be performed, instead. 2. sqlite::transaction features a throwing destructor, which I find surprising.

- What is your evaluation of the implementation?

I've only peaked at it in particular sites, and have the following comments: 1. transaction::commit throwing and non-throwing overloads seem to have different behavior on error. One will call rollback() and the other one won't. This case doesn't look to be covered by unit tests. 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. 3. Compiling with -Wall generates a lot of warnings, most of which are located in headers. I'd advise building with -Wall and -Werror in the CI to clean up these. 4. Passing incorrect parameter types to .execute() and .query() doesn't yield clean error messages. It would be beneficial implementing concepts (or a static_assert) for better messages. As I mentioned earlier, this also applies for the static interface. 5. Compiler coverage in CI looks improvable. The asan job seems commented out, and I can't see any build targeting C++20 or 23. 6. If BOOST_SQLITE_NO_VIRTUAL is to stay, it needs to be documented and tested.

- What is your evaluation of the documentation?

In my opinion, the documentation is insufficient for the level required by a Boost library. I've found the virtual table functionality specially involved because of this. Some of my major comments: 1. All supported, public functionality should be documented, both in the reference pages and in the discussion. This is not the case for most of the library: virtual tables, hooks, json, metadata, mutexes, transactions, blobs and memory management (sqlite::memory_tag) are not explained in the discussion. 2. Excepting virtual tables, the rest of the functionality isn't explained using examples, either. 3. Some Doxygen comments seem outdated, referencing functions that have been removed or renamed. See the bottom of this email for a list of what I found. 4. Many Doxygen comments don't provide the information I'd expect to find, like exception safety, lifetime rules, or the underlying sqlite3_xxx functions they call.

- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

I've spent around 12h doing inspecting the library, building the examples, writing the online store simulator, adapting the multi_index.cpp example and writing this review.

- Are you knowledgeable about the problem domain?

I've had mild SQLite experience in the past, mainly in prototypes. I didn't have prior experience with virtual tables. I have experience with SQL in general, and am the author of Boost.MySQL.

Final decision

Unfortunately, my final recommendation is to REJECT Boost.Sqlite in its current form. I think the concept is interesting and could benefit many people, but the library still requires some work before it gets into Boost. The documentation needs a lot of work. Getting some field experience would also be beneficial. Thanks again Klemens and Richard for your effort. I'd love to see the library being proposed again in the future. Typos and documentation comments: * The documentation should state which C++ standards and compilers are tested and supported. * The examples on virtual tables require step-by-step comments, since they implement non-trivial logic. * The examples on virtual tables need a rationale. That is, as a user, when would I find myself writing code like this? * The discussion seems to jump to advanced details too quickly. There are very few docs on everyday tasks. * The following types don't have any documentation at all but appear in the public namespace. They should be either documented or made private: allocator, unique_ptr, msize, make_unique, set_variant_result, like, glob, icmp, * Functionality that allocates using sqlite3-specific memory allocations should likely state that fact it in the reference. * Intro: "This library provides a simple C++ sqlite library." Do you mean "simple C++ SQLite wrapper library"? * Discussion: "Prepared statements can also be used multiple time and used with named parameters instead of indexed." should be "multiple times" and "named parameters instead of indexed ones." * Discussion: "The result of a query is a field type,": this is technically not true - you get a resultset, which yields rows that have fields. * Discussion: "Fields & values can have subtypes, while parameter to prepared statements do not have thos associated.": should be "parameters" and "do not have associated subtypes". * In general: avoid & as a way to state "and" * vtable.hpp: "@tparam T The implementation type of the module. It must inherit": I don't know what this phrase means. The type requirements for T likely need a page on their own. * vtable.hpp: "It's lifetime is managed by the database.": should be "Its" * vtable.hpp: "@param The requirements for `module`.": this creates a bogus parameter entry, looks like a leftover * field.hpp and value.hpp: "Returns the value as text, i.e. a string_view. Note that this value may be invalidated`.": this is not enough information. It should list when it gets invalidated. * statement.hpp: "The handle is shared between the statement & resultset. The statemens need to be kept alive.": should be "statement" and "needs". * statement.hpp: "This can be a map, a vector or a stuple": should be "tuple" * vtable.hpp (create function): "The instance_type gets used & managed" references an instance_type that does not exist (presumably you meant table_type). * vtable.hpp: "Destroy the storage = this function needs to be present for non eponymous tables": syntax error * vtable.hpp: "index info used by the find_index function": no find_index function exists, you presumably mean best_index * cstring_ref.hpp: doesn't have any function documented * connection.hpp: "Perform a query without parametert, It execute a multiple statement.": should be "parameter" * collation.hpp: "a case insensitive string omparison, e.g. from boost.urls": should be "comparison" * README.md: "auto r = q.current();''" there's an extra '' there. I'd advise to run documentation snippets to prevent such errors * multi_index.cpp: the create table statement uses the "url" name, which doesn't harm but is misleading * multi_index.cpp: I'd advise to mark overriding methods with the "override" keyword * multi_index.cpp: "static_assert(sizeof(const_iterator) <= sizeof(sqlite3_int64), "");": why is the static_assert needed? Please add a comment documenting it * multi_index.cpp and ordered_map.cpp: declares an "enum indices" that presumably was thought to make bit fiddling more clear, but don't seem to be used, increasing user confusion. On Wed, 13 Nov 2024 at 13:31, Richard Hodges via Boost wrote:

Dear All,

Surprise!

The Boost formal review of the Boost SQLITE library starts *TODAY*, taking place

from November 13th, 2024 to November 22nd, 2024 (inclusive).

I apologise profusely for springing this on you without prior warning. The error is entirely mine. I am extending the period by one day to compensate.

The library is authored by Klemens Morgenstern (@klemens-morgenstern in the CppLang slack).

Documentation: https://klemens.dev/sqlite/ https://anarthal.github.io/mysql/index.html Source: https://github.com/klemens-morgenstern/sqlite https://github.com/anarthal/mysql/

From the documentation:

boost.sqlite is a simple to use C++ sqlite library. It provides a modern interface using facilities like error_code, views (e.g. for blobs) and the ability to use boost.describe or boost.pfr for parameterised queries.

Supported features include:

- typed queries - prepared statements - json support - custom functions (scalar, aggregate, windows) - event hooks - virtual tables

SQLite provides an excellent C-API, so this library does not attempt to hide, but to augment it.

Please provide in your review information you think is valuable to explain your choice to ACCEPT or REJECT including SQLITE as a Boost library. Please be explicit about your decision (ACCEPT or REJECT).

Some other questions you might want to consider answering:

- Will the library bring additional out-of-the-box utility to Boost? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - Will the choice of API abstraction model ease the development of software that must talk to a SQLITE database? - Are there any immediate improvements that could be made after acceptance, if acceptance should happen? - Did you try to use the library? With which compiler(s)? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain?

More information about the Boost Formal Review Process can be found at: http://www.boost.org/community/reviews.html

The review is open to anyone who is prepared to put in the work of evaluating and reviewing the library. Prior experience in contributing to Boost reviews is not a requirement.

Thank you for your efforts in the Boost community. They are very much appreciated.

Richard Hodges - review manager of the proposed Boost.SQLITE library

Klemens is often available on CppLang Slack and of course by email should you require any clarification not covered by the documentation, as am I.

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

On Wed, Nov 13, 2024 at 4:31 AM Richard Hodges via Boost wrote:

Source: https://github.com/klemens-morgenstern/sqlite

Question for the author: * Who has tried integrating this library into their existing code, before the review period? Thanks

# boost_sqlite review Please accept my apologies for the poor formatting. And a bit thank you to everyone involved in this process. I have long wished for a sqlite wrapper with critical mass. ## Author's Background I am a professional C++ software developer. I have been a professional user and proponent of Boost libraries since the late 2000s. I have been using SQLite for more than 10 years and at one time wrote my own C++ wrapper. ## Initial Thoughts and Use Case At the time I saw the announcement for this review, I was actively evaluating libraries for a project I was (and continue to be) working on. I successfully used the boost_sqlite library in a limited capacity (create, insert, and pragmas only for on disk and in memory DBs). My first impression of the library is that it's extremely lightweight. This simplicity has value but also leaves the end user reinventing the wheel for certain repetitive operations. For example, using class enums for pragma get and set moves defect finding from runtime (bad) to compile time (better!). My second thought was how painful it was to include in my professional project. Our legacy software does not build Boost.json and, while it was trivial to add it for my sandbox app, it is an unnecessary extra dependency. ## Design Evaluation Simple, straight forward, and intuitive. I believe it meets my needs. ### Wishlist Here are some wishlist items I have. They may be inappropriate for this library and boost in general; however, they are still on my wishlist. #### connection It would be great if connection had a simple enum so I could do this: ```cpp boost::sqlite::connection mydb(boost::sqlite::memory); ``` Also, given C++17 `std::fstream` takes `std::filesystem::path`, it would be nice if `connection()` et all did as well. And I wouldn't complain if `boost::filesystem::path` was supported as well. ## Implementation Evaluation I reviewed this library as a user. ## Documentation Evaluation The readme.md file was adequate for me to quickly build a simple SQLite DB application. ## Potential Usefulness This is a useful library. I intend to incorporate this into professional projects as soon as it becomes available in a stable boost release. ## Personal Usage I used the following compiler in my development: gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 I included Boost and Boost.sqlite using CPM.cmake (fetchcontent). It was a trivial operation to include and link. ## Final Thoughts For the past few years I've lamented the lack of a Modern C++ SQLite wrapper with a critical mass of user backing. This library has the potential to fill that gap. I would like to see this library build without the sin dependency. With or without the build dependency change, this library should be accepted into Boost. Scott Bailey On Wed, Nov 13, 2024 at 6:30 AM Richard Hodges via Boost < boost@lists.boost.org> wrote:

Dear All,

Surprise!

The Boost formal review of the Boost SQLITE library starts *TODAY*, taking place

from November 13th, 2024 to November 22nd, 2024 (inclusive).

I apologise profusely for springing this on you without prior warning. The error is entirely mine. I am extending the period by one day to compensate.

The library is authored by Klemens Morgenstern (@klemens-morgenstern in the CppLang slack).

Documentation: https://klemens.dev/sqlite/ https://anarthal.github.io/mysql/index.html Source: https://github.com/klemens-morgenstern/sqlite https://github.com/anarthal/mysql/

From the documentation:

boost.sqlite is a simple to use C++ sqlite library. It provides a modern interface using facilities like error_code, views (e.g. for blobs) and the ability to use boost.describe or boost.pfr for parameterised queries.

Supported features include:

- typed queries - prepared statements - json support - custom functions (scalar, aggregate, windows) - event hooks - virtual tables

SQLite provides an excellent C-API, so this library does not attempt to hide, but to augment it.

Please provide in your review information you think is valuable to explain your choice to ACCEPT or REJECT including SQLITE as a Boost library. Please be explicit about your decision (ACCEPT or REJECT).

Some other questions you might want to consider answering:

- Will the library bring additional out-of-the-box utility to Boost? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - Will the choice of API abstraction model ease the development of software that must talk to a SQLITE database? - Are there any immediate improvements that could be made after acceptance, if acceptance should happen? - Did you try to use the library? With which compiler(s)? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain?

More information about the Boost Formal Review Process can be found at: http://www.boost.org/community/reviews.html

The review is open to anyone who is prepared to put in the work of evaluating and reviewing the library. Prior experience in contributing to Boost reviews is not a requirement.

Thank you for your efforts in the Boost community. They are very much appreciated.

Richard Hodges - review manager of the proposed Boost.SQLITE library

Klemens is often available on CppLang Slack and of course by email should you require any clarification not covered by the documentation, as am I.

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost