You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tsingfu <gi...@git.apache.org> on 2014/11/28 02:31:32 UTC
[GitHub] spark pull request: [SPARK-4528][SQL] add comment support for Spar...
GitHub user tsingfu opened a pull request:
https://github.com/apache/spark/pull/3501
[SPARK-4528][SQL] add comment support for Spark SQL CLI
The relative JIRA: https://issues.apache.org/jira/browse/SPARK-4528?filter=-2
While using *spark-sql*, found it does not support comment while write sqls.
**Comment support** is widely using in many projects, it is a necessary to add this feature.
Added one function *filterComment()* in the source org.apache.spark.sql.hive.thriftserver.SparkSQLCLIDriver.scala.
##### three comment styles supported:
1. From a '#' character to the end of the line.
2. From a '--' sequence to the end of the line
3. From a /* sequence to the following */ sequence, as in the C programming language.
This syntax allows a comment to extend over multiple lines because the beginning and closing sequences need not be on the same line.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tsingfu/spark SPARK-4528
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3501.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 #3501
----
commit 390aaf2b348bdc70bcfd1fa2a64c8b481deafa6f
Author: tsingfu <ya...@gmail.com>
Date: 2014-11-26T10:01:24Z
[SparkSQL][SPARK-4528] add comment support for Spark SQL CLI spark-sql
commit 7a9681640e6059bc22686cbd01f2f9c7d1199153
Author: tsingfu <ya...@gmail.com>
Date: 2014-11-27T08:51:18Z
[SparkSQL][SPARK-4528] modify function filterComment to make code more concise
commit 8b8d343d30e6d3c8546622a06e0e05cec4d9e33a
Author: tsingfu <ya...@gmail.com>
Date: 2014-11-27T11:51:39Z
[SparkSQL][SPARK-4528] add case class CommentMark to make code more concise
----
---
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-4528][SQL] add comment support for Spar...
Posted by tsingfu <gi...@git.apache.org>.
Github user tsingfu commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65760992
@marmbrus Need we do something more.
If this implement is pretty complex, We may consider that reduce the number of comment support styles(only support -- as comment marker) to make code more simple, What do you think about?
---
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-4528][SQL] add comment support for Spar...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65186875
[Test build #24028 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24028/consoleFull) for PR 3501 at commit [`3b3db66`](https://github.com/apache/spark/commit/3b3db66aff2575343048bd49f9c7535fe526a98d).
* This patch merges cleanly.
---
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-4528][SQL] add comment support for Spar...
Posted by tsingfu <gi...@git.apache.org>.
Github user tsingfu commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65187921
I have correct the code uncompatible with scalastyle and passed the scalastyle check in local.
With test, Hive CLI *bin/hive* does not support comment **until now**.
Maybe the funciton fiterComment can be also used in in **Hive**.
---
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-4528][SQL] add comment support for Spar...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-64842839
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-4528][SQL] add comment support for Spar...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65190615
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24028/
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-4528][SQL] add comment support for Spar...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65138549
[Test build #24001 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24001/consoleFull) for PR 3501 at commit [`8b8d343`](https://github.com/apache/spark/commit/8b8d343d30e6d3c8546622a06e0e05cec4d9e33a).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class FilterCommentResult(var cmd: String,`
* `case class CommentMark(name: String, marker: String, regex: scala.util.matching.Regex)`
---
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-4528][SQL] add comment support for Spar...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3501
---
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-4528][SQL] add comment support for Spar...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65138556
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24001/
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-4528][SQL] add comment support for Spar...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-67391782
That sounds like a reasonable approach if there is no preexisting way to parse the more complicated form. Since we are trying to keep the PR queue small, I propose we close this issue for now and reopen once there is a new implementation. Thanks for working on this :)
---
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-4528][SQL] add comment support for Spar...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65137967
[Test build #24001 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24001/consoleFull) for PR 3501 at commit [`8b8d343`](https://github.com/apache/spark/commit/8b8d343d30e6d3c8546622a06e0e05cec4d9e33a).
* This patch merges cleanly.
---
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-4528][SQL] add comment support for Spar...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65190607
[Test build #24028 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24028/consoleFull) for PR 3501 at commit [`3b3db66`](https://github.com/apache/spark/commit/3b3db66aff2575343048bd49f9c7535fe526a98d).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class FilterCommentResult(var cmd: String,`
* `case class CommentMark(name: String, marker: String, regex: scala.util.matching.Regex)`
---
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-4528][SQL] add comment support for Spar...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65158098
I think this is a good feature to have. One question, is there not some function in hive we can just call to do this? While the code looks fine at a quick glance, its pretty complex and I'd have to look very closely to make sure that it is correct.
/cc @liancheng
---
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-4528][SQL] add comment support for Spar...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65337165
Yeah, I agree with @chenghao-intel. We are going to need a lot of tests for all of the various edge cases before we can trust some custom state machine for filtering comments. It would be much better if we can find something off-the-shelf for doing this (that also doesn't add new dependencies).
---
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-4528][SQL] add comment support for Spar...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65336693
Hive seems only ignore the lines start with "--" when reading SQL script from stream / file.
https://github.com/apache/hive/blob/branch-0.13/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java#L445
It still be nice if we support more powerful comment filtering, the concern is maintain the code seems not that easy, for example, what about if query like:
```
SELECT key, value FROM src where value like "_--##";
```
Build an escaping probably make thing more complicated.
---
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-4528][SQL] add comment support for Spar...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/3501#issuecomment-65137049
ok to test
---
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