Here is my review of Ruben Perez's proposed MySql library. Background ---------- I have previously implemented C++ wrappers for PostgreSQL and SQLite, so I have some experience of what an SQL API can look like. I know little about ASIO. I have also recently used the AWS SDKs for C++ and Javascript to talk to DynamoDB; this has async functionality, which is interesting to compare. I confess some minor disappointment that MySql, rather than PostgreSQL or SQLite, is the subject of this first Boost database library review, since those others have liberal licences that are closer to Boost's own licence than MySql (and MariaDB). But I don't think that should be a factor in the review. Trying the library ------------------ I have tried using the library with - g++ 10.2.1, Arm64, Debian Linux - ASIO from Boost 1.74 (Debian packages) - Amazon Aurora MySql-compatible edition I've written a handful of simple test programs. Everything works as expected. Compilation times are a bit slow but not terrible. The remainder of this review approximately follows the structure of the library documentation. Introduction ------------ I note that "Ease of use" is claimed as the first design goal, which is good. I feel that some mention should be made of the existing C / C++ APIs and their deficiencies. You should also indicate whether or not the network protocol you are using to communicate with the server is a "public" interface with some sort of stability guarantee. (I guess maybe it is, if it is common to MySql and MariaDB.) Tutorial -------- The code fragments should start with the necessary #includes, OR you should prominently link to the complete tutorial source code at the start. You say that "this tutorial assumes you have a basic familiarity with Boost.Asio". I think that's unfortunate. It should be possible for someone to use much of the library's functionality knowing almost nothing about ASIO. Remember your design goal of ease-of-use. In fact, it IS possible to follow the tutorial with almost no knowledge of ASIO because I have just done so. You have this boilerplate at the start of the tutorial: boost::asio::io_context ctx; boost::asio::ssl::context ssl_ctx (boost::asio::ssl::context::tls_client); boost::mysql::tcp_ssl_connection conn (ctx.get_executor(), ssl_ctx); boost::asio::ip::tcp::resolver resolver (ctx.get_executor()); auto endpoints = resolver.resolve(argv[3], boost::mysql::default_port_string); boost::mysql::connection_params params ( argv[1], // username argv[2] // password ); conn.connect(*endpoints.begin(), params); // I guess that should really be doing something more // intelligent than just trying the first endpoint, right? I would like to see a convenience function that hides all of that: auto conn = boost::mysql::make_connection( ...params... ); I guess this will need to manage a global, private, ctx object or something. There are various possibilities for the connection parameters to pass to that, e.g. connection_params params = { .hostname = ".....", .port = 3306, // why is that a string in yours? .user = "admin", .password = "12345", .dbname = "foo" }; make_connection(params); Or make_connection("mysql://admin:12345@hostname:3306/dbname"); Now... why the heck does your connection_params struct use string_views? That ought to be a Regular Type, with Value Semantics, using std::strings. Is this the cult of not using strings because "avoid copying above all else"? 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. Does MySQL support authentication using SSL client certs? I try to use this for PostgreSQL when I can. If it does, you should try to support that too. About two thirds of the way through the tutorial, it goes from "Hello World" to retrieving "employees". Please finish the hello world example with code that gets the "Hello World" string from the results and prints it. Queries ------- I encourage you to present prepared queries first in the documentation and to use them almost exclusively in the tutorial and examples. You say that "client side query composition is not available". What do you mean by "query composition"? I think you mean concatenating strings together'); drop table users; -- to form queries, right? Is that standard MySql terminology? I suggest that you replace the term with something like "dangerous string concatenation". In any case, that functionality *is* available, isn't it! It's trivial to concatenate strings and pass them to your text query functions. You're not doing anything to block that. So what you're really saying is that you have not provided any features to help users do this *safely*. I think that's a serious omission. It would not be difficult for you to provide an escape_for_mysql_quoted_string() function, rather than having every user roll their own slightly broken version. IIRC, in PostgreSQL you can only use prepared statements for SELECT, UPDATE, INSERT and DELETE statements; if you want to do something like ALTER TABLE a ALTER COLUMN c SET DEFAULT = ? or CREATE VIEW v as SELECT * FROM t WHERE c = ? then you are forced to do string concatenation with escaping of parameters. But according to https://dev.mysql.com/doc/refman/8.0/en/sql-prepared-statements.html, MySQL allows more statements to be prepared. In what cases can prepared statements not be used? Perhaps non-prepared statements really can be hidden as mysql::unsafe::statement? Regarding your prepared statements: tcp_ssl_prepared_statement is verbose. Why does the prepared statement type depend on the underlying connection type? I have to change it if I change the connection type?! If that's unavoidable, I suggest putting a type alias in the connection type: connection_t conn = ....; connection_t::prepared_statement stmt(.....); Does MySql allow numbered or named parameters? SQLite supports ?1 and :name; I think PostgreSQL uses $n. Queries with lots of parameters are error-prone if you just have ?. If MySql does support this, it would be good to see it used in some of the examples. Invoking the prepared statement seems unnecessarily verbose. Why can't I just write auto result = my_query("hello", "world", 42); ? That's a variadic operator(). In my PostgreSQL library, I chose to specify the query parameter types as template parameters to the query (prepared statement) type: Query<std::string, std::string, int> my_query("....."); Query's operator() takes exactly those types (maybe with some TMP to use std::string_view where std::string is specified). With hindsight, a default where all parameters are strings would have been useful in practice. I also added Query variants where the result is expected to be - A single value, e.g. a SELECT COUNT(*) statement. - Empty, e.g. INSERT or DELETE. - A single row. - Zero or one rows. - A single column. This is just syntactic sugar but it makes life easier. For example: SingletonQuery<int, string> sessions_for_user("SELECT COUNT(*) FROM sessions WHERE username = ?"); // int is the return type, string is the one parameter type. auto n_sessions = sessions_for_user("phil"); I don't see anything about the result of an INSERT or UPDATE. PostgreSQL tells me the number of rows affected, which I have found useful to return to the application. resultset, row and value ------------------------ I'm not enthusiastic about the behaviour nor the names of these types: - resultset is not a set of results. It's more like a sequence of rows. But more importantly, it's lazy; it's something like an input stream, or an input range. So why not actually make it an input range, i.e. make it "a model of the input_range concept". Then we could write things like: auto result = ...execute query... for (auto&& row: result) { ... } - row: not bad, it does actually represent a row; it's a shame it's not a regular type though. - value: it's not a value! It doesn't have value semantics! I guess it is time to address the string_view issue..... I just modified one of my example programs to copy from the string_view in the variant into a std::string, and it slowed the program down by... no, it actually went fractionally faster. The use of string_views in the value variant seems to me to be a premature optimisation. There are significant disadvantages to using string_view here. Your default API should have value semantics. If you want, provide another "low copying" API for users who want it with names that make it clear what they are doing. The problem is that the lifetime of the string_view is tied to the row in a non-obvious way. I'm also uncertain that a variant for the individual values is the right solution here. All the values in a column should have the same type, right? (Though some can be null.) So I would make row a tuple. Rather than querying individual values for their type, have users query the column. MySQL to C++ mapping reference ------------------------------ It seems odd that MySQL small integers all map to C++ 64-bit types. 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? Need to consume the entire resultset ------------------------------------ I seem to get an assertion if I fail to read the resultset (resultset.hpp:70). Could this throw instead? Or, should the library read and discard the unread results in this case? Going Async ----------- I have tried this with both callbacks and C++20 coroutines and it seems to work as expected. I'm actually delighted to see the support for coroutines here. But the lack of protocol support for multiple in-flight queries immediately becomes apparent. It almost makes me question the value of the library - what's the point of the async support, if we then have to serialise the queries? Should the library provide this serialisation for us? I.e. if I async_execute a query while another is in progress, the library could wait for the first to complete before starting the second. Or, should the library provide a connection pool? (Does some other part of ASIO provide connection pool functionality that can be used here?) Transactions ------------ 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. Nested transactions are an interesting problem. Trademark --------- Klemens Morgenstern makes the point that MySql is a trademark of Oracle. Calling this "Boost.MySql" doesn't look great to me. How can you write "The Boost MySql-compatible Database Library" more concisely? Conclusion ========== Good points: It actually works! Great to see coroutines! Bad points: The resultset / row / value type system is a poor design. A lot of boilerplate could be hidden from many users. Connection pool may be required for useful performance. Several features should be re-worked to be secure by default. Use of MySQL trademark needs to be considered. Overall, I think this proposal needs a fair amount of API re-design and additional features to be accepted, and should be rejected at this time. It does seem to be a good start though! Thanks to Ruben for the submission. (I have spend one working day on this review. I have not looked at any of the code.) Regards, Phil.
On Wed, 11 May 2022 at 20:23, Phil Endecott via Boost <boost@lists.boost.org> wrote:
Here is my review of Ruben Perez's proposed MySql library.
Hi Phil, thank you for taking your time to write a review.
Background ----------
I have previously implemented C++ wrappers for PostgreSQL and SQLite, so I have some experience of what an SQL API can look like. I know little about ASIO.
I have also recently used the AWS SDKs for C++ and Javascript to talk to DynamoDB; this has async functionality, which is interesting to compare.
I confess some minor disappointment that MySql, rather than PostgreSQL or SQLite, is the subject of this first Boost database library review, since those others have liberal licences that are closer to Boost's own licence than MySql (and MariaDB). But I don't think that should be a factor in the review.
Trying the library ------------------
I have tried using the library with
- g++ 10.2.1, Arm64, Debian Linux - ASIO from Boost 1.74 (Debian packages) - Amazon Aurora MySql-compatible edition
I've written a handful of simple test programs. Everything works as expected. Compilation times are a bit slow but not terrible.
The remainder of this review approximately follows the structure of the library documentation.
Introduction ------------
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. I list ease of use here in the sense that * I have tried to abstract as much of the oddities of the protocol as possible (e.g. text and binary encodings). * The library takes care of SSL as part of the handshake, vs having the user have to take care of it. * The library provides helper connect() and close() functions to make things easier. * The object model is as semantic as I have been able to achieve, vs. having a connection object and standalone functions. * The value class offers stuff like conversions to make some use-cases simpler. I guess I listed that point in comparison to Beast or Asio, which are even lower level. Apologies if it caused confusion.
I feel that some mention should be made of the existing C / C++ APIs and their deficiencies. You should also indicate whether or not the network protocol you are using to communicate with the server is a "public" interface with some sort of stability guarantee. (I guess maybe it is, if it is common to MySql and MariaDB.)
Updated https://github.com/anarthal/mysql/issues/50 on comparison with other APIs. The network protocol is public and documented (although the documentation is pretty poor). It's indeed a pretty old protocol that is not being extended right now, and it's widely used by a lot of clients today, so there is very little risk there.
Tutorial --------
The code fragments should start with the necessary #includes, OR you should prominently link to the complete tutorial source code at the start.
Raised https://github.com/anarthal/mysql/issues/71 to track it.
You say that "this tutorial assumes you have a basic familiarity with Boost.Asio". I think that's unfortunate. It should be possible for someone to use much of the library's functionality knowing almost nothing about ASIO. Remember your design goal of ease-of-use. In fact, it IS possible to follow the tutorial with almost no knowledge of ASIO because I have just done so.
If you are to really take advantage of the library (i.e. use the asynchronous API), you will need some Asio familiarity. I'd say a very basic understanding is enough (i.e. knowing what a io_context is). If you think this comment is misleading, I can remove it. But I don't think this is the right place to provide a basic Asio tutorial.
You have this boilerplate at the start of the tutorial:
boost::asio::io_context ctx; boost::asio::ssl::context ssl_ctx (boost::asio::ssl::context::tls_client); boost::mysql::tcp_ssl_connection conn (ctx.get_executor(), ssl_ctx); boost::asio::ip::tcp::resolver resolver (ctx.get_executor()); auto endpoints = resolver.resolve(argv[3], boost::mysql::default_port_string); boost::mysql::connection_params params ( argv[1], // username argv[2] // password ); conn.connect(*endpoints.begin(), params); // I guess that should really be doing something more // intelligent than just trying the first endpoint, right?
The way to go here is providing an extra overload for connection::connect. Raised https://github.com/anarthal/mysql/issues/72 to track it.
I would like to see a convenience function that hides all of that:
auto conn = boost::mysql::make_connection( ...params... );
I guess this will need to manage a global, private, ctx object or something.
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. If you take a look to this Boost.Beast example, you will see similar stuff: https://www.boost.org/doc/libs/1_79_0/libs/beast/example/http/client/sync-ss... 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). I may be wrong though, so I'd like to know what the rest of the community thinks on this.
.port = 3306, // why is that a string in yours?
It is not "mine", it's just how Asio works. Please have a look at https://www.boost.org/doc/libs/1_79_0/doc/html/boost_asio/reference/ip__basi...
make_connection("mysql://admin:12345@hostname:3306/dbname");
I guess you're suggesting that make_connection also perform the name resolution, the physical connect and the MySQL handshake? I'm not against this kind of URL-based way of specifying parameters. I've used it extensively in other languages. May be worth reconsidering it once Vinnie's Boost.Url gets accepted.
Now... why the heck does your connection_params struct use string_views? That ought to be a Regular Type, with Value Semantics, using std::strings. Is this the cult of not using strings because "avoid copying above all else"?
I may have been a little too enthusiastic about optimization here.
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. The official Python connector gets the password from a string. I think this is mixing concerns. Having the password passed as a string has nothing to do with having it embedded in the source code. 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.
Does MySQL support authentication using SSL client certs? I try to use this for PostgreSQL when I can. If it does, you should try to support that too.
AFAIK you can make the server validate the client's certificate (that doesn't require extra library support), but you still have to pass a password.
About two thirds of the way through the tutorial, it goes from "Hello World" to retrieving "employees". Please finish the hello world example with code that gets the "Hello World" string from the results and prints it.
My bad, that's a naming mistake - it should be named hello_resultset, instead. It's doing the right thing with the wrong variable name. Updated https://github.com/anarthal/mysql/issues/71
Queries -------
I encourage you to present prepared queries first in the documentation and to use them almost exclusively in the tutorial and examples.
It can definitely make sense.
You say that "client side query composition is not available". What do you mean by "query composition"? I think you mean concatenating strings together'); drop table users; -- to form queries, right? Is that standard MySql terminology? I suggest that you replace the term with something like "dangerous string concatenation".
Yes, I mean that.
In any case, that functionality *is* available, isn't it! It's trivial to concatenate strings and pass them to your text query functions. You're not doing anything to block that. So what you're really saying is that you have not provided any features to help users do this *safely*. I think that's a serious omission. It would not be difficult for you to provide an escape_for_mysql_quoted_string() function, rather than having every user roll their own slightly broken version.
Definitely not trivial (please have a look at MySQL source code) but surely beneficial, see below. Tracked by https://github.com/anarthal/mysql/issues/69.
IIRC, in PostgreSQL you can only use prepared statements for SELECT, UPDATE, INSERT and DELETE statements; if you want to do something like
ALTER TABLE a ALTER COLUMN c SET DEFAULT = ? or CREATE VIEW v as SELECT * FROM t WHERE c = ?
You are right, these cases aren't covered by prepared statements. https://github.com/anarthal/mysql/issues/69 tracks it.
tcp_ssl_prepared_statement is verbose. Why does the prepared statement type depend on the underlying connection type?
Because it implements I/O operations (execute() and close()), which means that it needs access to the connection object, thus becoming a proxy object.
I have to change it if I change the connection type?! If that's unavoidable, I suggest putting a type alias in the connection type:
connection_t conn = ....; connection_t::prepared_statement stmt(.....);
Raised https://github.com/anarthal/mysql/issues/73
Does MySql allow numbered or named parameters? SQLite supports ?1 and :name; I think PostgreSQL uses $n. Queries with lots of parameters are error-prone if you just have ?. If MySql does support this, it would be good to see it used in some of the examples.
Not AFAIK, just regular positional placeholders.
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. I'm not against this kind of signature, building on top of what there already is: statement.execute("hello", "world", 42); statement.async_execute("hello", "world", 42, use_future); Which saves you a function call. Raised https://github.com/anarthal/mysql/issues/74
I also added Query variants where the result is expected to be
- A single value, e.g. a SELECT COUNT(*) statement. - Empty, e.g. INSERT or DELETE. - A single row. - Zero or one rows . - A single column.
I think this can be useful. I've updated https://github.com/anarthal/mysql/issues/22 to track this.
I don't see anything about the result of an INSERT or UPDATE. PostgreSQL tells me the number of rows affected, which I have found useful to return to the application.
Please have a look at https://anarthal.github.io/mysql/mysql/resultsets.html#mysql.resultsets.comp...
resultset, row and value ------------------------
I'm not enthusiastic about the behaviour nor the names of these types:
Resultset is how MySQL calls it. It's not my choice.
- resultset is not a set of results. It's more like a sequence of rows. But more importantly, it's lazy; it's something like an input stream, or an input range. So why not actually make it an input range, i.e. make it "a model of the input_range concept". Then we could write things like:
auto result = ...execute query... for (auto&& row: result) { ... }
How does this translate to the async world?
- row: not bad, it does actually represent a row; it's a shame it's not a regular type though.
- value: it's not a value! It doesn't have value semantics!
If the library gets rejected I will likely make values owning (regular).
I'm also uncertain that a variant for the individual values is the right solution here. All the values in a column should have the same type, right? (Though some can be null.) So I would make row a tuple. Rather than querying individual values for their type, have users query the column.
Are you talking about something like this? https://github.com/anarthal/mysql/issues/60
It seems odd that MySQL small integers all map to C++ 64-bit types.
It is done like this to prevent the variant from having too many alternatives - I don't think having that would add much value to the user. If I implement something like https://github.com/anarthal/mysql/issues/60, each int type will be mapped to its exact type.
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.
I seem to get an assertion if I fail to read the resultset (resultset.hpp:70). Could this throw instead?
This assertion is not related to this. It's just checking that the resultset has a valid connection behind it, and is not a default constructed (invalid) resultset.
Or, should the library read and discard the unread results in this case?
Having a look into the Python implementation, it gives the option to do that. I think we can do a better job here. Track by https://github.com/anarthal/mysql/issues/14
But the lack of protocol support for multiple in-flight queries immediately becomes apparent. It almost makes me question the value of the library - what's the point of the async support, if we then have to serialise the queries?
As I pointed out in another email, it's a combination of lack of protocol support and lack of library support. Apologies if the documentation is not clear in this aspect. I think there is value in it, though, so you don't need to create 5000 threads to manage 5000 connections. The fact that the official MySQL client has added a "nonblocking" mode seems a good argument.
Should the library provide this serialisation for us? I.e. if I async_execute a query while another is in progress, the library could wait for the first to complete before starting the second.
I would go for providing that bulk interface I talk about in other emails.
Or, should the library provide a connection pool? (Does some other part of ASIO provide connection pool functionality that can be used here?)
Asio doesn't provide that AFAIK. It is definitely useful functionality, tracked by https://github.com/anarthal/mysql/issues/19
Transactions ------------
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? How does the destructor handle communication failures when issuing the ROLLBACK?
Klemens Morgenstern makes the point that MySql is a trademark of Oracle. Calling this "Boost.MySql" doesn't look great to me. How can you write "The Boost MySql-compatible Database Library" more concisely?
I'm not very original at naming, as you may have already noticed. Using Boost.Delfin was proposed at some point, but Boost.Mysql definitely expresses its purpose better.
Overall, I think this proposal needs a fair amount of API re-design and additional features to be accepted, and should be rejected at this time. It does seem to be a good start though!
Thanks to Ruben for the submission.
Thank you for sharing your thoughts, I think there is a lot of useful information here.
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.
On 13.05.22 17:23, Phil Endecott via Boost wrote:
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.
Looking for credentials in a file on disk may be more secure than embedding the credentials in code, but it is most definitely the most secure mechanism. The most secure mechanism is to always ask the user at program start-up. Or better yet, ask each time a connection is created, and then immediately wipe the credentials from RAM in order to mitigate RAM scanning attacks. -- Rainer Deyke (rainerd@eldwood.com)
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.
This is a big war, as then you have thousands of ASIO users who find the usual pattern convincing, and they would complain about something similar. It may be worth sharing your thoughts to Chris, Asio's author, so that all Asio based libraries follow the same approach.
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.
May be worth considering, raised https://github.com/anarthal/mysql/issues/86 to track it.
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.
I think this varies a lot depending on the deployment strategy you're using. If you're using kubernetes, you have several options, too. I don't think it is the job of this library to try to accommodate every single possible deployment strategy the user may think of.
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:
statement.execute("a", 42, "c"); statement("a", 42, "c"); I honestly don't see a meaningful difference between these two.
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);
AFAIK you can't co_await constructors, destructors, nor there is a Python's async for equivalent yet. So this API is restricted to sync ops.
I suggest mark the connection as broken but otherwise ignore the error.
So subsequent operations fail with a certain error code?
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.
Could you please elaborate? Regards, Ruben.
On Wed, 11 May 2022 at 19:23, Phil Endecott via Boost <boost@lists.boost.org> wrote:
Trademark ---------
Klemens Morgenstern makes the point that MySql is a trademark of Oracle. Calling this "Boost.MySql" doesn't look great to me.
I agree. Of all the trademarks in the world you could violate, Oracle's would be close to last on my list. As far as I know, Oracle haven't objected to MySQL++ < https://tangentsoft.com/mysqlpp/> or mysqlcppapi < http://mysqlcppapi.sourceforge.net/> but Boost is higher profile than both of them, and it's not a gamble I'd want to take. http://mysqlcppapi.sourceforge.net/
How can you write "The Boost MySql-compatible Database Library" more concisely?
It's not a MySQL-compatible database library because it isn't a database. MySQL does actually have a "database library", i.e. an embeddable library that provides an in-process RDBMS. That's not what the proposed library does. It's a client API for connecting to a database server. In MySQL world I think these are called "connectors". I recommend an original name for the project, and only mention MySQL in the "headline" description, something like "A client library for connecting to MySQL and MariaDB databases".
On Fri, May 20, 2022 at 1:03 PM Jonathan Wakely via Boost <boost@lists.boost.org> wrote:
I recommend an original name for the project,
I propose Boost.Mya :) Continues the "tradition" introduced by Boost.Hana: https://www.boost.org/doc/libs/1_61_0/libs/hana/doc/html/index.html#tutorial... My is probably uncontroversial and unencumbered. And the a could be for Maria and/or async. or asio Or Boost.Maria instead, why not, with even more direct connection. It's Friday. Just having a little fun...
and only mention MySQL in the "headline" description, something like "A client library for connecting to MySQL and MariaDB databases".
I think Boost.MySQL is the best name though. --DD
On Fri, May 20, 2022 at 7:15 AM Dominique Devienne wrote:
On Fri, May 20, 2022 at 1:03 PM Jonathan Wakely wrote:
I recommend an original name for the project,
I propose Boost.Mya :) Continues the "tradition" introduced by Boost.Hana: Or Boost.Maria instead, why not, with even more direct connection.
Or something more masculine, following the precedent set by Boost.Beast. Like Boost.Wolverine or Boost.Panserbjørn. Glen
On Fri, 20 May 2022 at 14:50, Glen Fernandes via Boost <boost@lists.boost.org> wrote:
On Fri, May 20, 2022 at 7:15 AM Dominique Devienne wrote:
On Fri, May 20, 2022 at 1:03 PM Jonathan Wakely wrote:
I recommend an original name for the project,
I propose Boost.Mya :) Continues the "tradition" introduced by Boost.Hana: Or Boost.Maria instead, why not, with even more direct connection.
Or something more masculine
MySQL is named after co-founder Monty Widenius's daughter, My. (...) The name of the MySQL Dolphin (our logo) is “Sakila,” (...) the feminine name Sakila has its roots in SiSwati (...) So, Boost.My Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
On Fri, 20 May 2022 at 15:18, Mateusz Loskot <mateusz@loskot.net> wrote:
On Fri, 20 May 2022 at 14:50, Glen Fernandes wrote:
On Fri, May 20, 2022 at 7:15 AM Dominique Devienne wrote:
On Fri, May 20, 2022 at 1:03 PM Jonathan Wakely wrote:
I recommend an original name for the project,
I propose Boost.Mya :) Continues the "tradition" introduced by Boost.Hana: Or Boost.Maria instead, why not, with even more direct connection.
Or something more masculine
From https://dev.mysql.com/doc/refman/8.0/en/history.html
MySQL is named after co-founder Monty Widenius's daughter, My. (...) The name of the MySQL Dolphin (our logo) is “Sakila,” (...) the feminine name Sakila has its roots in SiSwati (...)
So, Boost.My
Ha! Boost.Sakila is what I meant to type, of course :D Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
Hi all, thanks for the naming proposals. Norvald, who works at Oracle, has passed our query down to Oracle's legal team. Let's wait for their answer before making a decision here. The library will likely need breaking changes before it gets into Boost, so I wouldn't say there's a hurry. Regards, Ruben.
Hi all, thanks for the naming proposals. Norvald, who works at Oracle, has passed our query down to Oracle's legal team. Let's wait for their answer before making a decision here. The library will likely need breaking changes before it gets into Boost, so I wouldn't say there's a hurry.
Doesn't the common protocol have a name distinct from MySQL and MariaDB? Like the "X Protocol" has its own name distinct from MySQL. -- Alan Freitas
Hana is a good name. I think the name here should give some notion of “connection”, “commit”, “use-with”, or “alongside.” Following Hana’s naming convention, I found a few things on google translate: Alma (Spanish, store/stock) Indu (Latin, put toward) Erga (Latin, toward) Lager (German, warehouse) Junto (Spanish, alongside) Duda (Spanish, alongside) WL
On May 20, 2022, at 3:14 PM, Alan de Freitas via Boost <boost@lists.boost.org> wrote:
Hi all, thanks for the naming proposals. Norvald, who works at Oracle, has passed our query down to Oracle's legal team. Let's wait for their answer before making a decision here. The library will likely need breaking changes before it gets into Boost, so I wouldn't say there's a hurry.
Doesn't the common protocol have a name distinct from MySQL and MariaDB? Like the "X Protocol" has its own name distinct from MySQL.
-- Alan Freitas
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Boost.TheirSQL also fits WL
On May 22, 2022, at 10:23 AM, William Linkmeyer <wlink10@gmail.com> wrote:
Hana is a good name. I think the name here should give some notion of “connection”, “commit”, “use-with”, or “alongside.” Following Hana’s naming convention, I found a few things on google translate:
Alma (Spanish, store/stock) Indu (Latin, put toward) Erga (Latin, toward) Lager (German, warehouse) Junto (Spanish, alongside) Duda (Spanish, alongside)
WL
On May 20, 2022, at 3:14 PM, Alan de Freitas via Boost <boost@lists.boost.org> wrote:
Hi all, thanks for the naming proposals. Norvald, who works at Oracle, has passed our query down to Oracle's legal team. Let's wait for their answer before making a decision here. The library will likely need breaking changes before it gets into Boost, so I wouldn't say there's a hurry.
Doesn't the common protocol have a name distinct from MySQL and MariaDB? Like the "X Protocol" has its own name distinct from MySQL.
-- Alan Freitas
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
OurSQL and TheirSQL are the absolute winners. Puns aside, I'd like the word SQL somewhere. I think a "beautiful" name like Phoenix or Hana isn't very pragmatic - that's why I chose just MySQL. Ruben.
Lager is e.g. the German word for (the concept of) concentration camp, used in Italian among others. Some caution is due when dealing with foreign languages. N On Sun 22 May 2022, 16:23 William Linkmeyer via Boost, < boost@lists.boost.org> wrote:
Hana is a good name. I think the name here should give some notion of “connection”, “commit”, “use-with”, or “alongside.” Following Hana’s naming convention, I found a few things on google translate:
Alma (Spanish, store/stock) Indu (Latin, put toward) Erga (Latin, toward) Lager (German, warehouse) Junto (Spanish, alongside) Duda (Spanish, alongside)
WL
On May 20, 2022, at 3:14 PM, Alan de Freitas via Boost < boost@lists.boost.org> wrote:
Hi all, thanks for the naming proposals. Norvald, who works at Oracle, has passed our query down to Oracle's legal team. Let's wait for their answer before making a decision here. The library will likely need breaking changes before it gets into Boost, so I wouldn't say there's a hurry.
Doesn't the common protocol have a name distinct from MySQL and MariaDB? Like the "X Protocol" has its own name distinct from MySQL.
-- Alan Freitas
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Am 22.05.2022 um 19:41 schrieb Nicolai Egjar Engesland via Boost:
Lager is e.g. the German word for (the concept of) concentration camp,
[OT] It is not. It denotes the general concept of "storage of assets" or "the location where assets are stored". In much former times, centuries ago, it also meant (in its verb form) to "raise a tempory camp for soldiers, the people in their treck, and their assets" before moving on again. It is a common word in contemporary German for everything that satisifies the general concept of "storage". [/OT]
used in Italian among others. Some caution is due when dealing with foreign languages.
N
On Sun 22 May 2022, 16:23 William Linkmeyer via Boost, < boost@lists.boost.org> wrote:
Hana is a good name. I think the name here should give some notion of “connection”, “commit”, “use-with”, or “alongside.” Following Hana’s naming convention, I found a few things on google translate:
Alma (Spanish, store/stock) Indu (Latin, put toward) Erga (Latin, toward) Lager (German, warehouse) Junto (Spanish, alongside) Duda (Spanish, alongside)
WL
On May 20, 2022, at 3:14 PM, Alan de Freitas via Boost < boost@lists.boost.org> wrote:
Hi all, thanks for the naming proposals. Norvald, who works at Oracle, has passed our query down to Oracle's legal team. Let's wait for their answer before making a decision here. The library will likely need breaking changes before it gets into Boost, so I wouldn't say there's a hurry.
Doesn't the common protocol have a name distinct from MySQL and MariaDB? Like the "X Protocol" has its own name distinct from MySQL.
-- Alan Freitas
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes:http://lists.boost.org/mailman/listinfo.cgi/boost
-- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5
participants (13)
-
Alan de Freitas -
Daniela Engert -
Dominique Devienne -
Glen Fernandes -
Jonathan Wakely -
Mateusz Loskot -
Nicolai Egjar Engesland -
Peter Dimov -
Phil Endecott -
Rainer Deyke -
Ruben Perez -
Vinnie Falco -
William Linkmeyer