You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/12/06 06:11:56 UTC
[GitHub] [spark] AngersZhuuuu opened a new pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
AngersZhuuuu opened a new pull request #34815:
URL: https://github.com/apache/spark/pull/34815
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
Yes, if user write a wrong comment in sql/sql file or query in the end.
Before it's just ignored since it's not a statement. Now it will be passed to bakend and if the stament is not corrent, it will throw SQL exception.
### How was this patch tested?
added UT and test by handle.
```
spark-sql> /* SELECT /*+ HINT() 4; */;
Error in query:
mismatched input ';' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', 'ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 26)
== SQL ==
/* SELECT /*+ HINT() 4; */;
--------------------------^^^
spark-sql> /* SELECT /*+ HINT() 4; */
> SELECT 1;
1
Time taken: 3.16 seconds, Fetched 1 row(s)
spark-sql> /* SELECT /*+ HINT() */ 4; */;
spark-sql>
> ;
spark-sql>
> /* SELECT /*+ HINT() 4\\;
> SELECT 1;
Error in query:
Unclosed bracketed comment(line 1, pos 0)
== SQL ==
/* SELECT /*+ HINT() 4\\;
^^^
SELECT 1;
spark-sql>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986515982
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986491923
**[Test build #145941 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145941/testReport)** for PR 34815 at commit [`c67dcaf`](https://github.com/apache/spark/commit/c67dcaf041f6dd1bfc44642490c939ad2df41c00).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r762895207
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
##########
@@ -613,7 +613,7 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
isStatement = statementInProgress(index)
}
- if (isStatement) {
+ if (beginIndex < line.length()) {
Review comment:
Also keep same cod with hive
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986551404
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50415/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r794562062
##########
File path: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala
##########
@@ -620,4 +620,17 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with Logging {
|""".stripMargin -> "SELECT 1"
)
}
+
+ test("SPARK-37555: spark-sql should pass last unclosed comment to backend") {
Review comment:
I have run this many times. Not failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986649461
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50424/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986637274
**[Test build #145949 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145949/testReport)** for PR 34815 at commit [`e45b12d`](https://github.com/apache/spark/commit/e45b12dd2bf76d1e859af176a553c12260e02fed).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan edited a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-987563250
thanks, merging to master!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-989827071
Yea, maybe we should leave it as it is and document the behavior clearly in https://github.com/apache/spark/pull/34821/files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-989461733
In the spark-sql console, `;` has a special meaning and it means to terminate a sql statement (similar to ctrl+D in the paste mode). I don't have a strong opinion to change it, as writing comments in the console is not common anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986607350
**[Test build #145949 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145949/testReport)** for PR 34815 at commit [`e45b12d`](https://github.com/apache/spark/commit/e45b12dd2bf76d1e859af176a553c12260e02fed).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986654988
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145949/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986699052
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50424/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986484982
**[Test build #145939 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145939/testReport)** for PR 34815 at commit [`e874bbd`](https://github.com/apache/spark/commit/e874bbd009abd8d1602f6d3f13c791d6a56450cb).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986517659
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50417/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r793408067
##########
File path: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala
##########
@@ -620,4 +620,17 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with Logging {
|""".stripMargin -> "SELECT 1"
)
}
+
+ test("SPARK-37555: spark-sql should pass last unclosed comment to backend") {
Review comment:
Let me check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #34815:
URL: https://github.com/apache/spark/pull/34815
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] juliuszsompolski commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
juliuszsompolski commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-989303881
```
spark-sql> /* This is a comment without end symbol SELECT 1;
Error in query:
Unclosed bracketed comment(line 1, pos 0)
== SQL ==
/* This is a comment without end symbol SELECT 1;
^^^
```
Can `/*` be the start of a multiline comment, in which case the behaviour should be
```
spark-sql> /* This is a comment without end symbol SELECT 1;
>
```
(waiting for next line of multiline input)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-989450201
> ```
> spark-sql> /* This is a comment without end symbol SELECT 1;
> Error in query:
> Unclosed bracketed comment(line 1, pos 0)
>
> == SQL ==
> /* This is a comment without end symbol SELECT 1;
> ^^^
> ```
>
> Can `/*` be the start of a multiline comment, in which case the behaviour should be
>
> ```
> spark-sql> /* This is a comment without end symbol SELECT 1;
> >
> ```
>
> (waiting for next line of multiline input)?
If you write
```
spark-sql> /* This is a comment without end symbol SELECT 1\\;
>
```
it will wait for input.
If want
```
spark-sql> /* This is a comment without end symbol SELECT 1;
>
```
to wait for input, we need to change the console terminate logic.
cc @cloud-fan WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986484982
**[Test build #145939 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145939/testReport)** for PR 34815 at commit [`e874bbd`](https://github.com/apache/spark/commit/e874bbd009abd8d1602f6d3f13c791d6a56450cb).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986607350
**[Test build #145949 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145949/testReport)** for PR 34815 at commit [`e45b12d`](https://github.com/apache/spark/commit/e45b12dd2bf76d1e859af176a553c12260e02fed).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bogdanghit commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
bogdanghit commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r793407220
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
##########
@@ -613,7 +613,7 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
isStatement = statementInProgress(index)
}
- if (isStatement) {
+ if (beginIndex < line.length()) {
Review comment:
@AngersZhuuuu @cloud-fan Wondering why do we keep the `CLI` in the `hive-thriftserver` package. It has nothing to do with ThriftServer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986654988
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145949/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986505796
**[Test build #145939 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145939/testReport)** for PR 34815 at commit [`e874bbd`](https://github.com/apache/spark/commit/e874bbd009abd8d1602f6d3f13c791d6a56450cb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986515983
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986548887
> what if the users want to write the queries in multiple lines? does our CLI support it?
CLI will cut input when meet a un-escaped `;`
```
spark-sql> select
> 1,
> 2,
> 3,
> 4;
1 2 3 4
Time taken: 3.442 seconds, Fetched 1 row(s)
spark-sql>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986545849
what if the users want to write the queries in multiple lines? does our CLI support it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r762875263
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
##########
@@ -613,7 +613,7 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
isStatement = statementInProgress(index)
}
- if (isStatement) {
+ if (beginIndex < line.length()) {
Review comment:
what does this condition mean?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r762890523
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
##########
@@ -613,7 +613,7 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
isStatement = statementInProgress(index)
}
- if (isStatement) {
+ if (beginIndex < line.length()) {
Review comment:
Avoid pass an blank string to `processLine`, although in `processLine` it will handle this
```
// we can not use "split" function directly as ";" may be quoted
val commands = splitSemiColon(line).asScala
var command: String = ""
for (oneCmd <- commands) {
if (StringUtils.endsWith(oneCmd, "\\")) {
command += StringUtils.chop(oneCmd) + ";"
} else {
command += oneCmd
if (!StringUtils.isBlank(command)) {
val ret = processCmd(command)
command = ""
lastRet = ret
val ignoreErrors = HiveConf.getBoolVar(conf, HiveConf.ConfVars.CLIIGNOREERRORS)
if (ret != 0 && !ignoreErrors) {
CommandProcessorFactory.clean(conf.asInstanceOf[HiveConf])
return ret
}
}
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986692870
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50424/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986496652
ping @cloud-fan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986511781
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50415/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu edited a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu edited a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986548887
> what if the users want to write the queries in multiple lines? does our CLI support it?
CLI will cut input when meet a un-escaped `;`
```
spark-sql> select
> 1,
> 2,
> 3,
> 4;
1 2 3 4
Time taken: 3.442 seconds, Fetched 1 row(s)
spark-sql> select
> 1,
> 2 \\;
> ;
Error in query:
extraneous input '\' expecting {<EOF>, ';'}(line 3, pos 2)
== SQL ==
select
1,
2 \;
--^^^
spark-sql>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-987563250
thanks, merging to master/3.2!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986491923
**[Test build #145941 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145941/testReport)** for PR 34815 at commit [`c67dcaf`](https://github.com/apache/spark/commit/c67dcaf041f6dd1bfc44642490c939ad2df41c00).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986515035
**[Test build #145941 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145941/testReport)** for PR 34815 at commit [`c67dcaf`](https://github.com/apache/spark/commit/c67dcaf041f6dd1bfc44642490c939ad2df41c00).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] juliuszsompolski commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
juliuszsompolski commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-989805420
I don't have a strong opinion either, but in this case it is a `;` inside a comment, and other examples here showed that `;` inside a comment should not terminate command:
```
spark-sql> /* SELECT /*+ HINT() 4; */
> SELECT 1;
```
if the `;` after `HINT()` does not terminate a command here,
then I think the `;` at end of line in
```
spark-sql> /* This is a comment without end symbol SELECT 1;
```
also shouldn't?
I can imagine an edge case, where someone has a multi-line comment in their SQL that contains either some snippet of code with `;` terminated lines, or some bullet points with some arguments terminated with `;` that they copy paste into the console.
But I don't have a strong opinion and for the most part I think it's not a practical edge case...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bogdanghit commented on a change in pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
bogdanghit commented on a change in pull request #34815:
URL: https://github.com/apache/spark/pull/34815#discussion_r793406234
##########
File path: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala
##########
@@ -620,4 +620,17 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with Logging {
|""".stripMargin -> "SELECT 1"
)
}
+
+ test("SPARK-37555: spark-sql should pass last unclosed comment to backend") {
Review comment:
@AngersZhuuuu This test is flaky, fails quite often on repeated runs. Here's a sample error:
```
2021-12-08 12:01:27.68 - stderr> Setting default log level to "WARN".
2021-12-08 12:01:27.68 - stderr> To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
2021-12-08 12:01:39.459 - stderr> Spark master: local, Application Id: local-1638993689929
2021-12-08 12:01:40.688 - stdout> spark-sql> /* SELECT /*+ HINT() 4; */;
2021-12-08 12:01:41.299 - stdout> spark-sql> /* SELECT /*+ HINT() 4; */ SELECT 1;
2021-12-08 12:01:41.56 - stderr> Error in query:
2021-12-08 12:01:41.56 - stderr> mismatched input ';' expecting {'(', 'APPLY', 'CONVERT', 'COPY', 'OPTIMIZE', 'RESTORE', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', 'ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 26)
2021-12-08 12:01:41.56 - stderr>
2021-12-08 12:01:41.56 - stderr> == SQL ==
2021-12-08 12:01:41.56 - stderr> /* SELECT /*+ HINT() 4; */;
2021-12-08 12:01:41.56 - stderr> --------------------------^^^
2021-12-08 12:01:41.56 - stderr>
2021-12-08 12:01:47.573 - stdout> 1
2021-12-08 12:01:47.573 - stderr> Time taken: 6.272 seconds, Fetched 1 row(s)
2021-12-08 12:01:47.592 - stdout> spark-sql> /* Here is a unclosed bracketed comment SELECT 1;
2021-12-08 12:01:47.601 - stderr> Error in query:
2021-12-08 12:01:47.601 - stderr> Unclosed bracketed comment(line 1, pos 0)
2021-12-08 12:01:47.601 - stderr>
2021-12-08 12:01:47.601 - stderr> == SQL ==
2021-12-08 12:01:47.601 - stderr> /* Here is a unclosed bracketed comment SELECT 1;
2021-12-08 12:01:47.601 - stderr> ^^^
2021-12-08 12:01:47.601 - stderr>
2021-12-08 12:01:47.612 - stdout> spark-sql> /* SELECT /*+ HINT() */ 4; */;
2021-12-08 12:01:49.552 - stdout> spark-sql>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986546283
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50417/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986556778
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986556777
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34815: [SPARK-37555][SQL] spark-sql should pass last unclosed comment to backend
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34815:
URL: https://github.com/apache/spark/pull/34815#issuecomment-986699052
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50424/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org