[SPARK-30049][SQL] SQL fails to parse when comment contains an unmatched quote character. by javierivanov · Pull Request #27321 · apache/spark

javierivanov · 2020-01-22T17:41:06Z

What changes were proposed in this pull request?

A SQL statement that contains a comment with an unmatched quote character can lead to a parse error:

Added a insideComment flag in the splitter method to avoid checking single and double quotes within a comment:

spark-sql> SELECT 1 -- someone's comment here
> ;
Error in query: extraneous input ';' expecting <EOF>(line 2, pos 0)
== SQL ==
SELECT 1 -- someone's comment here
;
^^^

Why are the changes needed?

This misbehaviour was not present on previous spark versions.

Does this PR introduce any user-facing change?

No

How was this patch tested?

New tests were added.

dongjoon-hyun · 2020-01-22T18:50:50Z

ok to test

SparkQA · 2020-01-22T19:23:45Z

Test build #117249 has finished for PR 27321 at commit 6695496.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-01-23T01:36:57Z

cc @wangyum FYI

tgravescs · 2020-01-23T14:50:25Z

I haven't had time to test your patch but did you test this with just the spark-shell and spark.sql(""" """) syntax as well? I didn't think that went through the SparkSQLCliDriver and that case also fails.

kiszk · 2020-02-03T16:18:42Z

ping @javierivanov

javierivanov · 2020-02-03T19:29:59Z

Sorry for my delay here...
spark.sql will parse only 1 SQL statement, hence the ; is not accepted by SQL function.

scala> sql("""SELECT 1;""")
org.apache.spark.sql.catalyst.parser.ParseException:
extraneous input ';' expecting <EOF>(line 1, pos 8)
== SQL ==
SELECT 1;
--------^^^

In this case the ; is being catch in the comment.

scala> sql("""SELECT 1 -- someone's comment here;""")
res1: org.apache.spark.sql.DataFrame = [1: int]

The SQLDriver is parsing SQLStatements into a list, and foreach calls the sql context to interpret each.

This was failing because the parser was still detecting quotes inside of comments and passed the sql statement like this:

scala> sql("""SELECT 1 -- someone's comment here
| ;""")
org.apache.spark.sql.catalyst.parser.ParseException:
extraneous input ';' expecting <EOF>(line 2, pos 0)
== SQL ==
SELECT 1 -- someone's comment here
;
^^^

Let me know for any comments 👍

maropu · 2020-02-05T07:32:03Z

