zio / zio-jdbc

A small, idiomatic ZIO interface to JDBC.
Apache License 2.0
82 stars 64 forks source link

`JdbcDecoder.optionDecoder` should be a `null` test #188

Open oyvindberg opened 11 months ago

oyvindberg commented 11 months ago

This looks wrong. it returns None on failures to decode? The correct thing to do here is to first read the raw value from the ResultSet and then call wasNull. I discovered this when I ended up with an NPE :)

  implicit def optionDecoder[A](implicit decoder: JdbcDecoder[A]): JdbcDecoder[Option[A]] =
    JdbcDecoder(rs =>
      int =>
        decoder.decode(int, rs) match {
          case Left(_)      => None
          case Right(value) => Option(value._2)
        }
    )
oyvindberg commented 11 months ago

Oh, and there is another consequence:

Given this code:

    withConnection {
      for {
        _ <-
          sql"""insert into public.pgtestnull("bool", "float4", "float8", "int4", "int8")
          values (${Option.empty[Boolean]}, ${Option.empty[Float]}, ${Option.empty[Double]}, ${Option.empty[Int]}, ${Option.empty[Double]}) 
       """.insert
        _ <-
          sql"""select "bool", "float4", "float8", "int4", "int8" from public.pgtestnull"""
            .query[(Option[Boolean], Option[Float], Option[Double], Option[Int], Option[Long])]
            .selectStream.runHead.map(println)
      } yield ()

    }

valus are correctly stored as null, while we read:

Some((Some(false),Some(0.0),Some(0.0),Some(0),Some(0)))
karvanit commented 2 days ago

This also leads to "interesting" failures when attempting to .map decoders. For example: "SELECT NULL".query[Option[BigDecimal]] works. But "SELECT NULL".query[Option[Wrapped]] fails when we have

implicit val fromJdbc: JdbcDecoder[Wrapped] = JdbcDecoder.bigDecimal.map(bd => Wrapped(bd.toBigInteger))