com-lihaoyi / scalasql

Scala ORM to query SQL databases from Scala via concise, type-safe, and familiar case classes and collection operations. Connects to Postgres, MySql, H2, and Sqlite out of the box
194 stars 22 forks source link

AggOps.maxBy expects Numeric[T] for no reason #46

Open scalway opened 1 day ago

scalway commented 1 day ago

It seems like maxBy has an issue. Unlike minBy, it requires a Numeric[T] constraint, which is inconsistent and unnecessary.

package scalasql.operations

class AggOps[T](v: Aggregatable[T])(...){
  def minBy[V: TypeMapper] ...

  //it seams that none of those functions need Numeric! 
  def maxBy[V: Numeric: TypeMapper] ...
  def minByOpt[V: Numeric: TypeMapper] ...
  def maxByOpt[V: Numeric: TypeMapper] ...
}

Minified example that shows compilation error:

//> using dep com.lihaoyi::scalasql:0.1.11

import scalasql._, SqliteDialect._
import java.time.Instant
// Define your table model classes
//> using dep com.lihaoyi::scalasql:0.1.11

case class City[T[_]](
  id: T[Int],
  name: T[String],
  createdAt: T[Instant],
)

object City extends Table[City]

//uncomment to fix compilation
implicit val ops:Numeric[Instant] = null

def getCities() = City.select
  .maxBy(_.createdAt) //<---- [error] implicit Ordering defined for java.time.Instant.

Not sure why we get Ordering issue:

[error] No implicit Ordering defined for java.time.Instant.
[error]   .maxBy(_.createdAt)

Discord question that exposed it: https://discord.com/channels/632150470000902164/940067748103487558/1309464062014525451

Complete code that runs queries

//> using dep com.lihaoyi::scalasql:0.1.11
//> using dep org.xerial:sqlite-jdbc:3.47.0.0

import scalasql._, SqliteDialect._
import java.time.Instant
import scalasql.core.DbClient.DataSource

case class City[T[_]](
  id: T[Int],
  name: T[String],
  createdAt: T[Instant],
)

object City extends Table[City]

val dataSource = new org.sqlite.SQLiteDataSource()
dataSource.setUrl(s"jdbc:sqlite:file.db")
val dbClient: DataSource = new scalasql.DbClient.DataSource(
  dataSource,
  config = new scalasql.Config:
    override def nameMapper(v: String) = v.toLowerCase() // Override default snake_case mapper
    override def logSql(sql: String, file: String, line: Int) = println(s"$file:$line $sql")
)

//uncomment to fix compilation
//implicit val ops:Numeric[Instant] = null

val createTableQuery = """CREATE TABLE IF NOT EXISTS city (
    id integer AUTO_INCREMENT PRIMARY KEY,
    name varchar NOT NULL,
    createdAt timestamp NOT NULL
);"""

val insertQuery = """INSERT INTO city (name, createdAt) VALUES 
  ('New York', '2021-09-01 01:00:00'),
  ('Los Angeles', '2021-09-02 02:00:00'),
  ('Dallas', '2021-09-09 09:00:00');"""

def getCities() = City.select
  .sortBy(_.createdAt).desc
  .maxBy(_.createdAt) //<---- [error] implicit Ordering defined for java.time.Instant.

dbClient.transaction: db =>
  db.updateRaw(createTableQuery)
  db.updateRaw(insertQuery)
  println(db.run(getCities()))
lihaoyi commented 1 day ago

Seems like we can remove that restriction. Feel free to send a PR!