Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OffsetDateTime parsing is broken for MySQL datetime column #1984

Open
sjbog opened this issue Feb 4, 2019 · 9 comments
Open

OffsetDateTime parsing is broken for MySQL datetime column #1984

sjbog opened this issue Feb 4, 2019 · 9 comments
Assignees

Comments

@sjbog
Copy link

sjbog commented Feb 4, 2019

DESCRIPTION:

MySQL DATETIME column can't be parsed as ISO 8601 DateTimeFormatter.ISO_OFFSET_DATE_TIME

ISO format refers to ISO 9075, not ISO 8601

https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_get-format
https://stackoverflow.com/a/28323923

https://github.com/slick/slick/blob/v3.3.0/slick/src/main/scala/slick/jdbc/JdbcTypesComponent.scala#L327

STEPS TO REPRODUCE

MySQL v5.7, Slick v3.3.0

profile = "slick.jdbc.MySQLProfile$"
driver = "org.mariadb.jdbc.Driver"
connectionPool = HikariCP

Table

CREATE TABLE `abc` (
   ...
  `created_date`  DATETIME     NOT NULL DEFAULT CURRENT_TIMESTAMP,
   ...
)

class TableAbc(tag: Tag) extends Table[EntryType](tag, "abc") {
  val createdDate  = column[OffsetDateTime]("created_date")
  ...
}

RESULT

MySQL returns datetime values in ISO 9075. For example 2016-08-11 20:24:18.0

java.time.format.DateTimeParseException: Text '2016-08-11 20:24:18.0' could not be parsed at index 10
	at java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2049)
	at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1951)
	at java.base/java.time.OffsetDateTime.parse(OffsetDateTime.java:402)
	at java.base/java.time.OffsetDateTime.parse(OffsetDateTime.java:387)
	at slick.jdbc.JdbcTypesComponent$JdbcTypes$OffsetDateTimeJdbcType.getValue(JdbcTypesComponent.scala:327)
	at slick.jdbc.JdbcTypesComponent$JdbcTypes$OffsetDateTimeJdbcType.getValue(JdbcTypesComponent.scala:309)
	at slick.jdbc.SpecializedJdbcResultConverter$$anon$1.read(SpecializedJdbcResultConverters.scala:26)
	at slick.jdbc.SpecializedJdbcResultConverter$$anon$1.read(SpecializedJdbcResultConverters.scala:24)
	at slick.relational.ProductResultConverter.read(ResultConverter.scala:53)
	at slick.relational.ProductResultConverter.read(ResultConverter.scala:43)
	at slick.relational.TypeMappingResultConverter.read(ResultConverter.scala:133)
	at slick.relational.TypeMappingResultConverter.read(ResultConverter.scala:133)
	at slick.jdbc.JdbcInvokerComponent$QueryInvokerImpl.extractValue(JdbcInvokerComponent.scala:36)
	at slick.jdbc.StatementInvoker$$anon$2.extractValue(StatementInvoker.scala:66)
	at slick.jdbc.PositionedResultIterator.fetchNext(PositionedResult.scala:176)
	at slick.util.ReadAheadIterator.update(ReadAheadIterator.scala:28)
	at slick.util.ReadAheadIterator.hasNext(ReadAheadIterator.scala:34)
	at slick.util.ReadAheadIterator.hasNext$(ReadAheadIterator.scala:33)
	at slick.jdbc.PositionedResultIterator.hasNext(PositionedResult.scala:167)
	at scala.collection.Iterator.foreach(Iterator.scala:941)
	at scala.collection.Iterator.foreach$(Iterator.scala:941)
	at slick.jdbc.PositionedResultIterator.foreach(PositionedResult.scala:167)
	at slick.jdbc.Invoker.foreach(Invoker.scala:48)
	at slick.jdbc.Invoker.foreach$(Invoker.scala:46)
	at slick.jdbc.StatementInvoker.foreach(StatementInvoker.scala:15)
	at slick.jdbc.StreamingInvokerAction.run(StreamingInvokerAction.scala:22)
	at slick.jdbc.StreamingInvokerAction.run$(StreamingInvokerAction.scala:20)
	at slick.jdbc.JdbcActionComponent$QueryActionExtensionMethodsImpl$$anon$2.run(JdbcActionComponent.scala:216)
	at slick.jdbc.JdbcActionComponent$QueryActionExtensionMethodsImpl$$anon$2.run(JdbcActionComponent.scala:216)
	at slick.dbio.SynchronousDatabaseAction$FusedAndThenAction.$anonfun$run$4(DBIOAction.scala:533)
	at slick.dbio.SynchronousDatabaseAction$FusedAndThenAction.$anonfun$run$4$adapted(DBIOAction.scala:533)
	at scala.collection.Iterator.foreach(Iterator.scala:941)
	at scala.collection.Iterator.foreach$(Iterator.scala:941)
	at scala.collection.AbstractIterator.foreach(Iterator.scala:1429)
	at scala.collection.IterableLike.foreach(IterableLike.scala:74)
	at scala.collection.IterableLike.foreach$(IterableLike.scala:73)
	at scala.collection.AbstractIterable.foreach(Iterable.scala:56)
	at slick.dbio.SynchronousDatabaseAction$FusedAndThenAction.run(DBIOAction.scala:533)
	at slick.dbio.SynchronousDatabaseAction$$anon$6.run(DBIOAction.scala:469)
	at slick.basic.BasicBackend$DatabaseDef$$anon$3.liftedTree1$1(BasicBackend.scala:275)
	at slick.basic.BasicBackend$DatabaseDef$$anon$3.run(BasicBackend.scala:275)

