From e4900fe914e17388e558d8c04f302940c7e280c5 Mon Sep 17 00:00:00 2001 From: Sam Guymer Date: Sat, 27 Aug 2022 14:37:43 +1000 Subject: [PATCH] Better Java time instances for PostgreSQL Given that the Postgres JDBC driver does not return *WithTimezone types, change the type when creating meta column and parameter objects. This allows the Java time instance for `OffsetDateTime` to be defined as requiring a column to be a `TimestampWithTimezone`. --- .../src/main/scala/doobie/util/analysis.scala | 19 ++++++ .../doobie/postgres/JavaTimeInstances.scala | 24 ++++---- .../scala/doobie/postgres/CheckSuite.scala | 60 +++++++++++-------- .../scala/doobie/postgres/TypesSuite.scala | 4 -- 4 files changed, 65 insertions(+), 42 deletions(-) diff --git a/modules/core/src/main/scala/doobie/util/analysis.scala b/modules/core/src/main/scala/doobie/util/analysis.scala index 3084a8050..e0d06696e 100644 --- a/modules/core/src/main/scala/doobie/util/analysis.scala +++ b/modules/core/src/main/scala/doobie/util/analysis.scala @@ -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 + case t => t + } sealed trait AlignmentError extends Product with Serializable { def tag: String diff --git a/modules/postgres/src/main/scala/doobie/postgres/JavaTimeInstances.scala b/modules/postgres/src/main/scala/doobie/postgres/JavaTimeInstances.scala index 059448760..71b5b0d27 100644 --- a/modules/postgres/src/main/scala/doobie/postgres/JavaTimeInstances.scala +++ b/modules/postgres/src/main/scala/doobie/postgres/JavaTimeInstances.scala @@ -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 { @@ -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(_, _)) /** @@ -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 */ @@ -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(_, _)) + } diff --git a/modules/postgres/src/test/scala/doobie/postgres/CheckSuite.scala b/modules/postgres/src/test/scala/doobie/postgres/CheckSuite.scala index a3b3ffc7a..47644629c 100644 --- a/modules/postgres/src/test/scala/doobie/postgres/CheckSuite.scala +++ b/modules/postgres/src/test/scala/doobie/postgres/CheckSuite.scala @@ -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 { @@ -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") @@ -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") @@ -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") @@ -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") @@ -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) @@ -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 = { @@ -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) - } - } diff --git a/modules/postgres/src/test/scala/doobie/postgres/TypesSuite.scala b/modules/postgres/src/test/scala/doobie/postgres/TypesSuite.scala index 3e5bf8c9a..f713f9e72 100644 --- a/modules/postgres/src/test/scala/doobie/postgres/TypesSuite.scala +++ b/modules/postgres/src/test/scala/doobie/postgres/TypesSuite.scala @@ -157,10 +157,6 @@ class TypesSuite extends munit.ScalaCheckSuite { _.`with`(NANO_OF_SECOND, 0) .withOffsetSameInstant(ZoneOffset.UTC) ) - testInOutWithCustomTransform[java.time.ZonedDateTime]("timestamptz")( - _.`with`(NANO_OF_SECOND, 0) - .withZoneSameInstant(ZoneOffset.UTC) - ) /* local date & time (not an instant in time)