pjfanning commented on code in PR #537:
URL:
https://github.com/apache/pekko-persistence-r2dbc/pull/537#discussion_r4455021212
##########
core/src/main/scala/org/apache/pekko/persistence/r2dbc/ConnectionFactoryProvider.scala:
##########
@@ -197,10 +200,20 @@ class ConnectionFactoryProvider(system: ActorSystem[?])
extends Extension {
builder.option(PostgresqlConnectionFactoryProvider.SSL_PASSWORD,
settings.sslPassword)
}
- if (settings.driver == "mysql") {
- // Either `connectionTimeZone = SERVER` or
`forceConnectionTimeZoneToSession = true` need to be set for timezones to work
correctly,
- // likely caused by bug in
https://github.com/asyncer-io/r2dbc-mysql/pull/240.
- builder.option(Option.valueOf("connectionTimeZone"), "SERVER")
+ if (mySQL) {
+ // Instants are converted with the connection time zone, which must be
the session time zone for timestamps to be
+ // stored correctly, see
https://github.com/asyncer-io/r2dbc-mysql/pull/240. UTC has no ambiguous local
times when
+ // daylight saving time ends, and the server time zone may not be
resolvable by the driver (e.g. `EDT`), see
+ // https://github.com/apache/pekko-persistence-r2dbc/issues/533.
+ // Unlike the MariaDB time zone, these are not enforced on purpose: rows
written by earlier versions may be stored
+ // shifted, and the `url` or a customizer can keep the previous options
until the data is corrected, see the
+ // migration guide.
+ val connectionTimeZone = Option.valueOf[String]("connectionTimeZone")
+ val forceConnectionTimeZoneToSession =
Option.valueOf[java.lang.Boolean]("forceConnectionTimeZoneToSession")
+ if (!driverOptions.hasOption(connectionTimeZone))
+ builder.option(connectionTimeZone, "UTC")
+ if (!driverOptions.hasOption(forceConnectionTimeZoneToSession))
+ builder.option(forceConnectionTimeZoneToSession,
java.lang.Boolean.TRUE)
Review Comment:
Nit: the two options are only kept from the `url` independently. A `url`
with just `forceConnectionTimeZoneToSession=false` still gets
`connectionTimeZone=UTC` from the plugin, so the driver converts with UTC while
the session stays in the server time zone, and timestamps are stored shifted
without any warning. The `getting-started.md` note covers this, since the
connection time zone must also be the session time zone. Still, it might be
safer to set neither option when the `url` sets either one, or to log a warning
in that case.
--
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]
