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 2020/11/11 10:31:19 UTC

[GitHub] [spark] yaooqinn opened a new pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

yaooqinn opened a new pull request #30332:
URL: https://github.com/apache/spark/pull/30332


   
   ### 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.
   -->
   
   SparkSession.sql converts a string value to a DataFrame, and the string value should be one single SQL statement ending up w/ or w/o one or more semicolons. e.g.
   
   ```sql
   scala> spark.sql(" select 2").show
   +---+
   |  2|
   +---+
   |  2|
   +---+
   scala> spark.sql(" select 2;").show
   +---+
   |  2|
   +---+
   |  2|
   +---+
   
   scala> spark.sql(" select 2;;;;").show
   +---+
   |  2|
   +---+
   |  2|
   +---+
   ```
   If we put 2 or more statements in, it fails in the parser as expected, e.g.
   
   ```sql
   scala> spark.sql(" select 2; select 1;").show
   org.apache.spark.sql.catalyst.parser.ParseException:
   extraneous input 'select' expecting {<EOF>, ';'}(line 1, pos 11)
   
   == SQL ==
    select 2; select 1;
   -----------^^^
   
     at org.apache.spark.sql.catalyst.parser.ParseException.withCommand(ParseDriver.scala:263)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parse(ParseDriver.scala:130)
     at org.apache.spark.sql.execution.SparkSqlParser.parse(SparkSqlParser.scala:51)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parsePlan(ParseDriver.scala:81)
     at org.apache.spark.sql.SparkSession.$anonfun$sql$2(SparkSession.scala:610)
     at org.apache.spark.sql.catalyst.QueryPlanningTracker.measurePhase(QueryPlanningTracker.scala:111)
     at org.apache.spark.sql.SparkSession.$anonfun$sql$1(SparkSession.scala:610)
     at org.apache.spark.sql.SparkSession.withActive(SparkSession.scala:769)
     at org.apache.spark.sql.SparkSession.sql(SparkSession.scala:607)
     ... 47 elided
   ```
   
   As a very generic user scenario, users may want to change some settings before they execute
   the queries. They may pass a string value like `set spark.sql.abc=2; select 1;` into this API, which creates a confusing gap between the actual effect and the user's expectations.
   
   The user may want the query to be executed with spark.sql.abc=2, but Spark actually treats the whole part of `2; select 1;` as the value of the property 'spark.sql.abc',
   e.g.
   
   ```
   scala> spark.sql("set spark.sql.abc=2; select 1;").show
   +-------------+------------+
   |          key|       value|
   +-------------+------------+
   |spark.sql.abc|2; select 1;|
   +-------------+------------+
   ```
   
   What's more, the SET symbol could digest everything behind it, which makes it unstable from version to version, e.g.
   
   #### 3.1
   ```sql
   scala> spark.sql("set;").show
   org.apache.spark.sql.catalyst.parser.ParseException:
   Expected format is 'SET', 'SET key', or 'SET key=value'. If you want to include special characters in key, please use quotes, e.g., SET `ke y`=value.(line 1, pos 0)
   
   == SQL ==
   set;
   ^^^
   
     at org.apache.spark.sql.execution.SparkSqlAstBuilder.$anonfun$visitSetConfiguration$1(SparkSqlParser.scala:83)
     at org.apache.spark.sql.catalyst.parser.ParserUtils$.withOrigin(ParserUtils.scala:113)
     at org.apache.spark.sql.execution.SparkSqlAstBuilder.visitSetConfiguration(SparkSqlParser.scala:72)
     at org.apache.spark.sql.execution.SparkSqlAstBuilder.visitSetConfiguration(SparkSqlParser.scala:58)
     at org.apache.spark.sql.catalyst.parser.SqlBaseParser$SetConfigurationContext.accept(SqlBaseParser.java:2161)
     at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18)
     at org.apache.spark.sql.catalyst.parser.AstBuilder.$anonfun$visitSingleStatement$1(AstBuilder.scala:77)
     at org.apache.spark.sql.catalyst.parser.ParserUtils$.withOrigin(ParserUtils.scala:113)
     at org.apache.spark.sql.catalyst.parser.AstBuilder.visitSingleStatement(AstBuilder.scala:77)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.$anonfun$parsePlan$1(ParseDriver.scala:82)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parse(ParseDriver.scala:113)
     at org.apache.spark.sql.execution.SparkSqlParser.parse(SparkSqlParser.scala:51)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parsePlan(ParseDriver.scala:81)
     at org.apache.spark.sql.SparkSession.$anonfun$sql$2(SparkSession.scala:610)
     at org.apache.spark.sql.catalyst.QueryPlanningTracker.measurePhase(QueryPlanningTracker.scala:111)
     at org.apache.spark.sql.SparkSession.$anonfun$sql$1(SparkSession.scala:610)
     at org.apache.spark.sql.SparkSession.withActive(SparkSession.scala:769)
     at org.apache.spark.sql.SparkSession.sql(SparkSession.scala:607)
     ... 47 elided
   
   scala> spark.sql("set a;").show
   org.apache.spark.sql.catalyst.parser.ParseException:
   Expected format is 'SET', 'SET key', or 'SET key=value'. If you want to include special characters in key, please use quotes, e.g., SET `ke y`=value.(line 1, pos 0)
   
   == SQL ==
   set a;
   ^^^
   
     at org.apache.spark.sql.execution.SparkSqlAstBuilder.$anonfun$visitSetConfiguration$1(SparkSqlParser.scala:83)
     at org.apache.spark.sql.catalyst.parser.ParserUtils$.withOrigin(ParserUtils.scala:113)
     at org.apache.spark.sql.execution.SparkSqlAstBuilder.visitSetConfiguration(SparkSqlParser.scala:72)
     at org.apache.spark.sql.execution.SparkSqlAstBuilder.visitSetConfiguration(SparkSqlParser.scala:58)
     at org.apache.spark.sql.catalyst.parser.SqlBaseParser$SetConfigurationContext.accept(SqlBaseParser.java:2161)
     at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18)
     at org.apache.spark.sql.catalyst.parser.AstBuilder.$anonfun$visitSingleStatement$1(AstBuilder.scala:77)
     at org.apache.spark.sql.catalyst.parser.ParserUtils$.withOrigin(ParserUtils.scala:113)
     at org.apache.spark.sql.catalyst.parser.AstBuilder.visitSingleStatement(AstBuilder.scala:77)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.$anonfun$parsePlan$1(ParseDriver.scala:82)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parse(ParseDriver.scala:113)
     at org.apache.spark.sql.execution.SparkSqlParser.parse(SparkSqlParser.scala:51)
     at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parsePlan(ParseDriver.scala:81)
     at org.apache.spark.sql.SparkSession.$anonfun$sql$2(SparkSession.scala:610)
     at org.apache.spark.sql.catalyst.QueryPlanningTracker.measurePhase(QueryPlanningTracker.scala:111)
     at org.apache.spark.sql.SparkSession.$anonfun$sql$1(SparkSession.scala:610)
     at org.apache.spark.sql.SparkSession.withActive(SparkSession.scala:769)
     at org.apache.spark.sql.SparkSession.sql(SparkSession.scala:607)
     ... 47 elided
   ```
   
   #### 2.4
   
   ```sql
   scala> spark.sql("set;").show
   +---+-----------+
   |key|      value|
   +---+-----------+
   |  ;|<undefined>|
   +---+-----------+
   
   
   scala> spark.sql("set a;").show
   +---+-----------+
   |key|      value|
   +---+-----------+
   | a;|<undefined>|
   +---+-----------+
   ```
   
   In this PR, 
   1.  make `set spark.sql.abc=2; select 1;` in `SparkSession.sql` fail directly, user should call `.sql` for each statement separately.
   2. make the semicolon as the separator of statements, and if users want to use it as part of the property value, shall use quotes too.
   
   
   ### Why are the changes needed?
   
   1. disambiguation for  `SparkSession.sql` 
   2. make semicolon work same both w/ `SET` and other statements
   
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   
   yes,
   the semicolon works as a separator of statements now, it will be trimmed if it is at the end of the statement and fail the statement if it is in the middle. you need to use quotes if you want it to be part of the property value
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   
   new tests


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726095130






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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521855882



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParserUtils.scala
##########
@@ -71,6 +71,12 @@ object ParserUtils {
     stream.getText(interval)
   }
 
