From fc6f8ce7d041ee4c363b1eabfdfe59731546a704 Mon Sep 17 00:00:00 2001 From: Olivier Deckers <1965514+olivierdeckers@users.noreply.github.com> Date: Fri, 5 Apr 2024 16:33:01 +0200 Subject: [PATCH 1/4] Don't discard the timezone information when reading timezone with timestamp from the database --- core/src/main/scala/zio/jdbc/JdbcDecoder.scala | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/zio/jdbc/JdbcDecoder.scala b/core/src/main/scala/zio/jdbc/JdbcDecoder.scala index 9732d122..ce680a66 100644 --- a/core/src/main/scala/zio/jdbc/JdbcDecoder.scala +++ b/core/src/main/scala/zio/jdbc/JdbcDecoder.scala @@ -18,6 +18,7 @@ package zio.jdbc import zio._ import java.io._ +import java.time.{OffsetDateTime, OffsetTime} import java.sql.{ Array => _, _ } import scala.collection.immutable.ListMap @@ -747,16 +748,14 @@ trait JdbcDecoderLowPriorityImplicits { DynamicValue.Primitive(timestamp.toInstant(), StandardType.InstantType) case SqlTypes.TIMESTAMP_WITH_TIMEZONE => - // TODO: Timezone - val timestamp = resultSet.getTimestamp(columnIndex) + val timestamp = resultSet.getObject(columnIndex, classOf[OffsetDateTime]) - DynamicValue.Primitive(timestamp.toInstant(), StandardType.InstantType) + DynamicValue.Primitive(timestamp, StandardType.OffsetDateTimeType) case SqlTypes.TIME_WITH_TIMEZONE => - // TODO: Timezone - val time = resultSet.getTime(columnIndex) + val time = resultSet.getObject(columnIndex, classOf[OffsetTime]) - DynamicValue.Primitive(time.toLocalTime(), StandardType.LocalTimeType) + DynamicValue.Primitive(time, StandardType.OffsetTimeType) case SqlTypes.TINYINT => val short = resultSet.getShort(columnIndex) From 427c02420c8b46869e955f11619197db1c82c40a Mon Sep 17 00:00:00 2001 From: Olivier Deckers <1965514+olivierdeckers@users.noreply.github.com> Date: Fri, 5 Apr 2024 16:46:32 +0200 Subject: [PATCH 2/4] fmt --- core/src/main/scala/zio/jdbc/JdbcDecoder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/zio/jdbc/JdbcDecoder.scala b/core/src/main/scala/zio/jdbc/JdbcDecoder.scala index 16efd29e..315e750e 100644 --- a/core/src/main/scala/zio/jdbc/JdbcDecoder.scala +++ b/core/src/main/scala/zio/jdbc/JdbcDecoder.scala @@ -18,7 +18,7 @@ package zio.jdbc import zio._ import java.io._ -import java.time.{OffsetDateTime, OffsetTime} +import java.time.{ OffsetDateTime, OffsetTime } import java.sql.{ Array => _, _ } import scala.collection.immutable.ListMap From 2a505a4a2835cbbd77df5d3e4d6f402e0301b95a Mon Sep 17 00:00:00 2001 From: Olivier Deckers <1965514+olivierdeckers@users.noreply.github.com> Date: Tue, 16 Apr 2024 22:11:01 +0200 Subject: [PATCH 3/4] Add test using duckdb --- build.sbt | 14 ++-- .../src/test/scala/zio/jdbc/DuckDbSpec.scala | 78 +++++++++++++++++++ .../scala/zio/jdbc/JavaTimeSupportSpec.scala | 6 +- 3 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 integration/src/test/scala/zio/jdbc/DuckDbSpec.scala diff --git a/build.sbt b/build.sbt index a4296eba..998332d1 100644 --- a/build.sbt +++ b/build.sbt @@ -84,11 +84,13 @@ lazy val integration = project publish / skip := true, Test / fork := true, libraryDependencies ++= Seq( - "org.testcontainers" % "postgresql" % "1.19.1" % Test, - "org.postgresql" % "postgresql" % "42.6.0" % Test, - "dev.zio" %% "zio-test" % ZioVersion % Test, - "dev.zio" %% "zio-test-sbt" % ZioVersion % Test, - "org.slf4j" % "slf4j-api" % "2.0.9" % Test, - "org.slf4j" % "slf4j-simple" % "2.0.9" % Test + "org.testcontainers" % "postgresql" % "1.19.1" % Test, + "org.postgresql" % "postgresql" % "42.6.0" % Test, + "org.duckdb" % "duckdb_jdbc" % "0.10.1" % Test, + "dev.zio" %% "zio-schema-derivation" % ZioSchemaVersion % Test, + "dev.zio" %% "zio-test" % ZioVersion % Test, + "dev.zio" %% "zio-test-sbt" % ZioVersion % Test, + "org.slf4j" % "slf4j-api" % "2.0.9" % Test, + "org.slf4j" % "slf4j-simple" % "2.0.9" % Test ) ) diff --git a/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala b/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala new file mode 100644 index 00000000..205df3d8 --- /dev/null +++ b/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala @@ -0,0 +1,78 @@ +package zio.jdbc + +import org.duckdb.DuckDBConnection +import org.testcontainers.jdbc.ConnectionWrapper +import zio._ +import zio.schema.{ DeriveSchema, Schema } +import zio.test.TestAspect.{ repeats, sequential, shrinks, withLiveClock } +import zio.test.{ Gen, Spec, TestEnvironment, ZIOSpec, assertTrue, check } + +import java.sql.{ Array => _, _ } +import java.time.{ OffsetDateTime, ZoneOffset } +import java.time.temporal.ChronoUnit +import java.util.Properties + +object DuckDbSpec extends ZIOSpec[ZConnection] { + + override def bootstrap: ZLayer[Any, Any, ZConnection] = + ZLayer.scoped( + for { + connection <- ZIO.acquireRelease( + ZIO.succeedBlocking { + val props = new Properties() + val duckDb = DriverManager + .getConnection(s"jdbc:duckdb:", props) + .asInstanceOf[DuckDBConnection] + WorkaroundDuckDBConnection(duckDb) + } + )(c => ZIO.succeed(c.close())) + zConnection <- ZConnection.make(connection) + } yield zConnection + ) + + case class OffsetDateTimeRow(value: OffsetDateTime) + object OffsetDateTimeRow { + implicit val schema: Schema[OffsetDateTimeRow] = DeriveSchema.gen[OffsetDateTimeRow] + implicit val jdbcDecoder: JdbcDecoder[OffsetDateTimeRow] = JdbcDecoder.fromSchema + } + + override def spec: Spec[ZConnection with TestEnvironment with Scope, Any] = suite("DuckDB")( + test("should be able to decode case classes with OffsetDateTime fields and handle timezones correctly") { + check( + Gen.offsetDateTime( + OffsetDateTime.of(1970, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC), + OffsetDateTime.of(2100, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC) + ) + ) { offsetDateTime => + for { + _ <- sql"""CREATE TABLE offset_datetime (value TIMESTAMP WITH TIME ZONE)""".execute + _ <- sql"""INSERT INTO offset_datetime VALUES ($offsetDateTime)""".execute + d <- sql"""SELECT value FROM offset_datetime""".query[OffsetDateTimeRow].selectOne + _ <- sql"DROP TABLE offset_datetime".execute + expected = + offsetDateTime + .truncatedTo(ChronoUnit.MICROS) + .withOffsetSameInstant(ZoneOffset.UTC) + } yield assertTrue( + d.isDefined, + d.get.value == expected + ) + } + } + ) @@ sequential @@ shrinks(0) @@ repeats(100) @@ withLiveClock + + val noop = new Runnable() { + override def run(): Unit = () + } + + /** + * The DuckDBConnection hasn't implemented some operations that are used by zio-jdbc. This class works around those. + */ + case class WorkaroundDuckDBConnection(duckDb: DuckDBConnection) extends ConnectionWrapper(duckDb, noop) { + // Work around this feature not being implemented by duckdb jdbc + override def prepareStatement(sql: String, autoGeneratedKeys: Int): PreparedStatement = prepareStatement(sql) + override def getClientInfo(name: String): String = null + override def getClientInfo: Properties = new Properties() + } + +} diff --git a/integration/src/test/scala/zio/jdbc/JavaTimeSupportSpec.scala b/integration/src/test/scala/zio/jdbc/JavaTimeSupportSpec.scala index 3d082e09..00983157 100644 --- a/integration/src/test/scala/zio/jdbc/JavaTimeSupportSpec.scala +++ b/integration/src/test/scala/zio/jdbc/JavaTimeSupportSpec.scala @@ -59,9 +59,11 @@ object JavaTimeSupportSpec extends PgSpec { val genPGLocalDate: Gen[Any, LocalDate] = Gen.localDate(MIN_LOCAL_DATE, MAX_LOCAL_DATE) val genPGLocalTime: Gen[Any, LocalTime] = Gen.localTime(LocalTime.MIN, MAX_TIME) val genPGLocalDateTime: Gen[Any, LocalDateTime] = Gen.localDateTime(MIN_LOCAL_DATETIME, MAX_LOCAL_DATETIME) - // We need to set `UTC` as PG will move the date to UTC and so can generate a date that is not in the range of `MIN_TIMESTAMP` and `MAX_TIMESTAMP` + // We need to limit the offset range because postgres doesn't support offsets > +/-17:00 val genPGOffsetDateTime: Gen[Any, OffsetDateTime] = - Gen.offsetDateTime(MIN_OFFSET_DATETIME, MAX_OFFSET_DATETIME).map(_.withOffsetSameInstant(ZoneOffset.UTC)) + Gen + .offsetDateTime(MIN_OFFSET_DATETIME, MAX_OFFSET_DATETIME) + .filter(_.getOffset.getTotalSeconds.abs < 17 * 60) val genPGInstant: Gen[Any, Instant] = Gen.instant(MIN_TIMESTAMP, MAX_TIMESTAMP) /** From de5735f798dcd133b9172c74d5ede8518e6c2f81 Mon Sep 17 00:00:00 2001 From: Olivier Deckers <1965514+olivierdeckers@users.noreply.github.com> Date: Tue, 16 Apr 2024 22:12:55 +0200 Subject: [PATCH 4/4] Add test using duckdb --- .../src/test/scala/zio/jdbc/DuckDbSpec.scala | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala b/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala index 205df3d8..8e1f69f4 100644 --- a/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala +++ b/integration/src/test/scala/zio/jdbc/DuckDbSpec.scala @@ -58,6 +58,28 @@ object DuckDbSpec extends ZIOSpec[ZConnection] { d.get.value == expected ) } + }, + test("should be able to decode OffsetDateTime values and handle timezones correctly") { + check( + Gen.offsetDateTime( + OffsetDateTime.of(1970, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC), + OffsetDateTime.of(2100, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC) + ) + ) { offsetDateTime => + for { + _ <- sql"""CREATE TABLE offset_datetime (value TIMESTAMP WITH TIME ZONE)""".execute + _ <- sql"""INSERT INTO offset_datetime VALUES ($offsetDateTime)""".execute + d <- sql"""SELECT value FROM offset_datetime""".query[OffsetDateTime].selectOne + _ <- sql"DROP TABLE offset_datetime".execute + expected = + offsetDateTime + .truncatedTo(ChronoUnit.MICROS) + .withOffsetSameInstant(ZoneOffset.UTC) + } yield assertTrue( + d.isDefined, + d.get == expected + ) + } } ) @@ sequential @@ shrinks(0) @@ repeats(100) @@ withLiveClock