You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by techaddict <gi...@git.apache.org> on 2016/05/20 11:09:28 UTC

[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

GitHub user techaddict opened a pull request:

    https://github.com/apache/spark/pull/13223

    [HOTFIX][SPARK-15445] Build fails for java 1.7 after adding java.mathBigInteger support

    ## What changes were proposed in this pull request?
    Using longValue() and then checking whether the value is in the range for a long manually.
    
    ## How was this patch tested?
    Existing tests

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/techaddict/spark SPARK-15445

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/13223.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #13223
    
----
commit 02a1fefcfa59ec9218025ff55ce5181f912964ad
Author: Sandeep Singh <sa...@techaddict.me>
Date:   2016-05-20T11:01:36Z

    [SPARK-15445] [HOTFIX] Build fails for java 1.7 after adding java.math.BigInteger support

commit cd8362d2622273c24b196987b851683d8abd6e59
Author: Sandeep Singh <sa...@techaddict.me>
Date:   2016-05-20T11:04:29Z

    fix values

commit 29f5adb5978ba21a1e2174ff4344c978e9482ea1
Author: Sandeep Singh <sa...@techaddict.me>
Date:   2016-05-20T11:06:48Z

    fix

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220752195
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59048/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220583610
  
    **[Test build #58980 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58980/consoleFull)** for PR 13223 at commit [`90f2cdc`](https://github.com/apache/spark/commit/90f2cdc17b8e05bb476cfc0a0404011863f68470).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220723359
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59022/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220693628
  
    Also I think you can remove "HOTFIX" -- those are for master build breaking changes, which isn't happening here yet. (fwiw we don't run java 7 on master)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220611169
  
    **[Test build #58987 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58987/consoleFull)** for PR 13223 at commit [`e5a5839`](https://github.com/apache/spark/commit/e5a5839108150fe8fbf3521922362579f2e03f38).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64092601
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -132,17 +133,15 @@ final class Decimal extends Ordered[Decimal] with Serializable {
        * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
        */
       def set(bigintval: BigInteger): Decimal = {
    -    try {
    -      this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    -      this._precision = DecimalType.MAX_PRECISION
    -      this._scale = 0
    -      this
    -    }
    -    catch {
    -      case e: ArithmeticException =>
    -        throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
    -     }
    +    // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +    require(
    --- End diff --
    
    why doing this as a require intsead of try catch?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64092530
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -412,8 +414,8 @@ object Decimal {
         value match {
           case j: java.math.BigDecimal => apply(j)
           case d: BigDecimal => apply(d)
    -      case k: scala.math.BigInt => apply(k)
    -      case l: java.math.BigInteger => apply(l)
    +      case k: BigInt => apply(k)
    --- End diff --
    
    this is too confusing. please keep the qualifiers. i'd actually keep qualifiers everywhere.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220579448
  
    **[Test build #58977 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58977/consoleFull)** for PR 13223 at commit [`29f5adb`](https://github.com/apache/spark/commit/29f5adb5978ba21a1e2174ff4344c978e9482ea1).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220593992
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58978/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64096982
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -412,8 +414,8 @@ object Decimal {
         value match {
           case j: java.math.BigDecimal => apply(j)
           case d: BigDecimal => apply(d)
    -      case k: scala.math.BigInt => apply(k)
    -      case l: java.math.BigInteger => apply(l)
    +      case k: BigInt => apply(k)
    --- End diff --
    
    I agree it's better to make it consistent, but let's not mix it into a simple fix for java 7 ...



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220597956
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220723357
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220773207
  
    Merged to master/2.0


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220597959
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58980/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64095788
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -132,17 +133,15 @@ final class Decimal extends Ordered[Decimal] with Serializable {
        * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
        */
       def set(bigintval: BigInteger): Decimal = {
    -    try {
    -      this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    -      this._precision = DecimalType.MAX_PRECISION
    -      this._scale = 0
    -      this
    -    }
    -    catch {
    -      case e: ArithmeticException =>
    -        throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
    -     }
    +    // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +    require(
    --- End diff --
    
    Now the error is not detected by catching an exception but by manually checking the value. Require is the simplest  way to get the desired result which is an IAE.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220591901
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58977/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64096905
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -132,17 +133,15 @@ final class Decimal extends Ordered[Decimal] with Serializable {
        * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
        */
       def set(bigintval: BigInteger): Decimal = {
    -    try {
    -      this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    -      this._precision = DecimalType.MAX_PRECISION
    -      this._scale = 0
    -      this
    -    }
    -    catch {
    -      case e: ArithmeticException =>
    -        throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
    -     }
    +    // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +    require(
    --- End diff --
    
    Doesn't this have a potential perf hit? Also - is this actually related to the problem this ticket is solving?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220592440
  
    **[Test build #58987 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58987/consoleFull)** for PR 13223 at commit [`e5a5839`](https://github.com/apache/spark/commit/e5a5839108150fe8fbf3521922362579f2e03f38).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220704998
  
    **[Test build #59022 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59022/consoleFull)** for PR 13223 at commit [`35da7c2`](https://github.com/apache/spark/commit/35da7c2d1bd9f40ec3b227cb9a8c44c9dc72aff2).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64097479
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -132,17 +133,15 @@ final class Decimal extends Ordered[Decimal] with Serializable {
        * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
        */
       def set(bigintval: BigInteger): Decimal = {
    -    try {
    -      this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    -      this._precision = DecimalType.MAX_PRECISION
    -      this._scale = 0
    -      this
    -    }
    -    catch {
    -      case e: ArithmeticException =>
    -        throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
    -     }
    +    // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +    require(
    --- End diff --
    
    Ah ok. Then well - we have to do this.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64031334
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -132,17 +133,18 @@ final class Decimal extends Ordered[Decimal] with Serializable {
        * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
        */
       def set(bigintval: BigInteger): Decimal = {
    -    try {
    -      this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    -      this._precision = DecimalType.MAX_PRECISION
    -      this._scale = 0
    -      this
    +    this.decimalVal = null
    +    // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +    val isExactLong = bigintval.compareTo(LONG_MAX_BIG_INT) <= 0 &&
    +      bigintval.compareTo(LONG_MIN_BIG_INT) >= 0
    +    if (isExactLong) {
    +      this.longVal = bigintval.longValue()
    +    } else {
    +      throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
    --- End diff --
    
    How about just 
    
    ```
    require(bigintval.compareTo(LONG_MAX_BIG_INT) <= 0 && bigintval.compareTo(LONG_MIN_BIG_INT) >= 0, ...message...)
    this.longValue = ...
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220752194
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220611445
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220597782
  
    **[Test build #58980 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58980/consoleFull)** for PR 13223 at commit [`90f2cdc`](https://github.com/apache/spark/commit/90f2cdc17b8e05bb476cfc0a0404011863f68470).
     * This patch **fails PySpark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220581053
  
    **[Test build #58978 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58978/consoleFull)** for PR 13223 at commit [`c0766f0`](https://github.com/apache/spark/commit/c0766f0044254918aafe74f0b8e714d0276641b9).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220627182
  
    @srowen All tests passed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220746410
  
    **[Test build #59048 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59048/consoleFull)** for PR 13223 at commit [`faff787`](https://github.com/apache/spark/commit/faff7879d6ce8a8652de4b8d4ffd9ae0c144cedf).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220595912
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58979/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220591761
  
    **[Test build #58977 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58977/consoleFull)** for PR 13223 at commit [`29f5adb`](https://github.com/apache/spark/commit/29f5adb5978ba21a1e2174ff4344c978e9482ea1).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64097495
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -382,6 +381,9 @@ object Decimal {
       private[sql] val ZERO = Decimal(0)
       private[sql] val ONE = Decimal(1)
     
    +  private[types] val LONG_MAX_BIG_INT = BigInteger.valueOf(JLong.MAX_VALUE)
    --- End diff --
    
    this can just be private


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220746454
  
    @rxin @srowen I've removed qualifiers


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220639711
  
    @techaddict @srowen @cloud-fan @gatorsmile : Hi Sandeep , thanks for fixing this. I didn't realize the method is java 1.8 only. The code looks good to me. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64026868
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -134,7 +135,14 @@ final class Decimal extends Ordered[Decimal] with Serializable {
       def set(bigintval: BigInteger): Decimal = {
         try {
           this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    +      // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +      val isExactLong = bigintval.compareTo(BigInteger.valueOf(JLong.MAX_VALUE)) <= 0 &&
    +        bigintval.compareTo(BigInteger.valueOf(JLong.MIN_VALUE)) >= 0
    +      if(isExactLong) {
    --- End diff --
    
    Since we don't need the try catch, this would be much better
    ```scala
    def set(bigintval: BigInteger): Decimal = {
        this.decimalVal = null
        // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
        val isExactLong = bigintval.compareTo(LONG_MAX_BIG_INT) <= 0 &&
          bigintval.compareTo(LONG_MIN_BIG_INT) >= 0
        if (isExactLong) {
          this.longVal = bigintval.longValue()
        } else {
          throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
        }
        this._precision = DecimalType.MAX_PRECISION
        this._scale = 0
        this
      }
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64095897
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -412,8 +414,8 @@ object Decimal {
         value match {
           case j: java.math.BigDecimal => apply(j)
           case d: BigDecimal => apply(d)
    -      case k: scala.math.BigInt => apply(k)
    -      case l: java.math.BigInteger => apply(l)
    +      case k: BigInt => apply(k)
    --- End diff --
    
    The code is inconsistent in this regard though. I thought it worth cleaning up but don't feel strongly about it. Alternative is to qualify both classes everywhere consistently 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/13223


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220595911
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220693354
  
    @techaddict thanks for doing this. Can you be more surgical in fixes like this? i.e. just fix the line that is causing problem.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220723152
  
    **[Test build #59022 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59022/consoleFull)** for PR 13223 at commit [`35da7c2`](https://github.com/apache/spark/commit/35da7c2d1bd9f40ec3b227cb9a8c44c9dc72aff2).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64097151
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -132,17 +133,15 @@ final class Decimal extends Ordered[Decimal] with Serializable {
        * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
        */
       def set(bigintval: BigInteger): Decimal = {
    -    try {
    -      this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    -      this._precision = DecimalType.MAX_PRECISION
    -      this._scale = 0
    -      this
    -    }
    -    catch {
    -      case e: ArithmeticException =>
    -        throw new IllegalArgumentException(s"BigInteger ${bigintval} too large for decimal")
    -     }
    +    // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +    require(
    --- End diff --
    
    The problem is we can't call a method that will throw an exception if the value is out of bounds during conversion. We just check it manually. I actually expect this is faster but it is vanishingly small as a difference. I don't think there is a choice  if Java 7 is being supported. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220579058
  
    cc: @srowen 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220582710
  
    **[Test build #58979 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58979/consoleFull)** for PR 13223 at commit [`a1e54c2`](https://github.com/apache/spark/commit/a1e54c2de9da98d9dcfd51910ad95b0a2e55b5cb).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220593818
  
    **[Test build #58978 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58978/consoleFull)** for PR 13223 at commit [`c0766f0`](https://github.com/apache/spark/commit/c0766f0044254918aafe74f0b8e714d0276641b9).
     * This patch **fails PySpark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220595772
  
    **[Test build #58979 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58979/consoleFull)** for PR 13223 at commit [`a1e54c2`](https://github.com/apache/spark/commit/a1e54c2de9da98d9dcfd51910ad95b0a2e55b5cb).
     * This patch **fails PySpark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64025476
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -134,7 +135,14 @@ final class Decimal extends Ordered[Decimal] with Serializable {
       def set(bigintval: BigInteger): Decimal = {
         try {
           this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    +      // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +      val isExactLong = bigintval.compareTo(BigInteger.valueOf(JLong.MAX_VALUE)) <= 0 &&
    --- End diff --
    
    Maybe extract constants for a `BigInteger` representation of the max/min values rather than compute each time? Also is this all uses of java.util.Long that can be JLong?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64025518
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -134,7 +135,14 @@ final class Decimal extends Ordered[Decimal] with Serializable {
       def set(bigintval: BigInteger): Decimal = {
         try {
           this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    +      // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +      val isExactLong = bigintval.compareTo(BigInteger.valueOf(JLong.MAX_VALUE)) <= 0 &&
    +        bigintval.compareTo(BigInteger.valueOf(JLong.MIN_VALUE)) >= 0
    +      if(isExactLong) {
    --- End diff --
    
    Space after if. Don't throw an exception just to catch it; throw the exception below here. No need for "else"


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220593990
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220636830
  
    cc @kevinyu98 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220611448
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58987/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64026748
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -134,7 +135,14 @@ final class Decimal extends Ordered[Decimal] with Serializable {
       def set(bigintval: BigInteger): Decimal = {
         try {
           this.decimalVal = null
    -      this.longVal = bigintval.longValueExact()
    +      // TODO: Remove this once we migrate to java8 and use longValueExact() instead.
    +      val isExactLong = bigintval.compareTo(BigInteger.valueOf(JLong.MAX_VALUE)) <= 0 &&
    +        bigintval.compareTo(BigInteger.valueOf(JLong.MIN_VALUE)) >= 0
    +      if(isExactLong) {
    --- End diff --
    
    sorry didn't get the part throw the exception below here.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13223#discussion_r64031221
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -174,7 +176,7 @@ final class Decimal extends Ordered[Decimal] with Serializable {
     
       def toScalaBigInt: BigInt = BigInt(toLong)
     
    -  def toJavaBigInteger: java.math.BigInteger = java.math.BigInteger.valueOf(toLong)
    +  def toJavaBigInteger: BigInteger = BigInteger.valueOf(toLong)
    --- End diff --
    
    Although I support removing the redundant qualified name, I wonder if we should also un-qualify scala.math.BigInt, or else leave both qualified even though they're not ambiguous. BigDecimal is used unqualified while the Java version is always qualified, to disambiguate. I guess it would become consistent if BigInt were not qualified.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-15445][SQL] Build fails for java 1.7 af...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220752136
  
    **[Test build #59048 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59048/consoleFull)** for PR 13223 at commit [`faff787`](https://github.com/apache/spark/commit/faff7879d6ce8a8652de4b8d4ffd9ae0c144cedf).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [HOTFIX][SPARK-15445] Build fails for java 1.7...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13223#issuecomment-220591899
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org