On Tue, 2022-05-10 at 09:14 +0200, Richard Hodges via Boost wrote: Here's my review
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/
I've spun up a small test program https://gist.github.com/klemens-morgenstern/0195a502340bd33029437d33eb914f2c which worked very well & was intuitive given my asio & sql experience. This library should definitely be accepted into boost. A few things are odd (like the .async_handshake on the ssl-conn; I would have expected that to require .next_layer().async_handshake). The ssl option also seems to be a runtime parameter in many functions, which confused me a bit. Is that so I can use an ssl::stream<> but turn it off at runtime? If so, why is that needed, why wouldn't I just us a tcp::socket directly? I think returning objects through completion handlers is absolutely fine and Asio-ish; but the library will surely grow to include more overloads in the future.
Some other questions you might want to consider answering:
- Will the library bring additional out-of-the-box utility to Boost?
Yes.
- What is your evaluation of the implementation?
It looks to be based on async_compose & asio::coroutine, which is a proven pattern . In addition, I am impressed and happy that it supports cancellation. The prepared statements are easy to use, so that one can prevent injection attacks properly.
- What is your evaluation of the documentation?
It desribes the library quite well. I would like a section on transactions, because I feel like there might be some sharp edges there.
- Will the choice of API abstraction model ease the development of software that must talk to a MySQL database?
It's exactly the right amount of detail for what it is: a database connector. It's not an ORM nor some wrapper that tries to prevent users from doing stupid things (like forwarding stdin to the DB). Either of those problems should be solved in another library that could be built on top of boost.mysql.
- Are there any immediate improvements that could be made after acceptance, if acceptance should happen?
I think additional overloads, depending on user requests. I also wonder if there is some useful transaction support, but this largely depends on the mysql protocol. It could be that there is no point to that.
- Did you try to use the library? With which compiler(s)? Did you have any problems?
Clang-13 with libc++ and C++20.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Build a test program and read the doc for ~1h.
- Are you knowledgeable about the problem domain?
Yes. I vote to accept the library into boost. PS: Isn't mysql a registered trademark owned by oracle?