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

Better Java time instances for PostgreSQL #1736

Merged
merged 1 commit into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions modules/core/src/main/scala/doobie/util/analysis.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,28 @@ object analysis {

/** Metadata for the JDBC end of a column/parameter mapping. */
final case class ColumnMeta(jdbcType: JdbcType, vendorTypeName: String, nullability: Nullability, name: String)
object ColumnMeta {
def apply(jdbcType: JdbcType, vendorTypeName: String, nullability: Nullability, name: String): ColumnMeta = {
new ColumnMeta(tweakJdbcType(jdbcType, vendorTypeName), vendorTypeName, nullability, name)
}
}

/** Metadata for the JDBC end of a column/parameter mapping. */
final case class ParameterMeta(jdbcType: JdbcType, vendorTypeName: String, nullability: Nullability, mode: ParameterMode)
object ParameterMeta {
def apply(jdbcType: JdbcType, vendorTypeName: String, nullability: Nullability, mode: ParameterMode): ParameterMeta = {
new ParameterMeta(tweakJdbcType(jdbcType, vendorTypeName), vendorTypeName, nullability, mode)
}
}

private def tweakJdbcType(jdbcType: JdbcType, vendorTypeName: String) = jdbcType match {
// the Postgres driver does not return *WithTimezone types but they are pretty much required for proper analysis
// https://github.com/pgjdbc/pgjdbc/issues/2485
// https://github.com/pgjdbc/pgjdbc/issues/1766
case JdbcType.Time if vendorTypeName.compareToIgnoreCase("timetz") == 0 => JdbcType.TimeWithTimezone
case JdbcType.Timestamp if vendorTypeName.compareToIgnoreCase("timestamptz") == 0 => JdbcType.TimestampWithTimezone
guymers marked this conversation as resolved.
Show resolved Hide resolved
case t => t
}

sealed trait AlignmentError extends Product with Serializable {
def tag: String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import java.time.{OffsetDateTime, ZoneOffset} // Using database JDBC driver nati
/**
* Instances for JSR-310 date time types.
*
* Implementation is based on https://jdbc.postgresql.org/documentation/head/8-date-time.html, using
* Implementation is based on https://jdbc.postgresql.org/documentation/head/java8-date-time.html, using
* native support for Postgres JDBC driver.
*/
trait JavaTimeInstances extends MetaConstructors {
Expand All @@ -26,8 +26,8 @@ trait JavaTimeInstances extends MetaConstructors {
*/
implicit val JavaTimeOffsetDateTimeMeta: Meta[java.time.OffsetDateTime] =
Basic.one[java.time.OffsetDateTime](
JT.Timestamp,
List(JT.Time),
JT.TimestampWithTimezone,
List(JT.Timestamp, JT.TimeWithTimezone),
_.getObject(_, classOf[java.time.OffsetDateTime]), _.setObject(_, _), _.updateObject(_, _))

/**
Expand All @@ -36,15 +36,6 @@ trait JavaTimeInstances extends MetaConstructors {
implicit val JavaTimeInstantMeta: Meta[java.time.Instant] =
JavaTimeOffsetDateTimeMeta.timap(_.toInstant)(OffsetDateTime.ofInstant(_, ZoneOffset.UTC))

/**
* This type should map to TIMESTAMP WITH TIMEZONE (TIMESTAMPTZ)
* When writing to the database, the same instant is preserved if your target column is of type TIMESTAMPTZ
* (The JDBC driver works out the timezone conversion for you). Note that since zone information is not stored in
* the database column, retrieving the same value will yield the same instant in time, but in UTC.
*/
implicit val JavaTimeZonedDateTimeMeta: Meta[java.time.ZonedDateTime] =
JavaTimeOffsetDateTimeMeta.timap(_.atZoneSameInstant(ZoneOffset.UTC))(_.toOffsetDateTime)

/**
* This type should map to TIMESTAMP
*/
Expand Down Expand Up @@ -72,4 +63,13 @@ trait JavaTimeInstances extends MetaConstructors {
Nil,
_.getObject(_, classOf[java.time.LocalTime]), _.setObject(_, _), _.updateObject(_, _))

/**
* This type should map to TIME WITH TIMEZONE (TIMETZ)
*/
implicit val JavaTimeOffsetTimeMeta: Meta[java.time.OffsetTime] =
Basic.one[java.time.OffsetTime](
JT.TimeWithTimezone,
Nil,
_.getObject(_, classOf[java.time.OffsetTime]), _.setObject(_, _), _.updateObject(_, _))

}
60 changes: 34 additions & 26 deletions modules/postgres/src/test/scala/doobie/postgres/CheckSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import doobie.implicits._
import doobie.postgres.enums._
import doobie.postgres.implicits._
import doobie.util.analysis.{ColumnTypeError, ColumnTypeWarning, ParameterTypeError}
import java.time.{Instant, LocalDate, LocalDateTime, LocalTime, OffsetDateTime}
import java.time.{Instant, LocalDate, LocalDateTime, LocalTime, OffsetDateTime, OffsetTime}

class CheckSuite extends munit.FunSuite {

Expand All @@ -32,11 +32,11 @@ class CheckSuite extends munit.FunSuite {
successRead[OffsetDateTime](sql"SELECT '2019-02-13T22:03:21.000' :: TIMESTAMPTZ")
successWrite[OffsetDateTime](t, "TIMESTAMPTZ")

successReadUnfortunately[OffsetDateTime](sql"SELECT '2019-02-13T22:03:21.000' :: TIMESTAMP")
successWriteUnfortunately[OffsetDateTime](t, "TIMESTAMP")
warnRead[OffsetDateTime](sql"SELECT '2019-02-13T22:03:21.000' :: TIMESTAMP")
errorWrite[OffsetDateTime](t, "TIMESTAMP")

failedRead[OffsetDateTime](sql"SELECT '2019-02-13T22:03:21.000' :: TEXT")
_warnRead[OffsetDateTime](sql"SELECT '03:21' :: TIME") // driver cannot read but TIME and TIMETZ are returned as the same JDBC type
failedRead[OffsetDateTime](sql"SELECT '03:21' :: TIME")
warnRead[OffsetDateTime](sql"SELECT '03:21' :: TIMETZ")
failedRead[OffsetDateTime](sql"SELECT '2019-02-13' :: DATE")

Expand All @@ -54,11 +54,11 @@ class CheckSuite extends munit.FunSuite {
successRead[Instant](sql"SELECT '2019-02-13T22:03:21.000' :: TIMESTAMPTZ")
successWrite[Instant](t, "TIMESTAMPTZ")

successReadUnfortunately[Instant](sql"SELECT '2019-02-13T22:03:21.000' :: TIMESTAMP")
successWriteUnfortunately[Instant](t, "TIMESTAMP")
warnRead[Instant](sql"SELECT '2019-02-13T22:03:21.000' :: TIMESTAMP")
errorWrite[Instant](t, "TIMESTAMP")

failedRead[Instant](sql"SELECT '2019-02-13T22:03:21.000' :: TEXT")
_warnRead[Instant](sql"SELECT '03:21' :: TIME") // driver cannot read but TIME and TIMETZ are returned as the same JDBC type
failedRead[Instant](sql"SELECT '03:21' :: TIME")
warnRead[Instant](sql"SELECT '03:21' :: TIMETZ")
failedRead[Instant](sql"SELECT '2019-02-13' :: DATE")

Expand All @@ -76,8 +76,8 @@ class CheckSuite extends munit.FunSuite {
successRead[LocalDateTime](sql"SELECT '2019-02-13T22:03:21.051' :: TIMESTAMP")
successWrite[LocalDateTime](t, "TIMESTAMP")

successReadUnfortunately[LocalDateTime](sql"SELECT '2019-02-13T22:03:21.051' :: TIMESTAMPTZ")
successWriteUnfortunately[LocalDateTime](t, "TIMESTAMPTZ")
failedRead[LocalDateTime](sql"SELECT '2019-02-13T22:03:21.051' :: TIMESTAMPTZ")
errorWrite[LocalDateTime](t, "TIMESTAMPTZ")

failedRead[LocalDateTime](sql"SELECT '2019-02-13T22:03:21.051' :: TEXT")
failedRead[LocalDateTime](sql"SELECT '03:21' :: TIME")
Expand All @@ -99,7 +99,7 @@ class CheckSuite extends munit.FunSuite {
successWrite[LocalDate](t, "DATE")

warnRead[LocalDate](sql"SELECT '2015-02-23T01:23:13.000' :: TIMESTAMP")
_warnRead[LocalDate](sql"SELECT '2015-02-23T01:23:13.000Z' :: TIMESTAMPTZ") // driver cannot read but TIMESTAMP and TIMESTAMPTZ are returned as the same JDBC type
failedRead[LocalDate](sql"SELECT '2015-02-23T01:23:13.000Z' :: TIMESTAMPTZ")
failedRead[LocalDate](sql"SELECT '2015-02-23' :: TEXT")
failedRead[LocalDate](sql"SELECT '03:21' :: TIME")
failedRead[LocalDate](sql"SELECT '03:21' :: TIMETZ")
Expand All @@ -121,15 +121,35 @@ class CheckSuite extends munit.FunSuite {
failedRead[LocalTime](sql"SELECT '2015-02-23T01:23:13.000Z' :: TIMESTAMPTZ")
failedRead[LocalTime](sql"SELECT '2015-02-23' :: TEXT")
failedRead[LocalTime](sql"SELECT '2015-02-23' :: DATE")
failedRead[LocalTime](sql"SELECT '23:13' :: TIMETZ")

errorWrite[LocalTime](t, "TEXT")
successWriteUnfortunately[LocalTime](t, "TIMETZ")
errorWrite[LocalTime](t, "TIMETZ")
errorWrite[LocalTime](t, "DATE")

failedRead[LocalTime](sql"SELECT '123' :: BYTEA")
errorWrite[LocalTime](t, "BYTEA")
}

test("OffsetTime Read and Write typechecks") {
val t = OffsetTime.parse("23:13-01:00")
successRead[OffsetTime](sql"SELECT '23:13' :: TIMETZ")
successWrite[OffsetTime](t, "TIMETZ")

failedRead[OffsetTime](sql"SELECT '2015-02-23T01:23:13.000' :: TIMESTAMP")
failedRead[OffsetTime](sql"SELECT '2015-02-23T01:23:13.000Z' :: TIMESTAMPTZ")
failedRead[OffsetTime](sql"SELECT '2015-02-23' :: TEXT")
failedRead[OffsetTime](sql"SELECT '2015-02-23' :: DATE")
failedRead[OffsetTime](sql"SELECT '23:13' :: TIME")

errorWrite[OffsetTime](t, "TEXT")
errorWrite[OffsetTime](t, "TIME")
errorWrite[OffsetTime](t, "DATE")

failedRead[OffsetTime](sql"SELECT '123' :: BYTEA")
errorWrite[OffsetTime](t, "BYTEA")
}

private def successRead[A: Read](frag: Fragment): Unit = {
val analysisResult = frag.query[A].analysis.transact(xa).unsafeRunSync()
assertEquals(analysisResult.columnAlignmentErrors, Nil)
Expand All @@ -145,16 +165,12 @@ class CheckSuite extends munit.FunSuite {
}

private def warnRead[A: Read](frag: Fragment): Unit = {
_warnRead[A](frag)

val result = frag.query[A].unique.transact(xa).attempt.unsafeRunSync()
assert(result.isRight)
}

private def _warnRead[A: Read](frag: Fragment): Unit = {
val analysisResult = frag.query[A].analysis.transact(xa).unsafeRunSync()
val errorClasses = analysisResult.columnAlignmentErrors.map(_.getClass)
assertEquals(errorClasses, List(classOf[ColumnTypeWarning]))

val result = frag.query[A].unique.transact(xa).attempt.unsafeRunSync()
assert(result.isRight)
}

private def failedRead[A: Read](frag: Fragment): Unit = {
Expand All @@ -173,12 +189,4 @@ class CheckSuite extends munit.FunSuite {
assertEquals(errorClasses, List(classOf[ParameterTypeError]))
}

private def successWriteUnfortunately[A: Put](value: A, dbType: String): Unit = successWrite(value, dbType)

// Some DB types really shouldn't type check but driver is too lenient
private def successReadUnfortunately[A: Read](frag: Fragment): Unit = {
val analysisResult = frag.query[A].analysis.transact(xa).unsafeRunSync()
assertEquals(analysisResult.columnAlignmentErrors, Nil)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ class TypesSuite extends munit.ScalaCheckSuite {
testInOut[java.sql.Timestamp]("timestamptz")
testInOut[java.time.Instant]("timestamptz")
testInOutTweakExpected[java.time.OffsetDateTime]("timestamptz")(_.withOffsetSameInstant(ZoneOffset.UTC)) // +148488-07-03T02:38:17Z != +148488-07-03T00:00-02:38:17
testInOutTweakExpected[java.time.ZonedDateTime]("timestamptz")(_.withZoneSameInstant(ZoneOffset.UTC))

/*
local date & time (not an instant in time)
Expand Down