You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by cloud-fan <gi...@git.apache.org> on 2015/11/16 06:19:49 UTC

[GitHub] spark pull request: [SPARK-11750][SQL] revert SPARK-11727 and code...

GitHub user cloud-fan opened a pull request:

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

    [SPARK-11750][SQL] revert SPARK-11727 and code clean up

    After some experiment, I found it's not convenient to have separate encoder builders: `FlatEncoder` and `ProductEncoder`. For example, when create encoders for `ScalaUDF`, we have no idea if the type `T` is flat or not. So I revert the splitting change in https://github.com/apache/spark/pull/9693, while still keeping the bug fixes and tests.
    
    This PR also did some code clean up, like removing `trait ScalaReflection` and put all methods just in `object ScalaReflection`.

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

    $ git pull https://github.com/cloud-fan/spark follow

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

    https://github.com/apache/spark/pull/9726.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 #9726
    
----
commit 6dbc1247f6f4a3a0ac50828ec64672a793a76c7b
Author: Wenchen Fan <we...@databricks.com>
Date:   2015-11-16T05:04:26Z

    revert #11727

----


---
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-11750][SQL] revert SPARK-11727 and code...

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

    https://github.com/apache/spark/pull/9726#issuecomment-156945880
  
    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-11750][SQL] revert SPARK-11727 and code...

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

    https://github.com/apache/spark/pull/9726#issuecomment-156945797
  
    **[Test build #45980 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45980/consoleFull)** for PR 9726 at commit [`6dbc124`](https://github.com/apache/spark/commit/6dbc1247f6f4a3a0ac50828ec64672a793a76c7b).
     * 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-11750][SQL] revert SPARK-11727 and code...

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

    https://github.com/apache/spark/pull/9726#issuecomment-156919553
  
    **[Test build #45980 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45980/consoleFull)** for PR 9726 at commit [`6dbc124`](https://github.com/apache/spark/commit/6dbc1247f6f4a3a0ac50828ec64672a793a76c7b).


---
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-11750][SQL] revert SPARK-11727 and code...

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

    https://github.com/apache/spark/pull/9726#issuecomment-156945881
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45980/
    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-11750][SQL] revert SPARK-11727 and code...

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/9726#discussion_r44889853
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects.scala ---
    @@ -288,14 +287,13 @@ case class WrapOption(optionType: DataType, child: Expression)
         throw new UnsupportedOperationException("Only code-generated evaluation is supported")
     
       override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): String = {
    -    val javaType = ctx.javaType(optionType)
         val inputObject = child.gen(ctx)
     
         s"""
           ${inputObject.code}
     
           boolean ${ev.isNull} = false;
    -      scala.Option<$javaType> ${ev.value} =
    +      scala.Option ${ev.value} =
    --- End diff --
    
    Janino doesn't support generic type, so we don't need to pass in the `optionType`.


---
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-11750][SQL] revert SPARK-11727 and code...

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/9726#discussion_r44889803
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala ---
    @@ -718,39 +630,4 @@ trait ScalaReflection {
             throw new UnsupportedOperationException(s"Schema for type $other is not supported")
         }
       }
    -
    -  def typeOfObject: PartialFunction[Any, DataType] = {
    --- End diff --
    
    This method is only used in tests, so I think we should remove it and its related tests.


---
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-11750][SQL] revert SPARK-11727 and code...

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

    https://github.com/apache/spark/pull/9726#issuecomment-156918831
  
    cc @marmbrus 


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