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

Finagle Postgres Int decoder throws when reading a value #1844

Closed
jonathan-ostrander opened this issue May 4, 2020 · 4 comments · Fixed by #1848
Closed

Finagle Postgres Int decoder throws when reading a value #1844

jonathan-ostrander opened this issue May 4, 2020 · 4 comments · Fixed by #1848

Comments

@jonathan-ostrander
Copy link
Contributor

@jonathan-ostrander jonathan-ostrander commented May 4, 2020

This template isn't a strict requirement to open issues, but please try to provide as much information as possible.

Version: 3.4.10
Module: quill-finagle-postgres
Database: Postgres (embedded through zonky)

Expected behavior

val client: PostgresClient
val ctx = new FinaglePostgresContext(SnakeCase, client)
import ctx._

case class Person(name: String, age: Int)
val p = Person("Bob", 25)

// Expected value is for this to be equal to `p`
Await.result(
  for {
    _ <- client.execute("CREATE TABLE person (name VARCHAR(100), age INT)")
    _ <- run(query[Person].insert(lift(p)))
    people <- run(query[Person])
  } yield people.head
)

Actual behavior

Throws:

java.lang.IllegalStateException: Cannot decode value None at index 0 to Int
  at io.getquill.util.Messages$.fail(Messages.scala:38)
  at io.getquill.context.finagle.postgres.FinaglePostgresDecoders.$anonfun$decoder$1(FinaglePostgresDecoders.scala:29)
  at io.getquill.context.finagle.postgres.FinaglePostgresDecoders.$anonfun$decoder$1$adapted(FinaglePostgresDecoders.scala:25)
  at io.getquill.context.finagle.postgres.FinaglePostgresDecoders$FinaglePostgresDecoder.apply(FinaglePostgresDecoders.scala:21)
  at $anon$1.$anonfun$extract$1(<console>:59)
  at com.twitter.util.Return.map(Try.scala:311)
  at com.twitter.util.Future.$anonfun$map$1(Future.scala:1910)
  at com.twitter.util.ConstFuture.transformTry(ConstFuture.scala:43)
  at com.twitter.util.Future.map(Future.scala:1910)
  at com.twitter.concurrent.AsyncStream.map(AsyncStream.scala:241)
  at com.twitter.finagle.postgres.PreparedStatement.$anonfun$selectToStream$1(PreparedStatement.scala:19)

Steps to reproduce the behavior

Here's a Scastie but for some reason it constantly times out. I got it to resolve the final Future in one run and it errored out with the same IllegalStateException that I get locally.

Edit: Scastie run with the error

Workaround

Currently, using a Short instead of an Int works fine.

@getquill/maintainers

@jonathan-ostrander
Copy link
Contributor Author

@jonathan-ostrander jonathan-ostrander commented May 4, 2020

Using 3.5.1 instead seems to fix the issue, although looking at the git log between the 2 versions it's not entirely clear why.

@jonathan-ostrander
Copy link
Contributor Author

@jonathan-ostrander jonathan-ostrander commented May 5, 2020

Seems like this is still popping up with 3.5.1. Another workaround is to use BIGSERIAL instead of INT.

@jilen
Copy link
Collaborator

@jilen jilen commented May 5, 2020

@j-ostrich Are you using the binary format ?

@jonathan-ostrander
Copy link
Contributor Author

@jonathan-ostrander jonathan-ostrander commented May 5, 2020

@jilen I figured out the issue late last night. By default, com.twitter.finagle.Postgres.Client() does not specify any receive functions so Row#getAnyOption will always return None. Specifying .withCustomReceiveFunctions(ValueDecoder.decoders orElse { case "noop" => ValueDecoder.unknown }) on the client fixes the issue.

I still question why the default decoders for Int and Long (and a few other types) use Row#getAnyOption rather than just get and the existing ValueDecoder for those types though.

