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

[GitHub] spark pull request #22899: [SPARK-25573] Combine resolveExpression and resol...

GitHub user dilipbiswal opened a pull request:

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

    [SPARK-25573] Combine resolveExpression and resolve in the Analyzer

    ## What changes were proposed in this pull request?
    Currently in the Analyzer, we have two methods 1) Resolve 2)ResolveExpressions that are called at different code paths to resolve attributes, column ordinal and extract value expressions. In this PR, we combine the two into one method to make sure, there is only one method that is tasked with resolving the attributes.
    
    ## How was this patch tested?
    Existing tests.

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

    $ git pull https://github.com/dilipbiswal/spark SPARK-25573-final

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

    https://github.com/apache/spark/pull/22899.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 #22899
    
----
commit ff375690c7a884c52fa4683116570a964e46a96d
Author: Dilip Biswal <db...@...>
Date:   2018-10-31T05:36:30Z

    [SPARK-25573] Combine resolveExpression and resolve in the Analyzer

----


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #99633 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99633/testReport)** for PR 22899 at commit [`3a32007`](https://github.com/apache/spark/commit/3a320075e2749e5ff21fc6fef616406fd8756cc9).


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4662/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    retest this please


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    retest this please


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98314 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98314/testReport)** for PR 22899 at commit [`db092c6`](https://github.com/apache/spark/commit/db092c6a5259286627b8c3432bb961f2da3bfcaa).
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4909/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #99642 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99642/testReport)** for PR 22899 at commit [`45e314a`](https://github.com/apache/spark/commit/45e314aa33fb0c108f185519ff58db691c34d58c).
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #99642 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99642/testReport)** for PR 22899 at commit [`45e314a`](https://github.com/apache/spark/commit/45e314aa33fb0c108f185519ff58db691c34d58c).


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4651/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    cc @gatorsmile 


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5699/
    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 #22899: [SPARK-25573] Combine resolveExpression and resol...

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

    https://github.com/apache/spark/pull/22899#discussion_r229609343
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1051,30 +1034,65 @@ class Analyzer(
         func.map(wrapper)
       }
     
    +
    +  /**
    +   * Resolves the attribute, column ordinal and extract value expressions(s) by traversing the
    +   * input expression in bottom up manner. This routine skips over the unbound lambda function
    +   * expressions as the lambda variables are resolved in separate rule [[ResolveLambdaVariables]].
    +   */
       protected[sql] def resolveExpression(
           expr: Expression,
           plan: LogicalPlan,
    -      throws: Boolean = false): Expression = {
    -    if (expr.resolved) return expr
    -    // Resolve expression in one round.
    -    // If throws == false or the desired attribute doesn't exist
    -    // (like try to resolve `a.b` but `a` doesn't exist), fail and return the origin one.
    -    // Else, throw exception.
    -    try {
    -      expr transformUp {
    +      throws: Boolean = false,
    +      resolvedFromChildAttributes: Boolean = false): Expression = {
    +
    +    def resolveExpression(
    +      expr: Expression,
    +      plan: LogicalPlan,
    +      resolveFromChildAttributes: Boolean): Expression = {
    +      expr match {
             case GetColumnByOrdinal(ordinal, _) => plan.output(ordinal)
    -        case u @ UnresolvedAttribute(nameParts) =>
    -          withPosition(u) {
    -            plan.resolve(nameParts, resolver)
    -              .orElse(resolveLiteralFunction(nameParts, u, plan))
    -              .getOrElse(u)
    -          }
    +        case u @ UnresolvedAttribute(nameParts) if resolveFromChildAttributes =>
    +          // Leave unchanged if resolution fails. Hopefully will be resolved next round.
    +          val result =
    +            withPosition(u) {
    +              plan.resolveChildren(nameParts, resolver)
    --- End diff --
    
    Is there any way to fully replace `resolveChildren` by `resolve`? Just from the logic, `resolve` is next step of `resolveChildren` in recursion.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98675 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98675/testReport)** for PR 22899 at commit [`3a32007`](https://github.com/apache/spark/commit/3a320075e2749e5ff21fc6fef616406fd8756cc9).
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resol...

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

    https://github.com/apache/spark/pull/22899#discussion_r229608676
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -1051,30 +1034,65 @@ class Analyzer(
         func.map(wrapper)
       }
     
    +
    +  /**
    +   * Resolves the attribute, column ordinal and extract value expressions(s) by traversing the
    +   * input expression in bottom up manner. This routine skips over the unbound lambda function
    +   * expressions as the lambda variables are resolved in separate rule [[ResolveLambdaVariables]].
    +   */
       protected[sql] def resolveExpression(
           expr: Expression,
           plan: LogicalPlan,
    -      throws: Boolean = false): Expression = {
    -    if (expr.resolved) return expr
    -    // Resolve expression in one round.
    -    // If throws == false or the desired attribute doesn't exist
    -    // (like try to resolve `a.b` but `a` doesn't exist), fail and return the origin one.
    -    // Else, throw exception.
    -    try {
    -      expr transformUp {
    +      throws: Boolean = false,
    +      resolvedFromChildAttributes: Boolean = false): Expression = {
    +
    +    def resolveExpression(
    +      expr: Expression,
    +      plan: LogicalPlan,
    +      resolveFromChildAttributes: Boolean): Expression = {
    +      expr match {
             case GetColumnByOrdinal(ordinal, _) => plan.output(ordinal)
    -        case u @ UnresolvedAttribute(nameParts) =>
    -          withPosition(u) {
    -            plan.resolve(nameParts, resolver)
    -              .orElse(resolveLiteralFunction(nameParts, u, plan))
    -              .getOrElse(u)
    -          }
    +        case u @ UnresolvedAttribute(nameParts) if resolveFromChildAttributes =>
    --- End diff --
    
    Maybe put the `if(resolveFromChildAttributes)` into `case u @ UnresolvedAttribute(nameParts)` to reduce some code duplication?


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98674 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98674/testReport)** for PR 22899 at commit [`3a32007`](https://github.com/apache/spark/commit/3a320075e2749e5ff21fc6fef616406fd8756cc9).


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #99633 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99633/testReport)** for PR 22899 at commit [`3a32007`](https://github.com/apache/spark/commit/3a320075e2749e5ff21fc6fef616406fd8756cc9).
     * This patch **fails to build**.
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98293 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98293/testReport)** for PR 22899 at commit [`ff37569`](https://github.com/apache/spark/commit/ff375690c7a884c52fa4683116570a964e46a96d).
     * This patch **fails due to an unknown error code, -9**.
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    retest this please


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    LGTM
    
    Merged to master. 


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    retest this please


---

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


[GitHub] spark pull request #22899: [SPARK-25573] Combine resolveExpression and resol...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    @gatorsmile Thanks a lot. I completely agree that we should try and combine these two. I will continue to think about it :-)


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    cc @gatorsmile 


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4668/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4648/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98320 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98320/testReport)** for PR 22899 at commit [`db092c6`](https://github.com/apache/spark/commit/db092c6a5259286627b8c3432bb961f2da3bfcaa).
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98297 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98297/testReport)** for PR 22899 at commit [`ff37569`](https://github.com/apache/spark/commit/ff375690c7a884c52fa4683116570a964e46a96d).
     * 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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5690/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98675 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98675/testReport)** for PR 22899 at commit [`3a32007`](https://github.com/apache/spark/commit/3a320075e2749e5ff21fc6fef616406fd8756cc9).


---

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


[GitHub] spark pull request #22899: [SPARK-25573] Combine resolveExpression and resol...

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

    https://github.com/apache/spark/pull/22899#discussion_r238483571
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -880,21 +880,38 @@ class Analyzer(
           }
         }
     
    -    private def resolve(e: Expression, q: LogicalPlan): Expression = e match {
    -      case f: LambdaFunction if !f.bound => f
    -      case u @ UnresolvedAttribute(nameParts) =>
    -        // Leave unchanged if resolution fails. Hopefully will be resolved next round.
    -        val result =
    -          withPosition(u) {
    -            q.resolveChildren(nameParts, resolver)
    -              .orElse(resolveLiteralFunction(nameParts, u, q))
    -              .getOrElse(u)
    -          }
    -        logDebug(s"Resolving $u to $result")
    -        result
    -      case UnresolvedExtractValue(child, fieldExpr) if child.resolved =>
    -        ExtractValue(child, fieldExpr, resolver)
    -      case _ => e.mapChildren(resolve(_, q))
    +    /**
    +     * Resolves the attribute and extract value expressions(s) by traversing the
    +     * input expression in top down manner. The traversal is done in top-down manner as
    +     * we need to skip over unbound lamda function expression. The lamda expressions are
    +     * resolved in a different rule [[ResolveLambdaVariables]]
    +     *
    +     * Example :
    +     * SELECT transform(array(1, 2, 3), (x, i) -> x + i)"
    +     *
    +     * In the case above, x and i are resolved as lamda variables in [[ResolveLambdaVariables]]
    +     *
    +     * Note : In this routine, the unresolved attributes are resolved from the input plan's
    +     * children attributes.
    +     */
    +    private def resolveExpressionTopDown(e: Expression, q: LogicalPlan): Expression = {
    +      if (e.resolved) return e
    --- End diff --
    
    A good catch!


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

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


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    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 #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    To be honest, we might still need to revisit it since it is still very confusing to the developer which one they should use, top-down? or bottom-up? The current use case for top-down is majorly for resolving the higher order functions. 
    
    This PR at least improves the description. We might need to combine them in the future. 


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4908/
    Test PASSed.


---

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


[GitHub] spark issue #22899: [SPARK-25573] Combine resolveExpression and resolve in t...

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

    https://github.com/apache/spark/pull/22899
  
    **[Test build #98674 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98674/testReport)** for PR 22899 at commit [`3a32007`](https://github.com/apache/spark/commit/3a320075e2749e5ff21fc6fef616406fd8756cc9).
     * This patch **fails due to an unknown error code, -9**.
     * 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