yea, right. Spark doesnt support multi-line queries in spark.sql: (related jira: https://issues.apache.org/jira/browse/SPARK-24260)

maropu · 2020-02-05T07:38:06Z

+ |;""".stripMargin -> "testcomment"
+ )
+ }
+


nit: Can you avoid this unnecessary blank?

@maropu Not sure, which blank?

tgravescs · 2020-02-06T14:47:10Z

yea, right. Spark doesnt support multi-line queries in spark.sql: (related jira: https://issues.apache.org/jira/browse/SPARK-24260)

ah ok, that makes sense, thanks

SparkQA · 2020-02-06T14:57:11Z

Test build #117997 has finished for PR 27321 at commit 074a943.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-02-07T10:15:07Z

ping @wangyum

SparkQA · 2020-02-26T11:02:54Z

Test build #118967 has finished for PR 27321 at commit c4924a7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

tgravescs · 2020-02-28T21:12:54Z

@wangyum any comments?

…hed quote character ### What changes were proposed in this pull request? A SQL statement that contains a comment with an unmatched quote character can lead to a parse error: - Added a insideComment flag in the splitter method to avoid checking single and double quotes within a comment: ``` spark-sql> SELECT 1 -- someone's comment here > ; Error in query: extraneous input ';' expecting <EOF>(line 2, pos 0) == SQL == SELECT 1 -- someone's comment here ; ^^^ ``` ### Why are the changes needed? This misbehaviour was not present on previous spark versions. ### Does this PR introduce any user-facing change? - No ### How was this patch tested? - New tests were added. Closes#27321 from javierivanov/SPARK-30049B. Lead-authored-by: Javier <jfuentes@hortonworks.com> Co-authored-by: Javier Fuentes <j.fuentes.m@icloud.com> Signed-off-by: Thomas Graves <tgraves@apache.org> (cherry picked from commit 3ff2135) Signed-off-by: Thomas Graves <tgraves@apache.org>

tgravescs · 2020-03-03T15:57:19Z

merged to master and branch-3.0, thanks @javierivanov

gatorsmile · 2020-03-04T03:18:54Z

cc @juliuszsompolski

dongjoon-hyun · 2020-03-10T21:31:19Z

Hi, All.
It seems that https://issues.apache.org/jira/browse/SPARK-31102 is reported due to this by @wangyum . Could you take a look?

…hed quote character ### What changes were proposed in this pull request? A SQL statement that contains a comment with an unmatched quote character can lead to a parse error: - Added a insideComment flag in the splitter method to avoid checking single and double quotes within a comment: ``` spark-sql> SELECT 1 -- someone's comment here > ; Error in query: extraneous input ';' expecting <EOF>(line 2, pos 0) == SQL == SELECT 1 -- someone's comment here ; ^^^ ``` ### Why are the changes needed? This misbehaviour was not present on previous spark versions. ### Does this PR introduce any user-facing change? - No ### How was this patch tested? - New tests were added. Closesapache#27321 from javierivanov/SPARK-30049B. Lead-authored-by: Javier <jfuentes@hortonworks.com> Co-authored-by: Javier Fuentes <j.fuentes.m@icloud.com> Signed-off-by: Thomas Graves <tgraves@apache.org>

added fix and UT
943c8d8

javierivanov changed the title ~~SQL fails to parse when comment contains an unmatched quote character.~~[SPARK-30049][SQL] SQL fails to parse when comment contains an unmatched quote character.Jan 22, 2020

linter fix
6695496

dongjoon-hyun added the SQL label Jan 22, 2020

maropu reviewed Feb 5, 2020
View reviewed changes

Comment thread...e-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala Outdated

maropu reviewed Feb 5, 2020
View reviewed changes

Comment threadsql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala

maropu reviewed Feb 5, 2020
View reviewed changes

Comment thread...e-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala Outdated

fixes
074a943

javierivanov requested a review from wangyum February 12, 2020 11:16

tgravescs reviewed Feb 25, 2020
View reviewed changes

Fixes
c4924a7

tgravescs approved these changes Mar 3, 2020
View reviewed changes

asfgit closed this in 3ff2135Mar 3, 2020

This was referenced Jan 5, 2021
[SPARK-33100][SQL][3.0] Ignore a semicolon inside a bracketed comment in spark-sql #31033
Closed
[SPARK-33100][SQL][2.4] Ignore a semicolon inside a bracketed comment in spark-sql #31040
Closed

Uh oh!

Conversation

javierivanov commented Jan 22, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Jan 22, 2020

Uh oh!

SparkQA commented Jan 22, 2020

Uh oh!

HyukjinKwon commented Jan 23, 2020

Uh oh!

tgravescs commented Jan 23, 2020

Uh oh!

kiszk commented Feb 3, 2020

Uh oh!

javierivanov commented Feb 3, 2020

Uh oh!

maropu commented Feb 5, 2020

Uh oh!

Uh oh!

maropuFeb 5, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javierivanovFeb 6, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tgravescs commented Feb 6, 2020

Uh oh!

SparkQA commented Feb 6, 2020

Uh oh!

maropu commented Feb 7, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Feb 26, 2020

Uh oh!

tgravescs commented Feb 28, 2020

Uh oh!

tgravescs commented Mar 3, 2020

Uh oh!

gatorsmile commented Mar 4, 2020

Uh oh!

dongjoon-hyun commented Mar 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

maropuFeb 5, 2020•
edited
Loading