You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wzhfy <gi...@git.apache.org> on 2015/10/29 07:25:20 UTC

[GitHub] spark pull request: [SPARK-11398][SQL] misleading dialect conf at ...

GitHub user wzhfy opened a pull request:

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

    [SPARK-11398][SQL] misleading dialect conf at the start of spark-sql

    Instead of overriding def dialect in conf of HiveContext, I set the SQLConf.DIALECT directly as "hiveql", such that result of "set spark.sql.dialect;" will be "hiveql", not "sql".

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

    $ git pull https://github.com/wzhfy/spark dialect

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

    https://github.com/apache/spark/pull/9349.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 #9349
    
----
commit fa9667b815b2ff4cbbde87dfdd0e76816132e1bf
Author: Zhenhua Wang <wa...@huawei.com>
Date:   2015-10-29T02:19:59Z

    spark.sql.dialect should be hiveql at the start

commit de27ba3e81b96f9f0709e8650361c38b7e863a4e
Author: Zhenhua Wang <wa...@huawei.com>
Date:   2015-10-29T02:53:39Z

    testcase name

commit e9e23dabf3e80b534722ec492bbc7d2bf4eb6778
Author: Zhenhua Wang <wa...@huawei.com>
Date:   2015-10-29T05:59:53Z

    delete useless comment

----


---
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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152092653
  
    Can one of the admins verify this patch?


---
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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153589152
  
    **[Test build #1975 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1975/consoleFull)** for PR 9349 at commit [`d15e14a`](https://github.com/apache/spark/commit/d15e14ad7b817d874b904cc839d9ff34104bb53b).
     * 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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152092774
  
    test 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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153567087
  
    The changes LGTM, could you update the description to reflect the changes?


---
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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153262712
  
    **[Test build #1972 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1972/consoleFull)** for PR 9349 at commit [`6dec533`](https://github.com/apache/spark/commit/6dec53324b7ccc6c90252c4212d502d30d8e7a0c).


---
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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152094274
  
    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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153448480
  
    @wzhfy The first part change is good (remove dialectClassName). But other one may introduce regression, when you have `spark.sql.dialect sql` in conf/spark-default.conf, you will still got hiveql in HiveContext. I think we should fix the issue while procesing `set spark.sql.dialect`, which should call `conf.dialect`, not `conf.getConf`.


---
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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152116547
  
    @davies 


---
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-11398][SQL] unnecessary def dialectClas...

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

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


---
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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153262162
  
    @davies @liancheng I've updated the description of this problem, hoping to explain it better now.
    Can you review this pr and authorize testing? thx.


---
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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152215773
  
    @wzhfy Currently, we can use `sql`  as the dialog for HiveContext, but can't do this after the change. Is that the thing we want?


---
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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153922195
  
    @davies The description is updated


---
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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153291000
  
    **[Test build #1972 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1972/consoleFull)** for PR 9349 at commit [`6dec533`](https://github.com/apache/spark/commit/6dec53324b7ccc6c90252c4212d502d30d8e7a0c).
     * 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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153922944
  
    Merged into 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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153567183
  
    **[Test build #1975 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1975/consoleFull)** for PR 9349 at commit [`d15e14a`](https://github.com/apache/spark/commit/d15e14ad7b817d874b904cc839d9ff34104bb53b).


---
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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152373512
  
    @davies My intention is to change the displayed dialect when spark-sql starts, not to set "hiveql" for HiveContext forever. 
    
    Here's my description of the problem in JIRA:
    When we start bin/spark-sql, the default context is HiveContext, and the corresponding dialect is hiveql.
    However, if we type "set spark.sql.dialect;", the result is "sql", which is inconsistent with the actual dialect and is misleading. For example, we can create tables which is only allowed in hiveql, but this dialect conf shows it's "sql".
    Although this problem will not cause any execution error, it's misleading to spark sql users. Therefore I think we should fix it.
    
    After the change, we can still use "sql" as the dialect for HiveContext through "set spark.sql.dialect=sql", then the conf.dialect in HiveContext will become sql. Because in SQLConf, def dialect = getConf(), and now the dialect in "settings" becomes "sql".


---
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-11398][SQL] misleading dialect conf at ...

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

    https://github.com/apache/spark/pull/9349#issuecomment-152107307
  
    I also delete def dialectClassName. It's useless because getSQLDialect() is enough for selecting dialect.


---
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-11398][SQL] unnecessary def dialectClas...

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

    https://github.com/apache/spark/pull/9349#issuecomment-153553626
  
    @davies Thanks for the advice. The commit has been updated, please check if that's what we want.
    
    Btw, I think the cause of this problem is the inconsistency between the default dialect conf (sql) and the default sqlcontext (HiveContext), maybe we should fix it in the future.


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