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

Feature request: add Read and Write instances for None.type #2118

Open
satorg opened this issue Oct 17, 2024 · 1 comment
Open

Feature request: add Read and Write instances for None.type #2118

satorg opened this issue Oct 17, 2024 · 1 comment
Milestone

Comments

@satorg
Copy link
Contributor

satorg commented Oct 17, 2024

It would be nice to have build-in Read[None.type] and Write[None.type] instances that would be enabled by default (similar to Read[Unit], etc) with the following behavior:

  • Write[None.type] simply sets/updates a column to SQL NULL;
  • Read[None.type] gets a column value and either returns None if the column is SQL NULL or fails if it is not.

Why?

Currently the behavior is inconsistent between Option[A] and None.type types, e.g.:

scala> sql"SELECT 12, null, 34".query[(Int, Option[Int], Int)].unique.quick.unsafeRunSync()
  (12,None,34)

scala> sql"SELECT 12, null, 34".query[(Int, None.type, Int)].unique.quick.unsafeRunSync()
doobie.util.invariant$NonNullableColumnRead: SQL `NULL` read at column 2 (JDBC type Integer) but mapping is to a non-Option type; use Option here. Note that JDBC column indexing is 1-based.
  at doobie.util.Get.unsafeGetNonNullable(get.scala:29)
  at doobie.util.Read$.$anonfun$fromGet$1(read.scala:74)
  at doobie.util.Read$.$anonfun$fromGet$1$adapted(read.scala:74)
  at doobie.util.LowerPriorityRead.$anonfun$product$1(ReadPlatform.scala:31)

Since None is a subtype of Option these two queries are expected to work in the same way, but in fact they don't.

Use case

However, if the behavior was consistent, it could come in handy for some specific scenarios. For example, consider two queries that produce similar records that differ in just one column that is either guaranteed to be non-null for the first query and always null for the second one. In such a case it may make sense to re-use a single ADT for both queries (especially if it runs pretty big):

case class Foo[B <: Option[Int]](a: Int, b: B, c: String)

Having that, we could do the following:

sql"SELECT a, b, c FROM foo WHERE b IS NOT NULL".query[Foo[Some[Int]]]....
sql"SELECT a, b, c FROM foo WHERE b IS NULL".query[Foo[None.type]]....

That way we could get strictly typed values of Foo[Some[Int]] and Foo[None.type] independently of each other and either process them separately (when necessary) without doing any further runtime checks (b.isEmpty, etc) OR process them all together as a type widened to Foo[Option[Int]]:

// This method requires that `foo.b == None`
def processFooNone(foo: Foo[None.type]) = ???
// This method requires that `foo.b` is defined
def processFooSome(foo: Foo[Some[Int]]) = ???
// This method is `foo.b`-agnostic
def processFooBoth(foo: Foo[?]) = ???

Actual behavior

Currently, however, None values are treated as a product with arity 0 and therefore they behave similar to Unit (which is not great, because we already have Unit for that, don't we?)

Note that Some-type values do work correctly already because they are treated as products with arity 1 and therefore do not accept SQL NULL values in query results, which is expected:

scala> sql"SELECT 123, 456, 789".query[(Int, Some[Int], Int)].unique.quick.unsafeRunSync()
  (123,Some(456),789)

scala> sql"SELECT 123, null, 789".query[(Int, Some[Int], Int)].unique.quick.unsafeRunSync()
doobie.util.invariant$NonNullableColumnRead: SQL `NULL` read at column 2 (JDBC type Integer) but mapping is to a non-Option type; use Option here. Note that JDBC column indexing is 1-based.
  at doobie.util.Get.unsafeGetNonNullable(get.scala:29)
  at doobie.util.Read$.$anonfun$fromGet$1(read.scala:74)

Besides

Circe the library offers proper support for encoding/decoding None.type already:

scala> import io.circe.syntax.*, io.circe.generic.auto.*
import io.circe.syntax._
import io.circe.generic.auto._

scala> case class Foo[B <: Option[Int]](a: Int, b: B, c: String)
class Foo

scala> val fooNone = Foo(123, None, "abc")
val fooNone: Foo[None.type] = Foo(123,None,abc)

scala> val fooSome = Foo(456, Some(789), "def")
val fooSome: Foo[Some[Int]] = Foo(456,Some(789),def)

scala> fooNone.asJson
val res6: io.circe.Json =
{
  "a" : 123,
  "b" : null,
  "c" : "abc"
}

scala> res6.as[Foo[None.type]]
val res7: io.circe.Decoder.Result[Foo[None.type]] = Right(Foo(123,None,abc))

scala> res6.as[Foo[Some[Int]]]
val res8: io.circe.Decoder.Result[Foo[Some[Int]]] = Left(DecodingFailure at .b: Int)

scala> fooSome.asJson
val res9: io.circe.Json =
{
  "a" : 456,
  "b" : 789,
  "c" : "def"
}

scala> res9.as[Foo[None.type]]
val res10: io.circe.Decoder.Result[Foo[None.type]] = Left(DecodingFailure at .b: Got value '789' with wrong type, expecting null)

scala> res9.as[Foo[Some[Int]]]
val res11: io.circe.Decoder.Result[Foo[Some[Int]]] = Right(Foo(456,Some(789),def))
@jatcwang
Copy link
Collaborator

Although I haven't seen this use case in the wild it makes sense to me - perhaps something to add post 1.0 since it can be added without affecting binary compatibility.

The issue you specified in Why? section should be fixed in 1.0-RC6 (or the upcoming RC7 release), since I made the base case derivation require at least a Tuple1 (or A :: HNil), instead of the previous HNil.

@jatcwang jatcwang added this to the Post 1.0 milestone Oct 21, 2024
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

No branches or pull requests

2 participants