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/03/01 09:05:01 UTC

[GitHub] spark pull request: [SPARK-13591] [SQL] Remove Back-ticks in Attri...

GitHub user gatorsmile opened a pull request:

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

    [SPARK-13591] [SQL] Remove Back-ticks in AttributeReference/Alias Names

    #### What changes were proposed in this pull request?
    
    When calling .sql, back-ticks are automatically added. When using .sql as AttributeReference/Alias names, we hit a couple of issues when doing logicalPlan to SQL. Parser is unable to recognize it.
    
    For example, the name could be converted to 
    ```
    `sum(`name`)`
    ```
    
    This PR is to follow the same way to remove back-ticks from names as what we did in https://github.com/apache/spark/pull/10757. 
    
    **Question: ** This is a common issue. I am afraid the future coders might miss that. Should we call `usePrettyExpression` every time when we creating an alias name or attributeReference name? Or add another function (e.g., .sqlToName) like `.sql`? Thanks!
    
    @liancheng @cloud-fan 
    
    #### How was this patch tested?
    N/A

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

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

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

    https://github.com/apache/spark/pull/11441.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 #11441
    
----
commit 7ed21df2d829ceedf7f016efe5a5d6d27c088ed3
Author: gatorsmile <ga...@gmail.com>
Date:   2016-03-01T07:48:35Z

    remove backsticks in attribute/alias names.

----


---
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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190883985
  
    Let me close it at first. It sounds like this is specific to multi-distinct. Let us do it when we fixing SQL generation for multi distinct.


---
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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190639892
  
    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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190754911
  
    @cloud-fan I hit this issue when prototyping conversion from logical plans to SQL for multi distinct. For example, 
    ```scala
      val operator = Alias(e.copy(aggregateFunction = af), e.sql)()
    ```
    
    I thought this could be a general issue for all the cases when we create `Alias` and `AttributeReference` internally. Then I did a code search and submitted this PR.
    
    Have to take a train to SF now. Will try to reproduce the issue without multi distinct. : )


---
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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190639258
  
    **[Test build #52229 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52229/consoleFull)** for PR 11441 at commit [`7ed21df`](https://github.com/apache/spark/commit/7ed21df2d829ceedf7f016efe5a5d6d27c088ed3).
     * 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-13591] [SQL] Remove Back-ticks in Attri...

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

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


---
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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190604894
  
    **[Test build #52229 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52229/consoleFull)** for PR 11441 at commit [`7ed21df`](https://github.com/apache/spark/commit/7ed21df2d829ceedf7f016efe5a5d6d27c088ed3).


---
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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-192623565
  
    One minor comment: the example generated name mentioned in the PR description should be ``` `sum(``name``)` ``` instead of `` `sum(`name`)` ``, since back-ticks are escaped by double-back-ticks.


---
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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-192657041
  
    Yeah, true. 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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190639895
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52229/
    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-13591] [SQL] Remove Back-ticks in Attri...

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

    https://github.com/apache/spark/pull/11441#issuecomment-190609741
  
    Do you have a test that fails because of it? It looks to me that even we use back-ticks as alias name, the `Alias.sql` will handle this case and produce valid SQL string.


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