You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by kevinyu98 <gi...@git.apache.org> on 2015/12/03 09:07:48 UTC

[GitHub] spark pull request: Working on spark 11827

GitHub user kevinyu98 opened a pull request:

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

    Working on spark 11827

    Hello : Can you help check this PR? I am adding support for the java.math.BigInteger for java bean code path. I saw internally spark is converting the BigInteger to BigDecimal in ColumnType.scala and CatalystRowConverter.scala. I use the similar way and convert the BigInteger to the BigDecimal. . 

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

    $ git pull https://github.com/kevinyu98/spark working_on_spark-11827

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

    https://github.com/apache/spark/pull/10125.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 #10125
    
----
commit a67722094e8a9d0689ba022eb4f923e28791503e
Author: Kevin Yu <qy...@us.ibm.com>
Date:   2015-12-01T16:38:09Z

    adding java.math.BigInteger support for java bean

commit a58d92cd85719c6112c5cb0162be9b6104f9ba00
Author: Kevin Yu <qy...@us.ibm.com>
Date:   2015-12-02T05:37:56Z

    adding test case

commit f400a825f38a2e3559e9b4f63b4e58bdd17c5e3b
Author: Kevin Yu <qy...@us.ibm.com>
Date:   2015-12-03T07:38:15Z

    modify the JavaDataFrameSuite

commit 3db875a7d9a331d3a200d26338c956d694001046
Author: Kevin Yu <qy...@us.ibm.com>
Date:   2015-12-03T07:50:43Z

    clean the JavaDataFrameSuite

commit 0807550ae396231a19648c2f4db7e8946544d4a2
Author: Kevin Yu <qy...@us.ibm.com>
Date:   2015-12-03T07:57:20Z

    working on the JavaDataFrameSuite

