From 5c0a000687e0102a1961a7fdf2c272c1ce9f24cf Mon Sep 17 00:00:00 2001 From: Javier Fuentes Date: Sun, 15 Mar 2020 22:19:10 -0300 Subject: [PATCH 1/4] fixes for SPARK-31102 --- .../hive/thriftserver/SparkSQLCLIDriver.scala | 12 ++++----- .../sql/hive/thriftserver/CliSuite.scala | 26 ++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala index 6b76927bd415..c2ed32c72379 100644 --- a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala +++ b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala @@ -508,7 +508,6 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging { var insideComment = false var escape = false var beginIndex = 0 - var endIndex = line.length val ret = new JArrayList[String] for (index <- 0 until line.length) { @@ -534,8 +533,6 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging { } else if (hasNext && line.charAt(index + 1) == '-') { // ignore quotes and ; insideComment = true - // ignore eol - endIndex = index } } else if (line.charAt(index) == ';') { if (insideSingleQuote || insideDoubleQuote || insideComment) { @@ -545,8 +542,11 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging { ret.add(line.substring(beginIndex, index)) beginIndex = index + 1 } - } else { - // nothing to do + } else if (line.charAt(index) == '\n') { + // with a new line the inline comment should end. + if (!escape) { + insideComment = false + } } // set the escape if (escape) { @@ -555,7 +555,7 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging { escape = true } } - ret.add(line.substring(beginIndex, endIndex)) + ret.add(line.substring(beginIndex)) ret } } diff --git a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala index 43aafc3c8590..9251e71568ae 100644 --- a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala +++ b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala @@ -402,24 +402,20 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with Logging { } test("SPARK-30049 Should not complain for quotes in commented lines") { - runCliWithin(1.minute)( - """SELECT concat('test', 'comment') -- someone's comment here + runCliWithin(3.minute)( + """SELECT concat('test', 'comment'), -- someone's comment here + |2 |;""".stripMargin -> "testcomment" ) - } - - test("SPARK-30049 Should not complain for quotes in commented with multi-lines") { - runCliWithin(1.minute)( - """SELECT concat('test', 'comment') -- someone's comment here \\ - | comment continues here with single ' quote \\ - | extra ' \\ - |;""".stripMargin -> "testcomment" + runCliWithin(3.minute)( + """SELECT concat('test', 'comment'), + | -- someone's comment here + | 2;""".stripMargin -> "testcomment" ) - runCliWithin(1.minute)( - """SELECT concat('test', 'comment') -- someone's comment here \\ - | comment continues here with single ' quote \\ - | extra ' \\ - | ;""".stripMargin -> "testcomment" + runCliWithin(3.minute)( + """SELECT concat('test', 'comment'), + | -- someone's comment here + | 2;""".stripMargin -> "testcomment" ) } } From d69d27126c06ea9782c5422a5435809a062b8ed7 Mon Sep 17 00:00:00 2001 From: Javier Fuentes Date: Sun, 15 Mar 2020 23:09:48 -0300 Subject: [PATCH 2/4] fixed tests --- .../org/apache/spark/sql/hive/thriftserver/CliSuite.scala | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala index 9251e71568ae..ae3804b76bc7 100644 --- a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala +++ b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala @@ -403,8 +403,7 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with Logging { test("SPARK-30049 Should not complain for quotes in commented lines") { runCliWithin(3.minute)( - """SELECT concat('test', 'comment'), -- someone's comment here - |2 + """SELECT concat('test', 'comment') -- someone's comment here |;""".stripMargin -> "testcomment" ) runCliWithin(3.minute)( @@ -412,10 +411,5 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with Logging { | -- someone's comment here | 2;""".stripMargin -> "testcomment" ) - runCliWithin(3.minute)( - """SELECT concat('test', 'comment'), - | -- someone's comment here - | 2;""".stripMargin -> "testcomment" - ) } } From 440dcbd93986b82e36d911ee0461aab3b4927f8c Mon Sep 17 00:00:00 2001 From: Javier Fuentes Date: Sun, 12 Apr 2020 22:53:09 -0400 Subject: [PATCH 3/4] added multi-line to single comments --- .../org/apache/spark/sql/catalyst/parser/SqlBase.g4 | 2 +- .../apache/spark/sql/hive/thriftserver/CliSuite.scala | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 index d78f584aa9ae..0dd8725be705 100644 --- a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 +++ b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 @@ -1814,7 +1814,7 @@ fragment LETTER ; SIMPLE_COMMENT - : '--' ~[\r\n]* '\r'? '\n'? -> channel(HIDDEN) + : '--' ('\\\n' | ~[\r\n])* '\r'? '\n'? -> channel(HIDDEN) ; BRACKETED_COMMENT diff --git a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala index 406dab3dac5d..7594b3f6c99a 100644 --- a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala +++ b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala @@ -464,4 +464,14 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with BeforeAndAfterE | 2;""".stripMargin -> "testcomment" ) } + + test("SPARK-30049 Should not complain for quotes in commented with multi-lines") { + runCliWithin(3.minute)( + """SELECT concat('test', 'comment') -- someone's comment here \ + | comment continues here with single ' quote \ + | extra ' \ + |;""".stripMargin -> "testcomment" + ) + } + } From 0571f21a0c058e0e1c14efd54e174d64f9420b01 Mon Sep 17 00:00:00 2001 From: Javier Fuentes Date: Mon, 13 Apr 2020 09:03:23 -0400 Subject: [PATCH 4/4] fixes --- .../spark/sql/catalyst/parser/PlanParserSuite.scala | 7 ++++++- .../apache/spark/sql/hive/thriftserver/CliSuite.scala | 10 ++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala index 0a591ad9cde7..a1d88232b037 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala @@ -55,11 +55,16 @@ class PlanParserSuite extends AnalysisTest { With(plan, ctes) } - test("single comment") { + test("single comment case one") { val plan = table("a").select(star()) assertEqual("-- single comment\nSELECT * FROM a", plan) } + test("single comment case two") { + val plan = table("a").select(star()) + assertEqual("-- single comment\\\nwith line continuity\nSELECT * FROM a", plan) + } + test("bracketed comment case one") { val plan = table("a").select(star()) assertEqual( diff --git a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala index 7594b3f6c99a..5871728ccd58 100644 --- a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala +++ b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/CliSuite.scala @@ -454,11 +454,14 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with BeforeAndAfterE } test("SPARK-30049 Should not complain for quotes in commented lines") { - runCliWithin(3.minute)( + runCliWithin(1.minute)( """SELECT concat('test', 'comment') -- someone's comment here |;""".stripMargin -> "testcomment" ) - runCliWithin(3.minute)( + } + + test("SPARK-31102 spark-sql fails to parse when contains comment") { + runCliWithin(1.minute)( """SELECT concat('test', 'comment'), | -- someone's comment here | 2;""".stripMargin -> "testcomment" @@ -466,12 +469,11 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with BeforeAndAfterE } test("SPARK-30049 Should not complain for quotes in commented with multi-lines") { - runCliWithin(3.minute)( + runCliWithin(1.minute)( """SELECT concat('test', 'comment') -- someone's comment here \ | comment continues here with single ' quote \ | extra ' \ |;""".stripMargin -> "testcomment" ) } - }