Skip to content

Commit ed9f25d

Browse files
committed
fix: avoid BigDecimal.equals exception cost in FailedJoin/Table checks
1 parent 1945331 commit ed9f25d

2 files changed

Lines changed: 27 additions & 10 deletions

File tree

modules/sql-core/src/main/scala/FailedJoin.scala

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,18 @@
1616
package grackle.sql
1717

1818
/**
19-
* A sentinal value representing the empty column values from a failed join.
19+
* A sentinel value representing the empty column values from a failed join.
2020
*/
21-
case object FailedJoin
21+
case object FailedJoin {
22+
23+
/**
24+
* Cheap equality check for the `FailedJoin` sentinel.
25+
*
26+
* Column values are `Any` and may be a `scala.math.BigDecimal`, whose `equals` throws and
27+
* catches an `ArithmeticException` whenever compared to a value of another type (see
28+
* `BigDecimal.isValidLong`). `a == b` calls `a.equals(b)`, so keeping `FailedJoin` on the
29+
* left runs the singleton's own cheap `equals` instead of the column value's -- do not swap
30+
* the operands here.
31+
*/
32+
def isFailedJoin(v: Any): Boolean = FailedJoin == v
33+
}

modules/sql-core/src/main/scala/SqlMapping.scala

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import grackle.Predicate._
3131
import grackle.Query._
3232
import grackle.ValidationFailure.Severity
3333
import grackle.circe.CirceMappingLike
34+
import grackle.sql.FailedJoin.isFailedJoin
3435
import grackle.syntax._
3536

3637
abstract class SqlMapping[F[_]](implicit val M: MonadThrow[F])
@@ -4267,6 +4268,10 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
42674268
}
42684269

42694270
object Table {
4271+
// Same rationale as `FailedJoin.isFailedJoin`: keeps `None` on the left of `==` so its
4272+
// cheap equals runs instead of a possibly-`BigDecimal` column value's.
4273+
private def isNone(v: Any): Boolean = None == v
4274+
42704275
def apply(rows: Vector[Array[Any]]): Table = {
42714276
if (rows.sizeCompare(1) == 0) OneRowTable(rows.head)
42724277
else if (rows.isEmpty) EmptyTable
@@ -4304,7 +4309,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
43044309

43054310
def definesAll(cols: List[Int]): Boolean = {
43064311
val cs = cols
4307-
cs.forall(c => row(c) != FailedJoin)
4312+
cs.forall(c => !isFailedJoin(row(c)))
43084313
}
43094314

43104315
def group(cols: List[Int]): Iterator[Table] = {
@@ -4337,10 +4342,10 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
43374342
while (ir.hasNext) {
43384343
ir.next()(c) match {
43394344
case FailedJoin =>
4340-
case v if value == FailedJoin => value = v
4345+
case v if isFailedJoin(value) => value = v
43414346
case v if value == v =>
43424347
case None =>
4343-
case v @ Some(_) if value == None => value = v
4348+
case v @ Some(_) if isNone(value) => value = v
43444349
case _ => return None
43454350
}
43464351
}
@@ -4349,12 +4354,12 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
43494354

43504355
def filterDefined(cols: List[Int]): Table = {
43514356
val cs = cols
4352-
Table(rows.filter(r => cs.forall(c => r(c) != FailedJoin)))
4357+
Table(rows.filter(r => cs.forall(c => !isFailedJoin(r(c)))))
43534358
}
43544359

43554360
def definesAll(cols: List[Int]): Boolean = {
43564361
val cs = cols
4357-
rows.exists(r => cs.forall(c => r(c) != FailedJoin))
4362+
rows.exists(r => cs.forall(c => !isFailedJoin(r(c))))
43584363
}
43594364

43604365
def group(cols: List[Int]): Iterator[Table] = {
@@ -4369,7 +4374,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
43694374
case cs => row => cs.map(c => row(c))
43704375
}
43714376

4372-
val nonNull = rows.filter(r => cs.forall(c => r(c) != FailedJoin))
4377+
val nonNull = rows.filter(r => cs.forall(c => !isFailedJoin(r(c))))
43734378
val grouped = nonNull.groupBy(discrim)
43744379
grouped.iterator.map { case (_, rows) => Table(rows) }
43754380
}
@@ -4387,7 +4392,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
43874392
case cs => row => cs.map(c => row(c))
43884393
}
43894394

4390-
val nonNull = rows.filter(r => cs.forall(c => r(c) != FailedJoin))
4395+
val nonNull = rows.filter(r => cs.forall(c => !isFailedJoin(r(c))))
43914396
nonNull.map(discrim).distinct.size
43924397
}
43934398
}
@@ -4532,7 +4537,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self
45324537
case Some(f) if tpe.variantField(fieldName) && !fieldTpe.isNullable => f
45334538
case other => other
45344539
}
4535-
assert(leafFocus != FailedJoin)
4540+
assert(!isFailedJoin(leafFocus))
45364541
LeafCursor(fieldContext, leafFocus, Some(np), Env.empty)
45374542
})
45384543

0 commit comments

Comments
 (0)