Ruben Perez wrote:
I note that "Ease of use" is claimed as the first design goal, which is good.
I think I failed to make the scope of the library clear enough in this aspect. The library is supposed to be pretty low level and close to the protocol, and not an ORM.
It seems to me that what you have *could* be a genuinely easy-to-use library with a relatively small amount of extra work. Specifically, if you were to hide the initial ASIO boilerplate then synchronous queries could be quite concise and require no ASIO knowledge. By chance last night I noticed this linked from the isocpp.org website: Writing a Network Client with Facebook's Wangle is a Fail by Richard Thomson From the video description: Networking is often a core requirement for modern applications, but the standard C++ library doesn't yet include any networking support. The Boost libraries have had networking components in the ASIO library, but it looks complicated and filled with details. [snip] I certainly agree with "looks complicated"; I have never summoned the energy to dive into it. So why would you want to limit the audience of your library only to those who are already ASIO users or are willing to invest in learning to use it, when with the addition of just a few more convenience functions you could hide ASIO as an implementation detail for a useful subset of your functionality?
If you take a look to any other asio-based program, the user is always in charge of creating the io_context, and usually in charge of creating the SSL context, too.
But take a look at any other database connection library and you won't find that.
I'm not keen on creating a function that both resolves the hostname and connects the connection, as I think it encourages doing more name resolution than really required (you usually have one server but multiple connections).
It's a mistake to keep an IP address for too long; you should re-resolve the hostname based on the time-to-live. Consider the case where the server has failover DNS or load balancing. Does the ASIO resolver cache based on the TTL? If it does, those calls should be cheap.
make_connection("mysql://admin:12345@hostname:3306/dbname");
I guess you're suggesting that make_connection also perfor the name resolution, the physical connect and the MySQL handshake?
Yes, that's exactly what I'm suggesting. See e.g. PQconnectdb() in libpq.
Another point about the connection parameters: you should provide a way to supply credentials without embedding them in the source code. You should aim to make the secure option the default and the simplest to use. I suggest that you support the ~/.my.cnf and /etc/my.cnf files and read passwords etc. from there, by default. You might also support getting credentials from environment variables or by parsing the command line. You could even have it prompt for a password.
I don't know of any database access library that does this.
In libpq, I normally pass "service=foo" as the connection parameter string and it looks that up in pg_service.conf. I think that can specify a password. It also supports a ~/.pgpass file. The AWS SDK for DynamoDB looks for credentials in a ~/.something file. The aim should be "secure by default". Users are lazy. The particular danger in this case is that they do an initial test with the password in the source, and then move it somewhere secure later, but the password is still exposed in their revision control history. At this point in history, there is no excuse to repeat the mistakes that have lead to really very serious security problems in the past. Make the default mechanism, and the first one that you describe in the docs, the most secure one.
Just use std::getenv, std::stdin or whatever mechanism your application needs and get a string from there, then pass it to the library. All the examples read the password from argv. Additionally, having passwords in plain text files like ~/.my.cnf and /etc/my.cnf is considered bad practice in terms of security, I wouldn't encourage it.
The command line and the environment are potentially just as exposed as files, via /proc. If you want, refuse to read from a ~/. file if it is world-readable. lpbpq does this for .pgpass.
Invoking the prepared statement seems unnecessarily verbose. Why can't I just write
auto result = my_query("hello", "world", 42);
Because this invokes a network operation. By Asio convention, you need a pair of sync functions (error codes and exceptions) and at least an async function, that is named the same as the sync function but with the "_async" suffix.
Make operator() an alias for execute(). It would be good if it could also be used in the coroutine case, i.e. if my connection_type is based on a coro_executor_type then maybe operator() could be an alias for async_execute: auto r = co_await query.async_execute(params); or auto r = co_await query(params);
auto result = ...execute query... for (auto&& row: result) { ... }
How does this translate to the async world?
I have found it useful to have a Transaction class:
{ Transaction t(conn); // Issues "BEGIN" .... run queries .... t.commit(); // Issues "COMMIT" } // t's dtor issues "ROLLBACK" if we have not committed.
Again, how would this work in the async world?
I'm far from an expert, but you should be able to write code something like: auto result = co_await query(params); for co_await (auto&& row: result) { ... } auto t = co_await Transaction(conn); .... co_await t.commit(); // No idea about how to await the destructor, sorry. Of course it's much more horrible if you're using callbacks, but then everything is horrible with all those nested lambdas. Heck, it's difficult even to write a loop!
How does the destructor handle communication failures when issuing the ROLLBACK?
I suggest mark the connection as broken but otherwise ignore the error.
I use NUMERIC quite a lot in PostgreSQL; I don't know if the MySql type is similar. I would find it inconvenient that it is treated as a string. Can it not be converted to a numeric type, if that is what the user wants?
MySQL treats NUMERIC and DECIMAL the same, as exact numeric types. What C++ type would you put this into? float and double are not exact so they're not fit.
Provide a mechanism that allows the users to indicate what type they want to convert them to. Regards, Phil.