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 2016/05/18 09:57:28 UTC

[GitHub] spark pull request: [SPARK-15381][SQL] physical object operator sh...

GitHub user cloud-fan opened a pull request:

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

    [SPARK-15381][SQL] physical object operator should define reference correctly

    ## What changes were proposed in this pull request?
    
    Whole Stage Codegen depends on `SparkPlan.reference` to do some optimization. For physical object operators, they should be consistent with their logical version and set the `reference` correctly.
    
    
    ## How was this patch tested?
    
    new test in DatasetSuite

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

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

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

    https://github.com/apache/spark/pull/13167.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 #13167
    
----
commit 052ad6bd95f742aa1f149081fb44af5a3d1760f9
Author: Wenchen Fan <we...@databricks.com>
Date:   2016-05-18T09:49:10Z

    physical object operator should define reference correctly

----


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220005056
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58773/
    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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219983762
  
    **[Test build #58770 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58770/consoleFull)** for PR 13167 at commit [`052ad6b`](https://github.com/apache/spark/commit/052ad6bd95f742aa1f149081fb44af5a3d1760f9).
     * This patch **fails MiMa tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `trait ObjectProducerExec extends SparkPlan `
      * `trait ObjectConsumerExec extends UnaryExecNode `
      * `case class DeserializeToObjectExec(`


---
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-15381][SQL] physical object operator sh...

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

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


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220225530
  
    **[Test build #58836 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58836/consoleFull)** for PR 13167 at commit [`a97e358`](https://github.com/apache/spark/commit/a97e3586b7b856d5a62981ff459f48da8d1128bb).
     * 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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220004819
  
    **[Test build #58773 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58773/consoleFull)** for PR 13167 at commit [`052ad6b`](https://github.com/apache/spark/commit/052ad6bd95f742aa1f149081fb44af5a3d1760f9).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `trait ObjectProducerExec extends SparkPlan `
      * `trait ObjectConsumerExec extends UnaryExecNode `
      * `case class DeserializeToObjectExec(`


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219983788
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58770/
    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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219981727
  
    **[Test build #58770 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58770/consoleFull)** for PR 13167 at commit [`052ad6b`](https://github.com/apache/spark/commit/052ad6bd95f742aa1f149081fb44af5a3d1760f9).


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220225648
  
    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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220005053
  
    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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220226195
  
    Merging this into master and 2.0, 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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220214210
  
    LGTM


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219987203
  
    retest this please


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219980833
  
    cc @davies @rxin @yhuai 


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220217395
  
    **[Test build #58836 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58836/consoleFull)** for PR 13167 at commit [`a97e358`](https://github.com/apache/spark/commit/a97e3586b7b856d5a62981ff459f48da8d1128bb).


---
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-15381][SQL] physical object operator sh...

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/13167#discussion_r63675936
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/objects.scala ---
    @@ -28,17 +28,41 @@ import org.apache.spark.sql.catalyst.expressions.objects.Invoke
     import org.apache.spark.sql.catalyst.plans.physical._
     import org.apache.spark.sql.types.{DataType, ObjectType}
     
    +
    +/**
    + * Physical version of `ObjectProducer`.
    + */
    +trait ObjectProducerExec extends SparkPlan {
    +  // The attribute that reference to the single object field this operator outputs.
    +  protected def outputObjAttr: Attribute
    +
    +  override def output: Seq[Attribute] = outputObjAttr :: Nil
    +
    +  override def producedAttributes: AttributeSet = AttributeSet(outputObjAttr)
    +
    +  def outputObjectType: DataType = outputObjAttr.dataType
    +}
    +
    +/**
    + * Physical version of `ObjectConsumer`.
    + */
    +trait ObjectConsumerExec extends UnaryExecNode {
    +  assert(child.output.length == 1)
    +
    +  // This operator always need all columns of its child, even it doesn't reference to.
    +  override def references: AttributeSet = child.outputSet
    +
    +  def inputObjectType: DataType = child.output.head.dataType
    +}
    +
     /**
      * Takes the input row from child and turns it into object using the given deserializer expression.
      * The output of this operator is a single-field safe row containing the deserialized object.
      */
    -case class DeserializeToObject(
    +case class DeserializeToObjectExec(
    --- End diff --
    
    not related, but to make the name consistent with other physical plans.


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-220225651
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58836/
    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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219983786
  
    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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#discussion_r63815763
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/objects.scala ---
    @@ -70,7 +94,7 @@ case class DeserializeToObject(
      */
     case class SerializeFromObjectExec(
         serializer: Seq[NamedExpression],
    -    child: SparkPlan) extends UnaryExecNode with CodegenSupport {
    +    child: SparkPlan) extends UnaryExecNode with ObjectConsumerExec with CodegenSupport {
    --- End diff --
    
    minor: ObjectConsumerExec already extend UnaryExecNode


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#issuecomment-219988062
  
    **[Test build #58773 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58773/consoleFull)** for PR 13167 at commit [`052ad6b`](https://github.com/apache/spark/commit/052ad6bd95f742aa1f149081fb44af5a3d1760f9).


---
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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#discussion_r63815785
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/objects.scala ---
    @@ -166,10 +187,7 @@ case class MapElementsExec(
         func: AnyRef,
         outputObjAttr: Attribute,
         child: SparkPlan)
    -  extends UnaryExecNode with ObjectOperator with CodegenSupport {
    -
    -  override def output: Seq[Attribute] = outputObjAttr :: Nil
    -  override def producedAttributes: AttributeSet = AttributeSet(outputObjAttr)
    +  extends UnaryExecNode with ObjectProducerExec with ObjectConsumerExec with CodegenSupport {
    --- End diff --
    
    same 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-15381][SQL] physical object operator sh...

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

    https://github.com/apache/spark/pull/13167#discussion_r63815797
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/objects.scala ---
    @@ -141,15 +165,12 @@ case class MapPartitionsExec(
         func: Iterator[Any] => Iterator[Any],
         outputObjAttr: Attribute,
         child: SparkPlan)
    -  extends UnaryExecNode with ObjectOperator {
    -
    -  override def output: Seq[Attribute] = outputObjAttr :: Nil
    -  override def producedAttributes: AttributeSet = AttributeSet(outputObjAttr)
    +  extends UnaryExecNode with ObjectProducerExec with ObjectConsumerExec {
    --- End diff --
    
    same 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