+  /** Get all the text which between the given start and end tokens. */
+  def interval(start: Token, end: Token): String = {

Review comment:
       OK~




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725879801


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35577/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521811243



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParserUtils.scala
##########
@@ -71,6 +71,12 @@ object ParserUtils {
     stream.getText(interval)
   }
 
+  /** Get all the text which between the given start and end tokens. */
+  def interval(start: Token, end: Token): String = {

Review comment:
       this seems hacky. I think the named solution(`anything`) is better.




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725810615


   **[Test build #130961 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130961/testReport)** for PR 30332 at commit [`69d186d`](https://github.com/apache/spark/commit/69d186de52ea25bab1fe2d7dd69c179fba923ca9).


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725911875






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726562478


   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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725931045


   **[Test build #130982 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130982/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).


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

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] viirya commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521842714



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -141,6 +156,34 @@ class SparkSqlParserSuite extends AnalysisTest {
     intercept("RESET spark.sql :  key", expectedErrMsg)
   }
 
+  test("SPARK-33419: Semicolon handling in SET command") {
+    assertEqual("SET a=1;", SetCommand(Some("a" -> Some("1"))))
+    assertEqual("SET a=1;;", SetCommand(Some("a" -> Some("1"))))
+
+    assertEqual("SET a=`1`;", SetCommand(Some("a" -> Some("1"))))
+    assertEqual("SET a=`1;`", SetCommand(Some("a" -> Some("1;"))))
+    assertEqual("SET a=`1;`;", SetCommand(Some("a" -> Some("1;"))))
+
+    assertEqual("SET `a`=1;;", SetCommand(Some("a" -> Some("1"))))
+    assertEqual("SET `a`=`1;`", SetCommand(Some("a" -> Some("1;"))))
+
+    assertEqual("SET a=`1;`", SetCommand(Some("a" -> Some("1;"))))

Review comment:
       This is duplicate to L164.




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726032135


   **[Test build #130982 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130982/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).
    * This patch **fails Spark unit 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725912422






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521369965



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -141,6 +141,41 @@ class SparkSqlParserSuite extends AnalysisTest {
     intercept("RESET spark.sql :  key", expectedErrMsg)
   }
 
+  test("Semicolon handling in SET command") {
+    assertEqual("SET spark.sql.shuffle.partitions=1;",

Review comment:
       to shorten the test, we can check `SET a=1;`




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726219730






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521362495



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,20 +80,30 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key or semicolon is value," +

Review comment:
       `... in key, or include semicolons in value, ...`




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725467351


   **[Test build #130931 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130931/testReport)** for PR 30332 at commit [`45c7b04`](https://github.com/apache/spark/commit/45c7b04ede347ee4d1fca53cb09bee63c321e9d6).
    * This patch **fails Spark unit 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726032555


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130982/
   Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726034953


   **[Test build #130996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130996/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).


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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521857653



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -70,9 +70,21 @@ class SparkSqlParserSuite extends AnalysisTest {
     StaticSQLConf
     ConfigEntry.knownConfigs.values.asScala.foreach { config =>
       assertEqual(s"SET ${config.key}", SetCommand(Some(config.key -> None)))
-      if (config.defaultValue.isDefined && config.defaultValueString != null) {
-        assertEqual(s"SET ${config.key}=${config.defaultValueString}",

Review comment:
       yes. but seems not bad to add these test case involving quotes
   




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

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] yaooqinn commented on pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726034074


   retest this please


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725468308






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725468320


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130931/
   Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725804680


   I think it's OK to fail invalid use cases with a nice error message to tell users how to work around. We've already made similar changes to forbid `=` in key and value.


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

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] maropu commented on pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725737152


   In SQL statements, a semicolon has a special meaning, so the proposed idea sounds reasonable to me. But, in branch-3.0/2.4, we already have accepted it like the example below (the failed test above);
   ```
   // v3.0.1 and v2.6.7
   scala> sql("SET spark.ui.xXssProtection=1; mode=block").show()
   +--------------------+-------------+
   |                 key|        value|
   +--------------------+-------------+
   |spark.ui.xXssProt...|1; mode=block|
   +--------------------+-------------+
   ```
   Probably, the case above should be "SET spark.ui.xXssProtection=\`1; mode=block\`" as you suggested. So, how about adding a legacy config and updating the migration?


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726077494


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35602/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726032546






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521834964



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -70,9 +70,21 @@ class SparkSqlParserSuite extends AnalysisTest {
     StaticSQLConf
     ConfigEntry.knownConfigs.values.asScala.foreach { config =>
       assertEqual(s"SET ${config.key}", SetCommand(Some(config.key -> None)))
-      if (config.defaultValue.isDefined && config.defaultValueString != null) {
-        assertEqual(s"SET ${config.key}=${config.defaultValueString}",

Review comment:
       I think this test is to make sure we don't need to quote for builtin config keys and values.




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725892301






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725402222


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35537/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521361250



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -246,7 +246,9 @@ statement
     | SET TIME ZONE interval                                           #setTimeZone
     | SET TIME ZONE timezone=(STRING | LOCAL)                          #setTimeZone
     | SET TIME ZONE .*?                                                #setTimeZone
+    | SET configKey EQ configValue                                     #setQuotedConfiguration
     | SET configKey (EQ .*?)?                                          #setQuotedConfiguration

Review comment:
       nit: `SET configKey (EQ anything)?`




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

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] viirya commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521840768



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParserUtils.scala
##########
@@ -71,6 +71,12 @@ object ParserUtils {
     stream.getText(interval)
   }
 
+  /** Get all the text which between the given start and end tokens. */
+  def interval(start: Token, end: Token): String = {

Review comment:
       Oh, we have `remainder`...then it is okay to keep consistent.




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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521778847



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,20 +80,30 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key or semicolon is value," +
+        " please use quotes, e.g., SET `ke y`=`value`.", ctx)
     }
   }
 
-  override def visitSetQuotedConfiguration(ctx: SetQuotedConfigurationContext)
-    : LogicalPlan = withOrigin(ctx) {
-    val keyStr = ctx.configKey().getText
-    if (ctx.EQ() != null) {
-      SetCommand(Some(keyStr -> Option(remainder(ctx.EQ().getSymbol).trim)))
-    } else {
-      SetCommand(Some(keyStr -> None))
+  override def visitSetQuotedConfiguration(
+      ctx: SetQuotedConfigurationContext): LogicalPlan = withOrigin(ctx) {
+      if (ctx.configValue() != null && ctx.configKey() != null) {
+        SetCommand(Some(ctx.configKey().getText -> Option(ctx.configValue().getText)))
+      } else if (ctx.configValue() != null) {
+        SetCommand(Some(ctx.anything.getText -> Option(ctx.configValue().getText)))

Review comment:
       oh right~




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

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] yaooqinn commented on pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725798969


   ```
   scala> sql("SET spark.ui.xXssProtection=1; mode=block").show()
   ```
   
   I am a bit hesitant to add a new conf to restore behavior that you can do but is invalid as a matter of fact. 


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725912820


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130961/
   Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725344854


   **[Test build #130931 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130931/testReport)** for PR 30332 at commit [`45c7b04`](https://github.com/apache/spark/commit/45c7b04ede347ee4d1fca53cb09bee63c321e9d6).


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

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] viirya commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521839038



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParserUtils.scala
##########
@@ -71,6 +71,12 @@ object ParserUtils {
     stream.getText(interval)
   }
 
+  /** Get all the text which between the given start and end tokens. */
+  def interval(start: Token, end: Token): String = {

Review comment:
       `interval` sounds not a good method name. Maybe `getTextBetweenTokens`?




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726095136


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35602/
   Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725931045


   **[Test build #130982 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130982/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).


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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521813481



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParserUtils.scala
##########
@@ -71,6 +71,12 @@ object ParserUtils {
     stream.getText(interval)
   }
 
+  /** Get all the text which between the given start and end tokens. */
+  def interval(start: Token, end: Token): String = {

Review comment:
       https://github.com/apache/spark/pull/30332/files#diff-4ed0783e637633458d3b2e929553c134b5e2395785ec540b5347c64916c3d686R179-R180
   For the case above, `anything` converts "a b" -> "ab"




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725468308


   Merged build finished. Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726034953


   **[Test build #130996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130996/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725892301






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725828052


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35567/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30332:
URL: https://github.com/apache/spark/pull/30332


   


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

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] yaooqinn commented on pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725927347


   retest this please


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725973397


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35588/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725836751






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725973422






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

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] viirya commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521841476



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,18 +80,32 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key, or include semicolon " +
+        "in value, please use quotes, e.g., SET `ke y`=`v;alue`.", ctx)
     }
   }
 
-  override def visitSetQuotedConfiguration(ctx: SetQuotedConfigurationContext)
-    : LogicalPlan = withOrigin(ctx) {
-    val keyStr = ctx.configKey().getText
-    if (ctx.EQ() != null) {
-      SetCommand(Some(keyStr -> Option(remainder(ctx.EQ().getSymbol).trim)))
+  override def visitSetQuotedConfiguration(
+      ctx: SetQuotedConfigurationContext): LogicalPlan = withOrigin(ctx) {
+    if (ctx.configValue() != null && ctx.configKey() != null) {
+      SetCommand(Some(ctx.configKey().getText -> Option(ctx.configValue().getText)))
+    } else if (ctx.configValue() != null) {
+      interval(ctx.SET().getSymbol, ctx.EQ().getSymbol).trim match {

Review comment:
       Add `val valueStr = ctx.configValue().getText` and use it like `keyStr` below? It will be easier to read.




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725912433






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725836751






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

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] yaooqinn commented on pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725343570


   cc @maropu @cloud-fan @HyukjinKwon @viirya thanks very much.


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

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] maropu commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521716270



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -141,6 +141,41 @@ class SparkSqlParserSuite extends AnalysisTest {
     intercept("RESET spark.sql :  key", expectedErrMsg)
   }
 
+  test("Semicolon handling in SET command") {

Review comment:
       nit: add `SPARK-33419: Semicolon...`




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725810615






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725912422


   Merged build finished. Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725836736


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35567/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725912805






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725973422






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726218121


   **[Test build #130996 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130996/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).
    * 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725416442


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35537/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726219730






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725416466


   Merged build finished. Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521369466



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,20 +80,30 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key or semicolon is value," +
+        " please use quotes, e.g., SET `ke y`=`value`.", ctx)
     }
   }
 
-  override def visitSetQuotedConfiguration(ctx: SetQuotedConfigurationContext)
-    : LogicalPlan = withOrigin(ctx) {
-    val keyStr = ctx.configKey().getText
-    if (ctx.EQ() != null) {
-      SetCommand(Some(keyStr -> Option(remainder(ctx.EQ().getSymbol).trim)))
-    } else {
-      SetCommand(Some(keyStr -> None))
+  override def visitSetQuotedConfiguration(
+      ctx: SetQuotedConfigurationContext): LogicalPlan = withOrigin(ctx) {
+      if (ctx.configValue() != null && ctx.configKey() != null) {
+        SetCommand(Some(ctx.configKey().getText -> Option(ctx.configValue().getText)))
+      } else if (ctx.configValue() != null) {
+        SetCommand(Some(ctx.anything.getText -> Option(ctx.configValue().getText)))

Review comment:
       Shall we check the key here to make sure it matches `configKeyDef`?




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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521772799



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -257,6 +259,14 @@ configKey
     : quotedIdentifier
     ;
 
+configValue
+    : quotedIdentifier
+    ;
+
+anything

Review comment:
       or maybe we can replace `.*?` all over the file with `anything` in a follow-up?




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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521810711



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -246,7 +246,9 @@ statement
     | SET TIME ZONE interval                                           #setTimeZone
     | SET TIME ZONE timezone=(STRING | LOCAL)                          #setTimeZone
     | SET TIME ZONE .*?                                                #setTimeZone
+    | SET configKey EQ configValue                                     #setQuotedConfiguration
     | SET configKey (EQ .*?)?                                          #setQuotedConfiguration

Review comment:
       `anything` is unsuitable as it omits spaces, I have changed the related back to `.*?`




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725861097


   **[Test build #130971 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130971/testReport)** for PR 30332 at commit [`c049bd9`](https://github.com/apache/spark/commit/c049bd953a3b01ad3f47173e02ba7f926890d193).


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521824388



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParserUtils.scala
##########
@@ -71,6 +71,12 @@ object ParserUtils {
     stream.getText(interval)
   }
 
+  /** Get all the text which between the given start and end tokens. */
+  def interval(start: Token, end: Token): String = {

Review comment:
       interesting... Can you add a comment to explain 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521362726



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,20 +80,30 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key or semicolon is value," +
+        " please use quotes, e.g., SET `ke y`=`value`.", ctx)

Review comment:
       ```
   e.g., SET `ke y`=`valu;e`
   ```




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

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] maropu commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521715595



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -103,8 +103,8 @@ class SparkSqlParserSuite extends AnalysisTest {
       SetCommand(Some("spark.sql.    key" -> Some("-1"))))
 
     val expectedErrMsg = "Expected format is 'SET', 'SET key', or " +
-      "'SET key=value'. If you want to include special characters in key, " +
-      "please use quotes, e.g., SET `ke y`=value."
+      "'SET key=value'. If you want to include special characters in key or semicolon is value," +
+      " please use quotes, e.g., SET `ke y`=`value`."

Review comment:
       nit: could you move the leading space into the previous line?




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725416466






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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725892279


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35577/
   


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521363078



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,20 +80,30 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key or semicolon is value," +
+        " please use quotes, e.g., SET `ke y`=`value`.", ctx)
     }
   }
 
-  override def visitSetQuotedConfiguration(ctx: SetQuotedConfigurationContext)
-    : LogicalPlan = withOrigin(ctx) {
-    val keyStr = ctx.configKey().getText
-    if (ctx.EQ() != null) {
-      SetCommand(Some(keyStr -> Option(remainder(ctx.EQ().getSymbol).trim)))
-    } else {
-      SetCommand(Some(keyStr -> None))
+  override def visitSetQuotedConfiguration(
+      ctx: SetQuotedConfigurationContext): LogicalPlan = withOrigin(ctx) {
+      if (ctx.configValue() != null && ctx.configKey() != null) {

Review comment:
       2 space indentation for the method body.




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725416478


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35537/
   Test 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.

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] maropu commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521713733



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -257,6 +259,14 @@ configKey
     : quotedIdentifier
     ;
 
+configValue
+    : quotedIdentifier
+    ;
+
+anything

Review comment:
       `anything` -> `otherConfigKey`?




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725962055


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35588/
   


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

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] yaooqinn commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521263503



##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -257,6 +259,14 @@ configKey
     : quotedIdentifier
     ;
 
+configValue
+    : quotedIdentifier
+    ;
+
+anything

Review comment:
       This is need for https://github.com/apache/spark/pull/30332/files#diff-52c70b37bde4ed4abb96887207dd6aa45a1785be1d31013203800c97592e580bR251




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726095106


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35602/
   


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

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] viirya commented on a change in pull request #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30332:
URL: https://github.com/apache/spark/pull/30332#discussion_r521841476



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -79,18 +80,32 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
       case s if s.isEmpty =>
         SetCommand(None)
       case _ => throw new ParseException("Expected format is 'SET', 'SET key', or " +
-        "'SET key=value'. If you want to include special characters in key, " +
-        "please use quotes, e.g., SET `ke y`=value.", ctx)
+        "'SET key=value'. If you want to include special characters in key, or include semicolon " +
+        "in value, please use quotes, e.g., SET `ke y`=`v;alue`.", ctx)
     }
   }
 
-  override def visitSetQuotedConfiguration(ctx: SetQuotedConfigurationContext)
-    : LogicalPlan = withOrigin(ctx) {
-    val keyStr = ctx.configKey().getText
-    if (ctx.EQ() != null) {
-      SetCommand(Some(keyStr -> Option(remainder(ctx.EQ().getSymbol).trim)))
+  override def visitSetQuotedConfiguration(
+      ctx: SetQuotedConfigurationContext): LogicalPlan = withOrigin(ctx) {
+    if (ctx.configValue() != null && ctx.configKey() != null) {
+      SetCommand(Some(ctx.configKey().getText -> Option(ctx.configValue().getText)))
+    } else if (ctx.configValue() != null) {
+      interval(ctx.SET().getSymbol, ctx.EQ().getSymbol).trim match {

Review comment:
       Add `val valueStr = ctx.configValue().getText` and use it like `keyStr` below? It will easier to read.




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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-725344854


   **[Test build #130931 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130931/testReport)** for PR 30332 at commit [`45c7b04`](https://github.com/apache/spark/commit/45c7b04ede347ee4d1fca53cb09bee63c321e9d6).


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

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726095130


   Merged build finished. Test 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.

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 #30332: [SPARK-33419][SQL] Unexpected behavior when using SET commands before a query in SparkSession.sql

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30332:
URL: https://github.com/apache/spark/pull/30332#issuecomment-726032546


   Merged build finished. Test 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.

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