Re: [boost] Boost MySQL Review

From the current state of the documentation, if i have a table with

My (small) review of Boost MySQL. First, i want to apologize for this not to be a real review. I just could not find the time to make a proper one, so this is just some thoughts that i want to share in the hope they are useful. ======== Are you knowledgeable about the problem domain? ======== Not really. I've used mySQL in the far past, not from C++, and more recently used postgreSQL (via libpq) and sqlite3. I'm not unfamiliar with asio, but never used it outside very small projects. ======== How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? ======== A quick reading. My review is solely based on reading the documentation (which, by the way, is pretty good). ======== Will the library bring additional out-of-the-box utility to Boost? ======== That's a great YES. Definitely useful. I lacked such a library a few years ago (was for postgreSQL, but the issue is the same). ======== What is your evaluation of the implementation? ======== Can't tell, i did not look at it. ======== What is your evaluation of the documentation? ======== Pretty good. I stumbled several times while navigating into a « This Page Intentionally Left Blank » page. I have no doubt that this will be fixed. Just a few issues i noted : * the documentation on encodings/collation could probably be improved. three columns, one in utf8, the other one in utf-16, and the last one in iso8859-15, i'm not sure what the returned string_view contents will be. Is this the raw data ? And if i'm filtering in a WHERE clause, what is the expected encoding of the values ? The connection encoding or the field encoding ? Is this different between prepared statements and litteral queries ? (i have the intuition that it is, from a quick look at MySQL doc). * the same goes for date storage and time zone handling. I can live with the fact that the library only handles UTC dates, if that's correctly handled. I have had very bad experiences with postgreSQL trying to be smart and causing more pain than good when handling timezones. I don't know how bad mysql is in this regard, but I'm pretty sure there are some pitfalls here as well. While i understand this may be a bit out of scope for the library, it does provides conversion between mysql raw data and a c++ time_point, so it should at least make sure that this time_point is an UTC one. Given that some mysql date/time types are local or encode a time zone, i expect some issues to raise here. ======== Will the choice of API abstraction model ease the development of software that must talk to a MySQL database? ======== I believe yes, however, i have a few concerns with the API : * the value variant type has a get_optional member (and the get_std_optional variant). I would prefer that it follows the std variant get_if pattern. That would also solve the need to handle both std/boost optional. * i'm not at ease with the fact that value get and get_optional handles the same way a NULL value and a type mismatch. I would like to be able to write get(), and get an empty optional if value is NULL, and an exception if value is a float. get_if would cause implementation issues, but i think it's just not worth it and we can live without (a failure in that case means a database schema and program mistake, so an exception is fine here). Since the user would be in charge of providing the optional type, that may also solve the issue of handling both optional types. * for resultset, the async_read_all/many completion handlers takes a vector<row>. I would prefer if it were an opaque iterable type (whose iteration element is a row). This would probably gives far more room for further optimizations without breaking user code. * similarly, the row type exposes the fields via a vector of value. I would prefer if it would itself be iterable (iteration element being a value). IMHO it should also be "visitable", ie something like row.for_each(visitor), visitor being a class with overloaded either operator()(int field_index, int/float/string_view/...) or operator()(string_view field_name, int/float/string_view/...). That would also allow gives more room * the library define its own « days » type, and explicitly states that it may not be the same as c++20 days. I may have missed the reason (they seems equally defined), but i would like it to use c++20 days when compiled under a c++20 compiler. ======== Are there any immediate improvements that could be made after acceptance, if acceptance should happen? ======== I think the API can be improved. I think some API changes would allow some internal improvements. However, the library in its current state seems already useful. The overall impression from reading other reviews is that the quality of the implementation is very good. My opinion is thus that the library should be ACCEPTED, but it should be made quite clear that the library is still in a state where some API changes can occur, and any form of source compatibility may be broken by update. And great thanks to Ruben for writing this library, and taking the time to get it into boost. Regards, Julien Le samedi 14 mai 2022 à 10:58 +0200, Richard Hodges via Boost a écrit :

Dear All,

We've had a good response for calls to review this much anticipated library, and there is a lot of interest on Reddit: https://www.reddit.com/r/cpp/comments/unik91/acceptance_review_for_boostmysq... .

In general the public perception of utility libraries like this in Boost seems popular, and Boost's emphasis on quality is often mentioned.

If you are able, I would be more than grateful if you could take a few hours over this delightful weekend to offer a review. The more eyes on this project the better, as if it is successful, it will no doubt prompt submission of similar libraries targeting other popular database and messaging systems.

The Boost formal review of the MySQL library started on May 10th, 2022 and will conclude on May 19th, 2022 (inclusive) - In fact, I am able to accept submissions up until the (GMT) morning of May 20th as I'll be traveling on May19th.

The library is authored by Rubén Pérez Hidalgo (@anarthal in the CppLang slack).

Documentation: https://anarthal.github.io/mysql/index.html Source: https://github.com/anarthal/mysql/

The library is built on the bedrock of Boost.Asio and provides both synchronous and asynchronous client connectors for the MySQL database system.

Boost.MySQL is written from the ground up, implementing the entire protocol with no external dependencies beyond the Boost library. It is compatible with MariaDB.

Connectivity options include TCP, SSL and Unix Sockets.

For async interfaces, examples in the documentation demonstrate full compatibility with all Asio completion handler styles, including:

Callbacks:- https://anarthal.github.io/mysql/mysql/examples/query_async_callbacks.html

Futures :- https://anarthal.github.io/mysql/mysql/examples/query_async_futures.html

Boost.Coroutine :- https://anarthal.github.io/mysql/mysql/examples/query_async_coroutines.html

C++20 Coroutines :- https://anarthal.github.io/mysql/mysql/examples/query_async_coroutinescpp20....

Rubén has also implemented the Asio protocols for deducing default completion token types :- https://anarthal.github.io/mysql/mysql/examples/default_completion_tokens.ht...

Reviewing a database connector in depth will require setting up an instance of a MySQL database. Fortunately most (all?) Linux distributions carry a MySQL and/or MariaDB package. MySQL community edition is available for download on all platforms here: https://dev.mysql.com/downloads/

Rubén has spent quite some time in order to bring us this library candidate. The development process has no doubt been a journey of discovery into Asio, its concepts and inner workings. I am sure he has become a fount of knowledge along the way.

From a personal perspective, I was very happy to be asked to manage this review. I hope it will be the first of many more reviews of libraries that tackle business connectivity problems without further dependencies beyond Boost, arguably one of the most trusted foundation libraries available.

Please provide in your review information you think is valuable to understand your choice to ACCEPT or REJECT including Describe 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 MySQL 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.MySQL library

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