jonathan-ostrander pushed a commit to jonathan-ostrander/quill that referenced this issue May 5, 2020
This addresses a part of the cause of issue [zio#1844](zio#1844) which is that the decoders that are not direct in `FinaglePostgresDecoders` rely on `Row#getAnyOption`. `Row#getAnyOption` relies on custom receiving functions being defined on the `PostgresClient`. postgres-finagle could define these by default (which it probably should), but by default it does not.  When custom receiving functions are not defined, `Row#getAnyOption` always returns `None`.

This change introduces an `orElse` method on `FinaglePostgresDecoder` which allows for composition of `ValueDecoder`s that was previous being accomplished by `PartialFunction`s. This solution removes the need for `ClassTag` (although we may want to keep it to keep error messages consistent) as well as the reliance on `Row#getAnyOption`.  It also removes the definition of `decoder[T]`.
jonathan-ostrander pushed a commit to jonathan-ostrander/quill that referenced this issue May 5, 2020
This addresses a part of the cause of issue [zio#1844](zio#1844) which is that the decoders that are not direct in `FinaglePostgresDecoders` rely on `Row#getAnyOption`. `Row#getAnyOption` relies on custom receiving functions being defined on the `PostgresClient`. postgres-finagle could define these by default (which it probably should), but by default it does not.  When custom receiving functions are not defined, `Row#getAnyOption` always returns `None`.

This change introduces an `orElse` method on `FinaglePostgresDecoder` which allows for composition of `ValueDecoder`s that was previous being accomplished by `PartialFunction`s. This solution removes the need for `ClassTag` (although we may want to keep it to keep error messages consistent) as well as the reliance on `Row#getAnyOption`.  It also removes the definition of `decoder[T]`.
jonathan-ostrander added a commit to jonathan-ostrander/quill that referenced this issue May 5, 2020
This addresses a part of the cause of issue [zio#1844](zio#1844) which is that the decoders that are not direct in `FinaglePostgresDecoders` rely on `Row#getAnyOption`. `Row#getAnyOption` relies on custom receiving functions being defined on the `PostgresClient`. postgres-finagle could define these by default (which it probably should), but by default it does not.  When custom receiving functions are not defined, `Row#getAnyOption` always returns `None`.

This change introduces an `orElse` method on `FinaglePostgresDecoder` which allows for composition of `ValueDecoder`s that was previous being accomplished by `PartialFunction`s. This solution removes the need for `ClassTag` (although we may want to keep it to keep error messages consistent) as well as the reliance on `Row#getAnyOption`.  It also removes the definition of `decoder[T]`.
jonathan-ostrander added a commit to jonathan-ostrander/quill that referenced this issue May 5, 2020
This addresses a part of the cause of issue [zio#1844](zio#1844) which is that the decoders that are not direct in `FinaglePostgresDecoders` rely on `Row#getAnyOption`. `Row#getAnyOption` relies on custom receiving functions being defined on the `PostgresClient`. postgres-finagle could define these by default (which it probably should), but by default it does not.  When custom receiving functions are not defined, `Row#getAnyOption` always returns `None`.

This change introduces an `orElse` method on `FinaglePostgresDecoder` which allows for composition of `ValueDecoder`s that was previous being accomplished by `PartialFunction`s. This solution removes the need for `ClassTag` (although we may want to keep it to keep error messages consistent) as well as the reliance on `Row#getAnyOption`.  It also removes the definition of `decoder[T]`.
juliano added a commit that referenced this issue May 20, 2020
This addresses a part of the cause of issue [#1844](#1844) which is that the decoders that are not direct in `FinaglePostgresDecoders` rely on `Row#getAnyOption`. `Row#getAnyOption` relies on custom receiving functions being defined on the `PostgresClient`. postgres-finagle could define these by default (which it probably should), but by default it does not.  When custom receiving functions are not defined, `Row#getAnyOption` always returns `None`.

This change introduces an `orElse` method on `FinaglePostgresDecoder` which allows for composition of `ValueDecoder`s that was previous being accomplished by `PartialFunction`s. This solution removes the need for `ClassTag` (although we may want to keep it to keep error messages consistent) as well as the reliance on `Row#getAnyOption`.  It also removes the definition of `decoder[T]`.

Co-authored-by: Juliano Alves <von.juliano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants