slick / slick

Slick (Scala Language Integrated Connection Kit) is a modern database query and access library for Scala
https://scala-slick.org/
BSD 2-Clause "Simplified" License
2.65k stars 608 forks source link

Provide an implicit macro to materialize column types for value classes #942

Open julienrf opened 10 years ago

julienrf commented 10 years ago

Also see this discussion.

Defining data types modeling each possible query result is cumbersome (e.g. Person, PersonWithAddress, PersonWithPhone, PersonWithPhoneAndAddress, etc.).

A convenient alternative is to use tuples to model query results (e.g. (Person, Address), (Person, String), (Person, String, Address), etc.). A limitation of this approach is that tuple members don’t have names as meaningful as a proper class definition (i.e. phone vs _2), so they can be error prone: “what does this String value mean?”

A probably good practice could be to use wrapper type as follows:

case class Phone(value: String)

So that we can reach a reasonable trade-off between boilerplate and readability by using (Person, Phone) where we previously used PersonWithPhone or (Person, String).

However, creating a Phone value has an overhead compared to creating just a String value. This can be avoided by using a value class:

case class Phone(value: String) extends AnyVal

Now the creation of a Phone value has no overhead.

A problem remains, though: if one wants to use the Phone type in a table mapping, he has to define a corresponding ColumnType[Phone] instance. This requires the following code:

implicit val phoneColumnType = MappedColumnType.base(_.value, Phone(_))

My suggestion is to materialize such column type instances for value classes using a macro so that users don’t even have to write the above implicit definition.

Note that there is a MappedTo trait intended to make it possible to not write the implicit column type definition. However, this trait can not be mixed in a value class:

scala> case class Latitude(value: Double) extends AnyVal with MappedTo[Double]
<console>:8: error: illegal inheritance; superclass AnyVal
 is not a subclass of the superclass Object
 of the mixin trait MappedTo
       case class Latitude(value: Double) extends AnyVal with MappedTo[Double]
                                                              ^

 

Also note that using a type alias instead of a wrapper type would make it possible to use the Phone type in table mapping without having to define a ColumnType instance, but I think that using a wrapper type makes things more explicit and less error prone.

julienrf commented 10 years ago

Note that there is a MappedTo trait intended to make it possible to not write the implicit column type definition. However, this trait can not be mixed in a value class

Well, it seems that this is wrong because MappedTo inherits from Any so you should be able to mix it into a value class without trouble.

It means that you can just write the following wrapper type:

case class Phone(value: String) extends AnyVal with MappedTo[String]

With no need to write the implicit ColumnType instance.

I didn’t tested that, though.

szeiger commented 9 years ago

Yes, you can mix MappedTo into a value class. There's a test case for this in RelationalMapperTest.

godenji commented 9 years ago

Now the creation of a Phone value has no overhead.

Not true, once you extend a universal trait (i.e. a trait that extends Any) you pay the runtime allocation price. Depending on the application this may or may not be an issue, but it would be nice to have safety, performance, and mappability -- I guess for the performance bit we have to wait for Java 10...

see my reply below

mdedetrich commented 9 years ago

@godenji, is there documentation for that use case?

godenji commented 9 years ago

@mdedetrich as far as Slick's MappedTo is concerned, there is no allocation overhead; it's when you call a method defined on a universal trait that allocation occurs. In other words, ignore me ;-)

There are additional caveats to keep in mind when working with value classes, however. Check out the docs