Possible solutions

  1. Use resultSet.getTimestamp as parsing base instead of getString.
  implicit val instantMapper = MappedColumnType.base[Instant, Timestamp](
    ins => Timestamp.from(ins),
    ts => ts.toInstant
  )

  implicit val localDateTimeMapper = MappedColumnType.base[LocalDateTime, Timestamp](
    ldt => Timestamp.valueOf(ldt),
    ts => ts.toLocalDateTime
  )

  implicit val zonedDateTimeMapper = MappedColumnType.base[ZonedDateTime, Timestamp](
    zdt => Timestamp.valueOf(zdt.toLocalDateTime),
    ts => ZonedDateTime.of(ts.toLocalDateTime, ZoneId.systemDefault())
  )

  implicit val offsetDateTimeMapper = MappedColumnType.base[OffsetDateTime, Timestamp](
    odt => Timestamp.valueOf(odt.toLocalDateTime),
    ts => OffsetDateTime.ofInstant(Instant.ofEpochMilli(ts.getTime), ZoneOffset.UTC)
  )
  1. Use DATE_FORMAT in queries
@hvesalai
Copy link
Member

hvesalai commented Feb 4, 2019

@smootoo is the expert here, but AFAIK this code is not meant to work with MySQL DATETIME, such as 2016-08-11 20:24:18.0, as it doesn't have a required timezone offset.

If that is the case, then this issue should be closed as not a bug.

@hvesalai hvesalai added this to the Future fix release milestone Feb 4, 2019
@hvesalai
Copy link
Member

hvesalai commented Feb 4, 2019

@sjbog the data type that corresponds to MySQL DATETIME is LocalDateTime

@sjbog
Copy link
Author

sjbog commented Feb 4, 2019

@hvesalai the bug I see - is an assumption that OffsetDateTime column is ISO 8601 string

case iso8601String : String => OffsetDateTime.parse(iso8601String)

Another solution would be to remove offsetDateTimeColumnType implicit for SQL profiles w/o TIMESTAMP WITH OFFSET database support

@hvesalai
Copy link
Member

hvesalai commented Feb 4, 2019

