You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by bkrieger <gi...@git.apache.org> on 2018/06/07 20:17:26 UTC

[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

GitHub user bkrieger opened a pull request:

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

    [SPARK-24488] [SQL] Fix issue when generator is aliased multiple times

    ## What changes were proposed in this pull request?
    
    Currently, the Analyzer throws an exception if your try to nest a generator. However, it special cases generators "nested" in an alias, and allows that. If you try to alias a generator twice, it is not caught by the special case, so an exception is thrown.
    
    This PR trims the unnecessary, non-top-level aliases, so that the generator is allowed.
    
    
    ## How was this patch tested?
    
    new tests in AnalysisSuite.

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

    $ git pull https://github.com/bkrieger/spark bk/SPARK-24488

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

    https://github.com/apache/spark/pull/21508.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 #21508
    
----
commit 44ae34d0387f763936cddeceae64ee98b7bb279f
Author: Brandon Krieger <bk...@...>
Date:   2018-06-07T20:09:09Z

    SPARK-24488

----


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    @gatorsmile @hvanhovell, I'm working with @bkrieger and we need this patch soon. May we please get a sign off or else any suggested changes here?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194082850
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,13 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      CleanupAliases.trimNonTopLevelAliases(expr) match {
    --- End diff --
    
    Updated to handle the `MultiAlias` and `UnresolvedAlias`, and updated the unit test to test all 3.


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194023397
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,13 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      CleanupAliases.trimNonTopLevelAliases(expr) match {
    --- End diff --
    
    `CleanupAliases.trimNonTopLevelAliases` only strips `Alias` expressions. Should we also handle the other two cases?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by bkrieger <gi...@git.apache.org>.
Github user bkrieger commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    @gatorsmile @hvanhovell is this good to merge?


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194537508
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,13 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      CleanupAliases.trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    --- End diff --
    
    If we do not have a valid case here, we should not add it. Here, I think we just need to handle the resolved alias. 


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194539357
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,13 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      CleanupAliases.trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    --- End diff --
    
    done


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by bkrieger <gi...@git.apache.org>.
Github user bkrieger commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    @gatorsmile @hvanhovell Gentle ping. Let me know if there's someone else who would be better to review.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by bkrieger <gi...@git.apache.org>.
Github user bkrieger commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    @gatorsmile @hvanhovell can you take another look at this?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91568 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91568/testReport)** for PR 21508 at commit [`46c4a55`](https://github.com/apache/spark/commit/46c4a55ea465a0a83112a6ce730175c293bef40b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91564 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91564/testReport)** for PR 21508 at commit [`44ae34d`](https://github.com/apache/spark/commit/44ae34d0387f763936cddeceae64ee98b7bb279f).


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91725 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91725/testReport)** for PR 21508 at commit [`e9605dc`](https://github.com/apache/spark/commit/e9605dc8af62f2e834901e5b1bc1363cadb73a43).


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91674 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91674/testReport)** for PR 21508 at commit [`5d5e8e5`](https://github.com/apache/spark/commit/5d5e8e56bdd2701a965fe4ec1715efe87a955acf).


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91569 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91569/testReport)** for PR 21508 at commit [`f174263`](https://github.com/apache/spark/commit/f174263e4eb3f00e4870a086bb0fc69bb78f21cc).


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

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


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #93177 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93177/testReport)** for PR 21508 at commit [`3021918`](https://github.com/apache/spark/commit/302191895353b7aa2664474ca26cb4c98aaffb9f).


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91564 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91564/testReport)** for PR 21508 at commit [`44ae34d`](https://github.com/apache/spark/commit/44ae34d0387f763936cddeceae64ee98b7bb279f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91665 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91665/testReport)** for PR 21508 at commit [`abd1457`](https://github.com/apache/spark/commit/abd1457123595364c76f0808b6c3a8ffab59da5b).


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194274604
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,32 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    +        case Alias(_: Generator, _) => false
    +        case MultiAlias(_: Generator, _) => false
    +        case other => hasGenerator(other)
    +      }
    +    }
    +
    +    def trimNonTopLevelAliases(e: Expression): Expression = e match {
    --- End diff --
    
    Sure- I didn't want to break any existing functionality, but I can do that instead. 


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    ok to test


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194273874
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,32 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    +        case Alias(_: Generator, _) => false
    +        case MultiAlias(_: Generator, _) => false
    +        case other => hasGenerator(other)
    +      }
    +    }
    +
    +    def trimNonTopLevelAliases(e: Expression): Expression = e match {
    +      case a: UnresolvedAlias =>
    --- End diff --
    
    Do we need to handle `UnresolvedAlias`? 


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by bkrieger <gi...@git.apache.org>.
Github user bkrieger commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    The test failure looks like a flake to me?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194436381
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,32 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    +        case Alias(_: Generator, _) => false
    +        case MultiAlias(_: Generator, _) => false
    +        case other => hasGenerator(other)
    +      }
    +    }
    +
    +    def trimNonTopLevelAliases(e: Expression): Expression = e match {
    --- End diff --
    
    done


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194467198
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,32 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    +        case Alias(_: Generator, _) => false
    +        case MultiAlias(_: Generator, _) => false
    +        case other => hasGenerator(other)
    +      }
    +    }
    +
    +    def trimNonTopLevelAliases(e: Expression): Expression = e match {
    +      case a: UnresolvedAlias =>
    --- End diff --
    
    I don't think it'll hurt to handle it.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by bkrieger <gi...@git.apache.org>.
Github user bkrieger commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    @gatorsmile @hvanhovell any chance you can take a look at this?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91725 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91725/testReport)** for PR 21508 at commit [`e9605dc`](https://github.com/apache/spark/commit/e9605dc8af62f2e834901e5b1bc1363cadb73a43).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `    logInfo(s\"Using output committer class $`
      * `public class JavaPowerIterationClusteringExample `
      * `class PowerIterationClustering(HasMaxIter, HasWeightCol, JavaParams, JavaMLReadable,`


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91674 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91674/testReport)** for PR 21508 at commit [`5d5e8e5`](https://github.com/apache/spark/commit/5d5e8e56bdd2701a965fe4ec1715efe87a955acf).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #93177 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93177/testReport)** for PR 21508 at commit [`3021918`](https://github.com/apache/spark/commit/302191895353b7aa2664474ca26cb4c98aaffb9f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194539275
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,13 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      CleanupAliases.trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    --- End diff --
    
    `hasNestedGenerator` already handled `UnresolvedAlias`. I'll change `CleanupAliases` back to only handling resolved aliases.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    cc @maropu Help review this?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91665 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91665/testReport)** for PR 21508 at commit [`abd1457`](https://github.com/apache/spark/commit/abd1457123595364c76f0808b6c3a8ffab59da5b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91568 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91568/testReport)** for PR 21508 at commit [`46c4a55`](https://github.com/apache/spark/commit/46c4a55ea465a0a83112a6ce730175c293bef40b).


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    **[Test build #91569 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91569/testReport)** for PR 21508 at commit [`f174263`](https://github.com/apache/spark/commit/f174263e4eb3f00e4870a086bb0fc69bb78f21cc).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194273780
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,32 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    +        case Alias(_: Generator, _) => false
    +        case MultiAlias(_: Generator, _) => false
    +        case other => hasGenerator(other)
    +      }
    +    }
    +
    +    def trimNonTopLevelAliases(e: Expression): Expression = e match {
    --- End diff --
    
    Instead of duplicating the function here, could we just fixing `CleanupAliases.trimNonTopLevelAliases`


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

    https://github.com/apache/spark/pull/21508
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

Posted by bkrieger <gi...@git.apache.org>.
Github user bkrieger commented on the issue:

    https://github.com/apache/spark/pull/21508
  
    @gatorsmile @hvanhovell can you take a last look at this? I think it's good to merge.


---

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


[GitHub] spark issue #21508: [SPARK-24488] [SQL] Fix issue when generator is aliased ...

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

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


---

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


[GitHub] spark pull request #21508: [SPARK-24488] [SQL] Fix issue when generator is a...

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

    https://github.com/apache/spark/pull/21508#discussion_r194274619
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1568,11 +1568,32 @@ class Analyzer(
           expr.find(_.isInstanceOf[Generator]).isDefined
         }
     
    -    private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
    -      case UnresolvedAlias(_: Generator, _) => false
    -      case Alias(_: Generator, _) => false
    -      case MultiAlias(_: Generator, _) => false
    -      case other => hasGenerator(other)
    +    private def hasNestedGenerator(expr: NamedExpression): Boolean = {
    +      trimNonTopLevelAliases(expr) match {
    +        case UnresolvedAlias(_: Generator, _) => false
    +        case Alias(_: Generator, _) => false
    +        case MultiAlias(_: Generator, _) => false
    +        case other => hasGenerator(other)
    +      }
    +    }
    +
    +    def trimNonTopLevelAliases(e: Expression): Expression = e match {
    +      case a: UnresolvedAlias =>
    --- End diff --
    
    In my use case, no. But I wasn't sure if another use case would care. 


---

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