FIX - Issue #408 - Column[ZonedDateTime] loses nanoseconds precision

[rrramiro]

Pull Request Checklist

• Have you read How to write the perfect pull request?
• Have you read through the contributor guidelines?
• Have you signed the Lightbend CLA?
• Have you squashed your commits?
• Have you added copyright headers to new files?
• Have you checked that both Scala and Java APIs are updated?
• Have you updated the documentation for both Scala and Java sections?
• Have you added tests for any changed functionality?

Fixes

Fixes #408

Purpose

Fix the default instance of Column[ZonedDateTime] in the JavaTimeColumn trait, so it doesn't lose the nanosecond precision during the SQL result reads.

Background Context

When reading an SQL result, TIMESTAMP[TZ] from database is mapped on the java.sql.Timestamp type. This type extends from java.util.Date but has a nanosecond precision. In current implementation the conversion is handle by temporalValueTo function:

  private def temporalValueTo[T](epoch: Long => T, description: String)(value: Any, meta: MetaDataItem): Either[SqlRequestError, T] = {
    val MetaDataItem(qualified, _, _) = meta
    value match {
      case date: java.util.Date => Right(epoch(date.getTime)) // Timestamp conversion
      case time: Long => Right(epoch(time))
      case TimestampWrapper1(ts) => Ts(ts)(t => epoch(t.getTime))
      case TimestampWrapper2(ts) => Ts(ts)(t => epoch(t.getTime))
      case _ => Left(TypeDoesNotMatch(s"Cannot convert $value: ${className(value)} to $description for column $qualified"))
    }
  }

java.sql.Timestamp is matched as a java.util.Date and date.getTime only provides milliseconds.

There was already a fix for the Instant type:

  implicit val columnToInstant: Column[Instant] = nonNull { (value, meta) =>
    val MetaDataItem(qualified, _, _) = meta

    value match {
      case date: LocalDateTime => Right(date.toInstant(ZoneOffset.UTC))
      case ts: java.sql.Timestamp => Ts(ts)(_.toInstant) //Timestamp conversion with nanoseconds
      case date: java.util.Date => Right(Instant ofEpochMilli date.getTime)
      case time: Long => Right(Instant ofEpochMilli time)
      case TimestampWrapper1(ts) => Ts(ts)(_.toInstant)
      case TimestampWrapper2(ts) => Ts(ts)(_.toInstant)
      case _ => Left(TypeDoesNotMatch(s"Cannot convert $value: ${className(value)} to Java8 Instant for column $qualified"))
    }
  }

My suggestion is to reuse this fix and to use Instant as the intermediate type for date/time conversion instead of Long.
So that all conversion will have to provide a function Instant => T when it was Long => T.

References

Column[Instant] fix #325 and PR #326

[lightbend-cla-validator]

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

[rrramiro]

Thanks for the review @cchantep. You're right no need to use Instant for LocalDateTime

[markarasev]

Is it me or the tested instant has a millisecond precision instead of nanosecond?

[rrramiro]

The tested instant has 441477 microseconds. The issue was happening for all the presisions under millisecond. So the test is not wrong, just the naming.