----


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219888712
  
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219885199
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58721/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

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


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by cloud-fan <gi...@git.apache.org>.

Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/10125#discussion_r63897785
  
    --- Diff: sql/core/src/test/java/test/org/apache/spark/sql/JavaDataFrameSuite.java ---
    @@ -182,6 +189,8 @@ void validateDataFrameWithBeans(Bean bean, Dataset<Row> df) {
         for (int i = 0; i < d.length(); i++) {
           Assert.assertEquals(bean.getD().get(i), d.apply(i));
         }
    +      // Java.math.BigInteger is equavient to Spark Decimal(38,0)
    +    Assert.assertEquals(new BigDecimal(bean.getE()), first.getDecimal(4).setScale(0));
    --- End diff --
    
    why do we need to `setScale` 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

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

    https://github.com/apache/spark/pull/10125#discussion_r47866947
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala ---
    @@ -75,6 +75,7 @@ object JavaTypeInference {
           case c: Class[_] if c == classOf[java.lang.Boolean] => (BooleanType, true)
     
           case c: Class[_] if c == classOf[java.math.BigDecimal] => (DecimalType.SYSTEM_DEFAULT, true)
    +      case c: Class[_] if c == classOf[java.math.BigInteger] => (DecimalType.SYSTEM_DEFAULT, true)
    --- End diff --
    
    Yes, I will use (38,0) for the BigInteger. No need to have scale. Thanks.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219888573
  
    **[Test build #58723 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58723/consoleFull)** for PR 10125 at commit [`536d20c`](https://github.com/apache/spark/commit/536d20cde27ee2f91d7b83f32a90ba5e30d92ea6).
     * This patch **fails Spark unit tests**.
     * This patch **does not merge 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220237290
  
    @cloud-fan I tried, and it still fail. It didn't go through the createDataFrame you added in SparkSession. 
    It went with  this createDataFrame(data: java.util.List[_], beanClass: Class[_]): DataFrame 
    -> val rows = SQLContext.beansToRows(data.asScala.iterator, beanInfo, attrSeq)
    
    the beanToRows will create internal rows and it is from SQLContext. 
    
    Should we add RowEncoder into the beansToRows call or leave the code as it is ? Thanks.
    
    here is the trace
    
    scala.MatchError: 1234567 (of class java.math.BigInteger)
    	at org.apache.spark.sql.catalyst.CatalystTypeConverters$DecimalConverter.toCatalystImpl(CatalystTypeConverters.scala:326)
    	at org.apache.spark.sql.catalyst.CatalystTypeConverters$DecimalConverter.toCatalystImpl(CatalystTypeConverters.scala:323)
    	at org.apache.spark.sql.catalyst.CatalystTypeConverters$CatalystTypeConverter.toCatalyst(CatalystTypeConverters.scala:102)
    	at org.apache.spark.sql.catalyst.CatalystTypeConverters$$anonfun$createToCatalystConverter$2.apply(CatalystTypeConverters.scala:401)
    	at org.apache.spark.sql.SQLContext$$anonfun$beansToRows$1$$anonfun$apply$1.apply(SQLContext.scala:892)
    	at org.apache.spark.sql.SQLContext$$anonfun$beansToRows$1$$anonfun$apply$1.apply(SQLContext.scala:892)
    	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
    	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
    	at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
    	at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:186)
    	at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
    	at scala.collection.mutable.ArrayOps$ofRef.map(ArrayOps.scala:186)
    	at org.apache.spark.sql.SQLContext$$anonfun$beansToRows$1.apply(SQLContext.scala:892)
    	at org.apache.spark.sql.SQLContext$$anonfun$beansToRows$1.apply(SQLContext.scala:890)
    	at scala.collection.Iterator$$anon$11.next(Iterator.scala:409)
    	at scala.collection.Iterator$class.toStream(Iterator.scala:1322)
    	at scala.collection.AbstractIterator.toStream(Iterator.scala:1336)
    	at scala.collection.TraversableOnce$class.toSeq(TraversableOnce.scala:298)
    	at scala.collection.AbstractIterator.toSeq(Iterator.scala:1336)
    	at org.apache.spark.sql.SparkSession.createDataFrame(SparkSession.scala:373)
    	at test.org.apache.spark.sql.JavaDataFrameSuite.testCreateDataFrameFromLocalJavaBeans(JavaDataFrameSuite.java:200)


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by tedyu <gi...@git.apache.org>.

Github user tedyu commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220716220
  
    Looks like bigintval.longValue() should have been used.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219120384
  
    **[Test build #58588 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58588/consoleFull)** for PR 10125 at commit [`741daff`](https://github.com/apache/spark/commit/741daffd4db0462792a92e0fb3097a461777f3e2).


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220375820
  
    **[Test build #58872 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58872/consoleFull)** for PR 10125 at commit [`3b4e360`](https://github.com/apache/spark/commit/3b4e3608b2c4eb69940ceb1a4949b97814534b51).


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220361041
  
    I will push the latest one after jenkins finish. Thanks very much !


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219141188
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58588/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

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

    https://github.com/apache/spark/pull/10125#discussion_r47871490
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -69,6 +68,16 @@ final class Decimal extends Ordered[Decimal] with Serializable {
       }
     
       /**
    +   * Set this Decimal to the given BigInt. Will have precision 38 and scale 0.
    +   */
    +  def set(intVal: BigInt): Decimal = {
    +    this.decimalVal = null
    +    this.longVal = intVal.toLong
    --- End diff --
    
    Hi Davies: Yes, we need to check the range, otherwise, it will cause overflow. Thanks. I will look into.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by cloud-fan <gi...@git.apache.org>.

Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/10125#discussion_r63835534
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala ---
    @@ -109,6 +109,7 @@ object DecimalType extends AbstractDataType {
       val MAX_SCALE = 38
       val SYSTEM_DEFAULT: DecimalType = DecimalType(MAX_PRECISION, 18)
       val USER_DEFAULT: DecimalType = DecimalType(10, 0)
    +  val BIGINT_DEFAULT: DecimalType = DecimalType(MAX_PRECISION, 0)
    --- End diff --
    
    please add a `private[sql] val BigIntegerDecimal = DecimalType(38, 0)` to the next section, instead of doing 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219136788
  
    I just run the ./dev/mima locally, it works,
    [info] Done packaging.
    [info] spark-examples: previous-artifact not set, not analyzing binary compatibility
    [info] spark-mllib: found 0 potential binary incompatibilities while checking against org.apache.spark:spark-mllib_2.11:1.6.0  (filtered 500)
    [info] spark-sql: found 0 potential binary incompatibilities while checking against org.apache.spark:spark-sql_2.11:1.6.0  (filtered 752)
    [success] Total time: 231 s, completed May 13, 2016 12:22:16 PM


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-164616756
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47689/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-164616643
  
    **[Test build #47689 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47689/consoleFull)** for PR 10125 at commit [`1f77804`](https://github.com/apache/spark/commit/1f77804ea3120997f9c3b03d1cd4ca65e0fa101e).
     * 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

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

    https://github.com/apache/spark/pull/10125#discussion_r63900146
  
    --- Diff: sql/core/src/test/java/test/org/apache/spark/sql/JavaDataFrameSuite.java ---
    @@ -182,6 +189,8 @@ void validateDataFrameWithBeans(Bean bean, Dataset<Row> df) {
         for (int i = 0; i < d.length(); i++) {
           Assert.assertEquals(bean.getD().get(i), d.apply(i));
         }
    +      // Java.math.BigInteger is equavient to Spark Decimal(38,0)
    +    Assert.assertEquals(new BigDecimal(bean.getE()), first.getDecimal(4).setScale(0));
    --- End diff --
    
    will remove that.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219885069
  
    **[Test build #58721 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58721/consoleFull)** for PR 10125 at commit [`bbed47a`](https://github.com/apache/spark/commit/bbed47aabe572d7f91d0d011249df4a9624f0f97).
     * This patch **fails Spark unit tests**.
     * This patch **does not merge 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219119071
  
    **[Test build #58587 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58587/consoleFull)** for PR 10125 at commit [`ae0be70`](https://github.com/apache/spark/commit/ae0be70734003cb0281f07a08f6af7c030475360).


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219963862
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58754/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219888690
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58723/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220373429
  
    **[Test build #58869 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58869/consoleFull)** for PR 10125 at commit [`43faed3`](https://github.com/apache/spark/commit/43faed35bc8ca3c5bcff573509d45bfecc583831).
     * 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219888687
  
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220400980
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58872/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219140939
  
    **[Test build #58588 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58588/consoleFull)** for PR 10125 at commit [`741daff`](https://github.com/apache/spark/commit/741daffd4db0462792a92e0fb3097a461777f3e2).
     * 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-217526107
  
    @srowen: sorry for the long delay. I will work on it now.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220373752
  
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-161707755
  
    Hello Sean: I am sorry, I forgot to update the title and description. I have made the changes, please let me know if anything needs to be changed. Thanks.
    Kevin


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

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

    https://github.com/apache/spark/pull/10125#discussion_r47866879
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala ---
    @@ -326,6 +326,7 @@ object CatalystTypeConverters {
           val decimal = scalaValue match {
             case d: BigDecimal => Decimal(d)
             case d: JavaBigDecimal => Decimal(d)
    +        case d: BigInteger => Decimal(d)
    --- End diff --
    
    Hi Wenchen: Sure, I will add that.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220400560
  
    **[Test build #58872 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58872/consoleFull)** for PR 10125 at commit [`3b4e360`](https://github.com/apache/spark/commit/3b4e3608b2c4eb69940ceb1a4949b97814534b51).
     * 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

Github user kevinyu98 commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219945586
  
    I just did rebase to solve the conflict. 



---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by cloud-fan <gi...@git.apache.org>.

Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/10125#discussion_r63276327
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala ---
    @@ -321,11 +323,13 @@ object CatalystTypeConverters {
       }
     
       private class DecimalConverter(dataType: DecimalType)
    -    extends CatalystTypeConverter[Any, JavaBigDecimal, Decimal] {
    +  extends CatalystTypeConverter[Any, JavaBigDecimal, Decimal] {
    --- End diff --
    
    Why change this? I think we should use encoders most of the time.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220400977
  
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219120938
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58587/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by kevinyu98 <gi...@git.apache.org>.

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

    https://github.com/apache/spark/pull/10125#discussion_r63284859
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -129,6 +129,23 @@ 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 = {
    --- End diff --
    
    I will change it.


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220345648
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58868/
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219963582
  
    **[Test build #58754 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58754/consoleFull)** for PR 10125 at commit [`db4bb48`](https://github.com/apache/spark/commit/db4bb48fead3b8324e291ad7f5cf776fd88c9def).
     * 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: [SPARK-11827] [SQL] Adding java.math.BigIntege...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219938017
  
    **[Test build #58749 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58749/consoleFull)** for PR 10125 at commit [`9aa8c25`](https://github.com/apache/spark/commit/9aa8c25b687f64d15b5f1e086c5e2f1313e8bfb3).


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by tedyu <gi...@git.apache.org>.

Github user tedyu commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-220720146
  
    See #13233


---
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-11827] [SQL] Adding java.math.BigIntege...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10125#issuecomment-219952272
  
    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-11827] [SQL] Adding java.math.BigIntege...

Posted by davies <gi...@git.apache.org>.

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

    https://github.com/apache/spark/pull/10125#discussion_r47681350
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -69,6 +68,16 @@ final class Decimal extends Ordered[Decimal] with Serializable {
       }
     
       /**
    +   * Set this Decimal to the given BigInt. Will have precision 38 and scale 0.
    +   */
    +  def set(intVal: BigInt): Decimal = {
    +    this.decimalVal = null
    +    this.longVal = intVal.toLong
    --- End diff --
    
    Should we check the range before call `toLong`?


---
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