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

Posted to reviews@spark.apache.org by davies <gi...@git.apache.org> on 2015/12/29 23:01:39 UTC

[GitHub] spark pull request: [SPARK-12286] [SPARK-12290] [SPARK-12294] alwa...

GitHub user davies opened a pull request:

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

    [SPARK-12286] [SPARK-12290] [SPARK-12294] always output UnsafeRow

    It's confusing that some operator output UnsafeRow but some not, easy to make mistake.
    
    This PR change to only output UnsafeRow for all the operators (SparkPlan), removed the rule to insert Unsafe/Safe conversions. For those that can't output UnsafeRow directly, added UnsafeProjection into them.
    
    Closes #10330 
    
    cc @JoshRosen @rxin 

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

    $ git pull https://github.com/davies/spark unsafe_row

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

    https://github.com/apache/spark/pull/10511.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 #10511
    
----
commit 94fcd010ee9241976ad6509d80d26d7fffc6a251
Author: Davies Liu <da...@databricks.com>
Date:   2015-12-29T21:34:13Z

    always output UnsafeRow

----


---
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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167951626
  
    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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167914227
  
    **[Test build #48432 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48432/consoleFull)** for PR 10511 at commit [`513597c`](https://github.com/apache/spark/commit/513597c3172cec7e68bd15f9c543533248d1c3e3).
     * 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: [SPARK-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167967383
  
    **[Test build #2263 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2263/consoleFull)** for PR 10511 at commit [`90405fc`](https://github.com/apache/spark/commit/90405fc0fc4c932a62037498b79cacc1eca27842).
     * 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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user davies commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167947271
  
    @viirya I'm very sorry for this. Originally, I thought this could be done step by step, then finally do this clean up. After reviewed some of the PRs, realized that trying to make them work with both UnsafeRow and SafeRow just waster ours time (because we will remove the SafeRow support in the end), also waste our time in the process for tiny PRs.
    
    I just start to work on this in this morning, could left some comments about this earlier (but can't say much without actual prototyping).
    
    If you think it's worth to address the comments in your PR, go ahead to do that, I could merge your PR before this one (then rebase this PR).


---
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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user viirya commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167949764
  
    @davies Thanks for replying my comments. It is good to know what it is going on. Now it does feel better. It is ok for the closed PR. I just got confusing why after several updates to the PR it is getting closed by replacing one. I would like to suggest to put some words explaining briefly why to close other PRs with replacing one like this when creating it, if it is possibly.


---
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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user rxin commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-168468978
  
    It doesn't look like the failure is related to this -- so I reverted the revert commit ... now it is merged again.



---
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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user asfgit closed the pull request at:

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


---
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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user rxin commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-168339460
  
    I took a quick look and I'm going to merge this because I'm working on something that conflicts with it. It would be great if @nongli or @JoshRosen can review this more carefully post-hoc.



---
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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167901053
  
    **[Test build #48425 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48425/consoleFull)** for PR 10511 at commit [`cba3934`](https://github.com/apache/spark/commit/cba393448c2d581bd62e31d3181a11e290a2a83d).
     * 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: [SPARK-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167914254
  
    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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167951627
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48454/
    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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167951333
  
    **[Test build #48454 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48454/consoleFull)** for PR 10511 at commit [`6bc518c`](https://github.com/apache/spark/commit/6bc518c2b6f1149b4deccc4a496e9a1f88c4562b).


---
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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167901091
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48425/
    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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167955293
  
    **[Test build #2263 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2263/consoleFull)** for PR 10511 at commit [`90405fc`](https://github.com/apache/spark/commit/90405fc0fc4c932a62037498b79cacc1eca27842).


---
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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167954227
  
    **[Test build #48457 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48457/consoleFull)** for PR 10511 at commit [`90405fc`](https://github.com/apache/spark/commit/90405fc0fc4c932a62037498b79cacc1eca27842).


---
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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167901009
  
    **[Test build #48432 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48432/consoleFull)** for PR 10511 at commit [`513597c`](https://github.com/apache/spark/commit/513597c3172cec7e68bd15f9c543533248d1c3e3).


---
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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167914257
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48432/
    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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167888719
  
    **[Test build #48425 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48425/consoleFull)** for PR 10511 at commit [`cba3934`](https://github.com/apache/spark/commit/cba393448c2d581bd62e31d3181a11e290a2a83d).


---
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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user davies commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167946717
  
    @nongli InsertIntoHiveTable also does not support UnsafeRow 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-12286] [SPARK-12290] [SPARK-12294] [SQL...

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

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167901090
  
    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-12286] [SPARK-12290] [SPARK-12294] [SPA...

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

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/10511#issuecomment-167954741
  
    **[Test build #48457 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48457/consoleFull)** for PR 10511 at commit [`90405fc`](https://github.com/apache/spark/commit/90405fc0fc4c932a62037498b79cacc1eca27842).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class ASTErrorNode extends ASTNode `
      * `public class ASTNode extends CommonTree implements Node, Serializable `
      * `public class ParseDriver `
      * `  //function of the super class ANTLRStringStream. The LA() function is the lookahead function`
      * `  public class ANTLRNoCaseStringStream extends ANTLRStringStream `
      * `  public class HiveLexerX extends SparkSqlLexer `
      * `public class ParseError `
      * `public class ParseException extends Exception `
      * `public final class ParseUtils `
      * `public abstract class SemanticAnalyzer `


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