FIX - Issue 325 - Column[Instant] loose nano seconds precision

[FXHibon]

Pull Request Checklist

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

Fixes

Fixes #325

Purpose

Fix the default instance of Column[Instant] in the JavaTimeColumn trait, so we don't loose the nano second 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. Therefore, the Column[Instant] should be able to transform a given java.sql.Timestamp into an java.time.Instant

As java.sql.Timestamp inherits java.util.Date, the first branch of following pattern-matching will be selected

   value match {
      case date: java.util.Date => Right(Instant ofEpochMilli date.getTime)
      case time: Long => Right(Instant ofEpochMilli time)
      case TimestampWrapper1(ts) => Ts(ts)(Instant ofEpochMilli _.getTime)
      case TimestampWrapper2(ts) => Ts(ts)(Instant ofEpochMilli _.getTime)
      case _ => Left(TypeDoesNotMatch(s"Cannot convert $value: ${className(value)} to Java8 Instant for column $qualified"))
    }

https://github.com/playframework/anorm/blob/master/core/src/main/scala/anorm/Column.scala#L715

The problem is that java.util.Date is precised at the miliseconds level, and not the nano seconds level. Dealing with a java.sql.Timestamp as as java.util.Date will loose this precision, as we can understand immediatly with the following code:

Instant ofEpochMilli date.getTime

(getTime returns EPOCH miliseconds, as explained in javadoc)

I wonder why the pattern-matching wasn't using directly the java.sql.Timestamp type? Maybe is there an hidden reason I couldn't understand?

Adding a line (in first position) with a pattern on java.sql.Timestamp seems to be ok, especially since Timestamp can be directly converted to Instant with the method .toInstant.

The 2 lines that matches TimestampWrapper1 and TimestampWrapper2 would loose in the same way, because it build the Instant using only the miliseconds.

I took this opportunty to use ts.toInstant here to, to avoid silent data loss behavior.

Actually, I'm not sure to understand when these TimestampWrapper become useful, but I might not have the full picture I my mind.

References

The issue is explained in details here #325

[cchantep]

For history, this test without code change:

[error]   x be parsed from timestamp with nano precision
[error]    parsed instant '2021-01-06T08:45:26.441Z != 2021-01-06T08:45:26.441477Z' (JavaTimeSpec.scala:46)
[error] Actual:   ...6.441[]Z
[error] Expected: ...6.441[477]Z

[cchantep]

Thanks for your contribution

[FXHibon]

hello @cchantep
Would you be able to release a version 2.6.9 of anorm that would include this PR?

[cchantep]

Rather 2.6.10

[cchantep]

https://search.maven.org/artifact/org.playframework.anorm/anorm_2.12/2.6.9/jar