zio / zio-quill

Compile-time Language Integrated Queries for Scala
https://zio.dev/zio-quill
Apache License 2.0
2.15k stars 346 forks source link

Add array support for FinaglePostgresContext #1123

Open davoclavo opened 6 years ago

davoclavo commented 6 years ago

Version: (e.g. 2.5.4) Module: (e.g. quill-finagle-postgres) Database: (e.g. postgres)

Expected behavior

It would be nice if FinaglePostgresContext had Array support, such as PostgresJdbcContext and PostgresAsyncContext

Actual behavior

Steps to reproduce the behavior

case class User(nicknames: Seq[String])
create table user(
  nicknames text[]
)
ctx.run(query[User])

Workaround

I guess write your own Encoder/Decoder. Not sure if it is possible to write such decoders for container types like Seq[T]

@getquill/maintainers


I will try to implement similar traits like ArrayEncoders extends ArrayEncoding and ArrayDecoders extends ArrayDecoding from the async/jdbc postgres contexts.

davoclavo commented 6 years ago

Quick update, I was able to get a very early stab at building an array encoder (Still missing the binary representation, and the decoder part) -- It seems that it is trickier to extend ArrayEncoding than to just implement the encoder similar to the one for Option[T] and also getting inspiration from finagle-postgres' Option[T] ValueEncoder

trait ArrayEncoders  {
  self: FinaglePostgresContext[_] =>

  implicit def array[T](implicit encodeT: ValueEncoder[T]): ValueEncoder[Seq[T]] =
    new ValueEncoder[Seq[T]] {
      val typeName = "array"
      val elemTypeName = None

      def encodeText(seqT: Seq[T]) =
        Some(seqT.flatMap(encodeT.encodeText).mkString("{", ",", "}"))

      def encodeBinary(tSeq: Seq[T], c: Charset) = ???
    }

  implicit def arrayEncoder[T](implicit e: Encoder[T]): Encoder[Seq[T]] =
    FinanglePostgresEncoder[Seq[T]](array(e.encoder))
}

~Note: I'm not yet sure if I should be encoding an empty array as null or {}~


I am able to do things like:

case class User(nicknames: Seq[String])
val user = User(Seq("davoclavo", "david"))
ctx.run(quote{
  query[User].insert(lift(user))
})

Which inserts the data as {davoclavo,david}

pettyjamesm commented 6 years ago

Definitely don't encode empty arrays as null, you lose the ability to distinguish between the two. Would you encode an empty string as null? I don't know anything about the finagle postgres implementation so sadly I can't help with that, but if it supports the same textual representation as arrays then that should still be a nice improvement for other users.

davoclavo commented 6 years ago

@pettyjamesm agree, thanks for the feedback! as soon as I did that I realized that it was a bad idea. This was a very quick draft on how I was attempting the implementation, mostly for future reference (I've updated the snippet to remove those null encodings)

pettyjamesm commented 6 years ago

Don't worry, no judgement on rough drafts. Just wanted to give feedback on the part that I could and I'm sorry I couldn't be more useful regarding the meat of the problem.

mosyp commented 6 years ago

Hello @davoclavo, this one is better to be done in their driver repo, I've opened an issue while ago, see https://github.com/finagle/finagle-postgres/issues/55.