Github user datumbox commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17059#discussion_r103427764
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala 
---
    @@ -82,12 +82,20 @@ private[recommendation] trait ALSModelParams extends 
Params with HasPredictionCo
        * Attempts to safely cast a user/item id to an Int. Throws an exception 
if the value is
        * out of integer range.
        */
    -  protected val checkedCast = udf { (n: Double) =>
    -    if (n > Int.MaxValue || n < Int.MinValue) {
    -      throw new IllegalArgumentException(s"ALS only supports values in 
Integer range for columns " +
    -        s"${$(userCol)} and ${$(itemCol)}. Value $n was out of Integer 
range.")
    -    } else {
    -      n.toInt
    +  protected val checkedCast = udf { (n: Any) =>
    +    n match {
    +      case v: Int => v // Avoid unnecessary casting
    +      case v: Number =>
    +        val intV = v.intValue()
    +        // Checks if number within Int range and has no fractional part.
    +        if (v.doubleValue == intV) {
    --- End diff --
    
    The way it is written it checks 2 things at the same time. 1) that it is 
within integer range (so if it overflows the equality will not hold) and 2) 
that it has no fractional part. The modulo check only covers you for point #2 
but not #1.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to