You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/02/03 15:34:25 UTC

[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

GitHub user gatorsmile opened a pull request:

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

    [SPARK-12725] [SQL] Resolving Name Conflicts in SQL Generation by Adding a flag `isGenerated` to Alias and AttributeReference

    Some analysis rules generate auxiliary attribute references with the same name but different expression IDs. For example, `ResolveAggregateFunctions` introduces `havingCondition` and `aggOrder`, and `DistinctAggregationRewriter` introduces `gid`.
    
    This is OK for normal query execution since these attribute references get expression IDs. However, it's troublesome when converting resolved query plans back to SQL query strings since expression IDs are erased.
    
    Here's an example Spark 1.6.0 snippet for illustration:
    ```scala
    sqlContext.range(10).select('id as 'a, 'id as 'b).registerTempTable("t")
    sqlContext.sql("SELECT SUM(a) FROM t GROUP BY a, b ORDER BY COUNT(a), COUNT(b)").explain(true)
    ```
    The above code produces the following resolved plan:
    ```
    == Analyzed Logical Plan ==
    _c0: bigint
    Project [_c0#101L]
    +- Sort [aggOrder#102L ASC,aggOrder#103L ASC], true
       +- Aggregate [a#47L,b#48L], [(sum(a#47L),mode=Complete,isDistinct=false) AS _c0#101L,(count(a#47L),mode=Complete,isDistinct=false) AS aggOrder#102L,(count(b#48L),mode=Complete,isDistinct=false) AS aggOrder#103L]
          +- Subquery t
             +- Project [id#46L AS a#47L,id#46L AS b#48L]
                +- LogicalRDD [id#46L], MapPartitionsRDD[44] at range at <console>:26
    ```
    Here we can see that both aggregate expressions in `ORDER BY` are extracted into an `Aggregate` operator, and both of them are named `aggOrder` with different expression IDs.
    
    Solution is to automatically add the expression IDs for the Alias and AttributeReferences that are generated by Analyzer in SQL Generation. 
    
    Could you review the solution? @marmbrus @liancheng 
    
    I did not set the newly added flag for all the alias and attribute reference generated by Analyzers. Please let me know if I should do it? Thank you! 

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

    $ git pull https://github.com/gatorsmile/spark namingConflicts

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

    https://github.com/apache/spark/pull/11050.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 #11050
    
----
commit 7937d2be2e163736ce90857bf6eb4209001e32e5
Author: gatorsmile <ga...@gmail.com>
Date:   2016-02-02T07:35:06Z

    turn on the test.

commit 82bb46fefd69a74f699ff97be5e63a866c318a80
Author: gatorsmile <ga...@gmail.com>
Date:   2016-02-03T14:20:09Z

    added a flag isGenerated to Alias and AttributeReference

----


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180882339
  
    **[Test build #50882 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50882/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179564713
  
    **[Test build #50704 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50704/consoleFull)** for PR 11050 at commit [`cf89603`](https://github.com/apache/spark/commit/cf8960356f0040d0fdbb302b48fdce8a1ee221ae).
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179388873
  
    @marmbrus Do you still remember the JIRA number or the case? If so, I can add it into the test cases and verify if the fix works well. 
    
    In this PR, `isGenerated` is added to `AttributeReference` and `Alias`. `AttributeReference` is always resolved. `Alias` is not resolved unless the child has been resolved, and thus, I think we are unable to skip resolving its child even if the `Alias` is added by Analyzer. Please correct me if my understanding is wrong. Thank you! 


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180992987
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50893/
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51977477
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -413,9 +413,11 @@ class Analyzer(
                 case UnresolvedAlias(f @ UnresolvedFunction(_, args, _), _) if containsStar(args) =>
                   val newChildren = expandStarExpressions(args, child)
                   UnresolvedAlias(child = f.copy(children = newChildren)) :: Nil
    -            case Alias(f @ UnresolvedFunction(_, args, _), name) if containsStar(args) =>
    +            case a @ Alias(f @ UnresolvedFunction(_, args, _), name)
    +                if containsStar(args) =>
    --- End diff --
    
    Nit: This change can be reverted 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-12725] [SQL] Resolving Name Conflicts i...

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

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


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181501876
  
    **[Test build #50926 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50926/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180828144
  
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r52088011
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -114,16 +117,21 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      * Note that exprId and qualifiers are in a separate parameter list because
      * we only pattern match on child and name.
      *
    - * @param child the computation being performed
    - * @param name the name to be associated with the result of computing [[child]].
    + * @param child The computation being performed
    + * @param name The name to be associated with the result of computing [[child]].
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers A list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
    + * @param isGenerated A flag to indicate if this alias is generated by Catalyst
      */
     case class Alias(child: Expression, name: String)(
         val exprId: ExprId = NamedExpression.newExprId,
         val qualifiers: Seq[String] = Nil,
    -    val explicitMetadata: Option[Metadata] = None)
    +    val explicitMetadata: Option[Metadata] = None,
    +    override val isGenerated: Option[Boolean] = None)
    --- End diff --
    
    You can use a `java.lang.Boolean`.


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180846748
  
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180992986
  
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181237467
  
    **[Test build #50908 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50908/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).
     * This patch **fails from timeout after a configured wait of \`250m\`**.
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-182673514
  
    LGTM, merging to master. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51977520
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -114,16 +117,21 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      * Note that exprId and qualifiers are in a separate parameter list because
      * we only pattern match on child and name.
      *
    - * @param child the computation being performed
    - * @param name the name to be associated with the result of computing [[child]].
    + * @param child The computation being performed
    + * @param name The name to be associated with the result of computing [[child]].
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers A list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
    + * @param isGenerated A flag to indicate if this alias is generated by Catalyst
      */
     case class Alias(child: Expression, name: String)(
         val exprId: ExprId = NamedExpression.newExprId,
         val qualifiers: Seq[String] = Nil,
    -    val explicitMetadata: Option[Metadata] = None)
    +    val explicitMetadata: Option[Metadata] = None,
    +    override val isGenerated: Option[Boolean] = None)
    --- End diff --
    
    Why does it have to be an `Option[Boolean]` instead of `Boolean`?


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180894828
  
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51977660
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -197,7 +209,8 @@ case class AttributeReference(
         nullable: Boolean = true,
         override val metadata: Metadata = Metadata.empty)(
         val exprId: ExprId = NamedExpression.newExprId,
    -    val qualifiers: Seq[String] = Nil)
    +    val qualifiers: Seq[String] = Nil,
    +    override val isGenerated: Option[Boolean] = None)
    --- End diff --
    
    Same here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51761274
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -116,11 +116,15 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      *
      * @param child the computation being performed
      * @param name the name to be associated with the result of computing [[child]].
    + * @param isGenerated a flag to indicate if this alias is generated by Catalyst
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers a list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
      */
    -case class Alias(child: Expression, name: String)(
    +case class Alias(child: Expression, name: String, isGenerated: Boolean = false)(
    --- End diff --
    
    Why not put `isGenerated` into the second list with the other metadata so that we don't have to rewrite every match statement in the code base?


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180992952
  
    **[Test build #50893 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50893/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179522483
  
    **[Test build #50695 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50695/consoleFull)** for PR 11050 at commit [`22cf88a`](https://github.com/apache/spark/commit/22cf88ac0d30e973189ce66fa348a6e84ae00521).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179528336
  
    I guess the PR you mentioned is https://github.com/apache/spark/pull/8231
    
    I think we should disallow users to accidentally use the internally generated names. Thus, adding the following test case to verify if the fix works well. 
    ```
    errorTest(
       "unresolved attributes with a generated name",
       testRelation2.groupBy('a)(max('b))
       .where(sum('b) > 0)
       .orderBy('havingCondition.asc),
       "cannot resolve" :: "havingCondition" :: Nil)
    ```


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179332729
  
    **[Test build #50662 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50662/consoleFull)** for PR 11050 at commit [`82bb46f`](https://github.com/apache/spark/commit/82bb46fefd69a74f699ff97be5e63a866c318a80).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class Alias(child: Expression, name: String, isGenerated: Boolean = false)(`


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180846515
  
    **[Test build #50875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50875/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180879283
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179558494
  
    **[Test build #50695 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50695/consoleFull)** for PR 11050 at commit [`22cf88a`](https://github.com/apache/spark/commit/22cf88ac0d30e973189ce66fa348a6e84ae00521).
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181237627
  
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181178930
  
    **[Test build #50908 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50908/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179531337
  
    Yeah, that test looks great.


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181237630
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50908/
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181502533
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51980394
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -114,16 +117,21 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      * Note that exprId and qualifiers are in a separate parameter list because
      * we only pattern match on child and name.
      *
    - * @param child the computation being performed
    - * @param name the name to be associated with the result of computing [[child]].
    + * @param child The computation being performed
    + * @param name The name to be associated with the result of computing [[child]].
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers A list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
    + * @param isGenerated A flag to indicate if this alias is generated by Catalyst
      */
     case class Alias(child: Expression, name: String)(
         val exprId: ExprId = NamedExpression.newExprId,
         val qualifiers: Seq[String] = Nil,
    -    val explicitMetadata: Option[Metadata] = None)
    +    val explicitMetadata: Option[Metadata] = None,
    +    override val isGenerated: Option[Boolean] = None)
    --- End diff --
    
    ```scala
    override protected final def otherCopyArgs: Seq[AnyRef]
    ```
    Since this is part of the second list, I have to add it into this `otherCopyArgs`. `Boolean` is not `AnyRef`. A lot of code changes are required to convert `AnyRef` to `Any`. Thus, I used `Option[Boolean]` here. Do you have any better suggestions? Can I keep it unchanged? Thanks!  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180974601
  
    **[Test build #50893 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50893/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179565153
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r55914401
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -174,7 +184,8 @@ case class Alias(child: Expression, name: String)(
       override def sql: String = {
         val qualifiersString =
           if (qualifiers.isEmpty) "" else qualifiers.map("`" + _ + "`").mkString("", ".", ".")
    -    s"${child.sql} AS $qualifiersString`$name`"
    +    val aliasName = if (isGenerated) s"$name#${exprId.id}" else s"$name"
    --- End diff --
    
    Why do we need to change the `sql` according to `isGenerate`?


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51977612
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -114,16 +117,21 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      * Note that exprId and qualifiers are in a separate parameter list because
      * we only pattern match on child and name.
      *
    - * @param child the computation being performed
    - * @param name the name to be associated with the result of computing [[child]].
    + * @param child The computation being performed
    + * @param name The name to be associated with the result of computing [[child]].
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers A list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
    + * @param isGenerated A flag to indicate if this alias is generated by Catalyst
      */
     case class Alias(child: Expression, name: String)(
         val exprId: ExprId = NamedExpression.newExprId,
         val qualifiers: Seq[String] = Nil,
    -    val explicitMetadata: Option[Metadata] = None)
    +    val explicitMetadata: Option[Metadata] = None,
    +    override val isGenerated: Option[Boolean] = None)
    --- End diff --
    
    Seems that a `Boolean` field defaulting to `false` should be enough?


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r52096964
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -114,16 +117,21 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      * Note that exprId and qualifiers are in a separate parameter list because
      * we only pattern match on child and name.
      *
    - * @param child the computation being performed
    - * @param name the name to be associated with the result of computing [[child]].
    + * @param child The computation being performed
    + * @param name The name to be associated with the result of computing [[child]].
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers A list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
    + * @param isGenerated A flag to indicate if this alias is generated by Catalyst
      */
     case class Alias(child: Expression, name: String)(
         val exprId: ExprId = NamedExpression.newExprId,
         val qualifiers: Seq[String] = Nil,
    -    val explicitMetadata: Option[Metadata] = None)
    +    val explicitMetadata: Option[Metadata] = None,
    +    override val isGenerated: Option[Boolean] = None)
    --- End diff --
    
    Thank you, will make a change.


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51761850
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -116,11 +116,15 @@ abstract class Attribute extends LeafExpression with NamedExpression {
      *
      * @param child the computation being performed
      * @param name the name to be associated with the result of computing [[child]].
    + * @param isGenerated a flag to indicate if this alias is generated by Catalyst
      * @param exprId A globally unique id used to check if an [[AttributeReference]] refers to this
      *               alias. Auto-assigned if left blank.
    + * @param qualifiers a list of strings that can be used to referred to this attribute in a fully
    + *                   qualified way. Consider the examples tableName.name, subQueryAlias.name.
    + *                   tableName and subQueryAlias are possible qualifiers.
      * @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
      */
    -case class Alias(child: Expression, name: String)(
    +case class Alias(child: Expression, name: String, isGenerated: Boolean = false)(
    --- End diff --
    
    uh, I see. Will do it. : )


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180846758
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50875/
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180827424
  
    **[Test build #50875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50875/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181175638
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179393445
  
    @marmbrus : ) Now I understand it. Will do it. Thank you!


---
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-12725] [SQL] Resolving Name Conflicts i...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r55914872
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -174,7 +184,8 @@ case class Alias(child: Expression, name: String)(
       override def sql: String = {
         val qualifiersString =
           if (qualifiers.isEmpty) "" else qualifiers.map("`" + _ + "`").mkString("", ".", ".")
    -    s"${child.sql} AS $qualifiersString`$name`"
    +    val aliasName = if (isGenerated) s"$name#${exprId.id}" else s"$name"
    --- End diff --
    
    Yeah. You can get more background from the discussion in the JIRA: https://issues.apache.org/jira/browse/SPARK-12725


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r51761300
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -195,7 +202,8 @@ case class AttributeReference(
         name: String,
         dataType: DataType,
         nullable: Boolean = true,
    -    override val metadata: Metadata = Metadata.empty)(
    +    override val metadata: Metadata = Metadata.empty,
    +    isGenerated: Boolean = false)(
    --- End diff --
    
    Same here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179273624
  
    **[Test build #50662 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50662/consoleFull)** for PR 11050 at commit [`82bb46f`](https://github.com/apache/spark/commit/82bb46fefd69a74f699ff97be5e63a866c318a80).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179382231
  
    We should also modify the `resolve` function to filter out attributes where `isGenerated = true`.  There have been bugs in the past where this creates ambiguity and we can finally fix that.


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180894832
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50882/
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180894176
  
    **[Test build #50882 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50882/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).
     * 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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179531114
  
    **[Test build #50704 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50704/consoleFull)** for PR 11050 at commit [`cf89603`](https://github.com/apache/spark/commit/cf8960356f0040d0fdbb302b48fdce8a1ee221ae).


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179558924
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179391285
  
    I'd have to search for it as it was a while ago, you should be able to trigger it by forcing the analyzer to generate a column while referencing a column with the same name.
    
    I wasn't referring to the `resolved` property of an `Expression`, but instead the [`resolve` function](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala#L178) on a `LogicalPlan`.  This should probably never return attributes that are generated.


---
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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180828145
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50874/
    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-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-179333780
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-180972695
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#discussion_r55914448
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala ---
    @@ -174,7 +184,8 @@ case class Alias(child: Expression, name: String)(
       override def sql: String = {
         val qualifiersString =
           if (qualifiers.isEmpty) "" else qualifiers.map("`" + _ + "`").mkString("", ".", ".")
    -    s"${child.sql} AS $qualifiersString`$name`"
    +    val aliasName = if (isGenerated) s"$name#${exprId.id}" else s"$name"
    --- End diff --
    
    ah, I see. This is to workaround an issue in SQLBuilder


---
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-12725] [SQL] Resolving Name Conflicts i...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181436878
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-12725] [SQL] Resolving Name Conflicts i...

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

    https://github.com/apache/spark/pull/11050#issuecomment-181443489
  
    **[Test build #50926 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50926/consoleFull)** for PR 11050 at commit [`4e00349`](https://github.com/apache/spark/commit/4e003499d3b6f10f59a816f0f4dd6accf829255d).


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