@sjbog The solution taken here is to use a VARCHAR column for OffsetDateTime (see https://github.com/slick/slick/blob/v3.3.0/slick/src/main/scala/slick/jdbc/JdbcTypesComponent.scala#L319).

In your case the database has not been created by Slick and that is what is causing the mismatch.

So the column would be in ISO 8601 if the database schema was of the expected type and the values had been written by slick.

@hvesalai
Copy link
Member

hvesalai commented Feb 4, 2019

The missing documentation on this feature is here: https://github.com/slick/slick/pull/1985/files

@mfulgo
Copy link

mfulgo commented Oct 2, 2020

In your case the database has not been created by Slick and that is what is causing the mismatch.

In my experience, that is often the case for production systems: Slick doesn't provide a good story for database migrations. So, you're left with using another tool like Flyway or Liquibase to create the tables. Given the 2038 limit for TIMESTAMP columns, I'd expect an increasing number of users will be migrating to DATETIME columns.

It would be great if Slick had a recommended way of supporting that scenario.

@phwiget
Copy link

phwiget commented Mar 4, 2022

Maybe some more solutions:


import slick.ast.{AnonSymbol, NullaryNode, ScalaBaseType, SimplyTypedNode}

import java.sql.{Time, Timestamp}
import java.time.{LocalDateTime, LocalTime}

/**
 * Custom MySQL profile to handle `DATETIME` and `TIME` columns. The default MySQL-Profile will work with strings instead, which is not optimal.
 */
trait MySQL8Profile extends slick.jdbc.MySQLProfile {

  import java.sql.{PreparedStatement, ResultSet}

  override val columnTypes = new JdbcTypes

  class JdbcTypes extends super.JdbcTypes {

    override val localDateTimeType : LocalDateTimeJdbcType = new LocalDateTimeJdbcType {
      override def sqlType : Int = {
        /**
         * [[LocalDateTime]] will be persisted as a [[java.sql.Types.TIMESTAMP]] in contrast to the default MySQL-Profile.
         */
        java.sql.Types.TIMESTAMP
      }
      override def setValue(v: LocalDateTime, p: PreparedStatement, idx: Int) : Unit = {
        p.setTimestamp(idx, if (v == null) null else Timestamp.valueOf(v))
      }

      override def getValue(r: ResultSet, idx: Int) : LocalDateTime = {
        r.getTimestamp(idx) match {
          case null => null
          case timestamp => timestamp.toLocalDateTime
        }
      }

      override def updateValue(v: LocalDateTime, r: ResultSet, idx: Int): Unit = {
        r.updateTimestamp(idx, if (v == null) null else Timestamp.valueOf(v))
      }

    }

    class LocalTimeJdbcType extends DriverJdbcType[LocalTime] {

      override def sqlType : Int = {
        /**
         * [[LocalTime]] will be persisted as a [[java.sql.Types.TIME]] in contrast to the default MySQL-Profile.
         */
        java.sql.Types.TIME
      }

      override def setValue(v: LocalTime, p: PreparedStatement, idx: Int) : Unit = {
        p.setTime(idx, if (v == null) null else  Time.valueOf(v))
      }

      override def getValue(r: ResultSet, idx: Int) : LocalTime = {
        r.getTime(idx) match {
          case null => null
          case time => time.toLocalTime
        }
      }

      override def updateValue(v: LocalTime, r: ResultSet, idx: Int): Unit = {
        r.updateTime(idx, if (v == null) null else  Time.valueOf(v))
      }

    }

  }

}

object MySQL8Profile extends MySQL8Profile {
  final case class RowNum(sym: AnonSymbol, inc: Boolean) extends NullaryNode with SimplyTypedNode {
    type Self = RowNum
    def buildType = ScalaBaseType.longType
    def rebuild = copy()
  }

  final case class RowNumGen(sym: AnonSymbol, init: Long) extends NullaryNode with SimplyTypedNode {
    type Self = RowNumGen
    def buildType = ScalaBaseType.longType
    def rebuild = copy()
  }
}


  • Or add a custom mapper to the slick table definitions (the important point is to add the mapper explicitly to override the default behaviour):
  def date  = column[LocalDateTime]("date", O.SqlType("DATETIME"))(DateSupport.localDateTimeToTimestamp)

...

  implicit val localDateTimeToTimestamp = MappedColumnType.base[LocalDateTime, Timestamp](
    l => Timestamp.valueOf(l),
    d => d.toLocalDateTime
  )

@nafg
Copy link
Member

nafg commented Mar 7, 2022

I'm confused. Is the issue specific to OffsetDateTime or does it apply to LocalDateTime?

@phwiget
Copy link

phwiget commented Mar 7, 2022

According to the source code it applies to Instant and LocalDateTime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants