Re: Boost MySQL Review Starts Today

From: Klemens Morgenstern (klemens.morgenstern_at_[hidden])
Date: 2022-05-11 05:13:13

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?