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

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

Open
scalway opened this issue Nov 22, 2024 · 1 comment
Open

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

scalway opened this issue Nov 22, 2024 · 1 comment

Comments

@scalway
Copy link

scalway commented Nov 22, 2024

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
Copy link
Member

lihaoyi commented Nov 23, 2024

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

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