pjfanning commented on issue #555: URL: https://github.com/apache/pekko-persistence-r2dbc/issues/555#issuecomment-6721684261
Thanks for the report. I checked it against `main` and the r2dbc-mysql 1.4.3 jar. **1. `r2dbc-postgresql` needed on the classpath: confirmed, this is a bug.** `r2dbc-postgresql` has been `provided` since 1.1.0 (#173), but `ConnectionFactoryProvider.createConnectionFactory` always sets `PostgresqlConnectionFactoryProvider.FORCE_BINARY`, `PREFER_ATTACHED_BUFFERS` and `PREPARED_STATEMENT_CACHE_QUERIES` (and `STATEMENT_TIMEOUT` when it is set) for every non-H2 driver, before the MySQL/MariaDB checks. Reading those static fields needs the PostgreSQL driver's classes. It affects MySQL and MariaDB; H2 takes a separate path. Our tests don't catch it because every test classpath includes the PostgreSQL driver. A fix should only set the PostgreSQL options for the PostgreSQL driver and add a test that creates a MySQL/MariaDB connection factory with a class loader that doesn't have `io.r2dbc.postgresql`. **2 and 3. MySQL SSL options: accurate, but I see this as a missing feature rather than a bug.** `ssl.mode` is documented in `reference.conf` as `PostgresqlConnectionFactoryProvider.SSL_MODE` with the PostgreSQL values, plus a MariaDB note. MariaDB already gets its own option mapping (`MariaDBConnections.sslOptions`), and MySQL needs the same: - The driver reads `sslMode` as its own `SslMode` (`DISABLED`, `PREFERRED`, `REQUIRED`, `VERIFY_CA`, `VERIFY_IDENTITY`, `TUNNEL`). Even a valid PostgreSQL mode such as `require` reaches it as a PostgreSQL `SSLMode` value, so `ssl.mode` can't really be used with MySQL today, not just the MySQL-named modes. - The driver reads `sslCa` and `sslKeyPassword`, so `ssl.root-cert` and `ssl.password` are ignored. `ssl.cert` and `ssl.key` already work because the names match (`sslCert` and `sslKey`). Things a fix for 2 and 3 should settle: - Whether `ssl.mode` takes the PostgreSQL names translated to MySQL ones, as the MariaDB mapping does (e.g. `verify-full` -> `VERIFY_IDENTITY`), MySQL's own names, or both. The docs and `reference.conf` need updating either way. - `ssl.enabled = on` without a mode currently just sets `ssl=true`, which works with MySQL, and that should keep working. - SSL options set in the `url` or a `connection-factory-options-customizer` should still take precedence, as they do for the MySQL time zone options. It might be cleaner to track 1 (bug) and 2/3 (enhancement) separately, but one PR with separate commits would also be fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
