You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2026/06/09 03:00:11 UTC
(spark) branch branch-4.2 updated: [SPARK-54876][SQL] Fix splitSemiColon dropping statement ending with block comment
This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 20f27e8cf80a [SPARK-54876][SQL] Fix splitSemiColon dropping statement ending with block comment
20f27e8cf80a is described below
commit 20f27e8cf80af9c06c0599b13ee38a43ea37af1f
Author: Anupam Yadav <an...@amazon.com>
AuthorDate: Mon Jun 8 19:58:42 2026 -0700
[SPARK-54876][SQL] Fix splitSemiColon dropping statement ending with block comment
### What changes were proposed in this pull request?
Replace the flag-based `splitSemiColon` implementation with a structural SQL-aware scanner (per cloud-fan's review). The scanner uses three `consumeXxx` helpers (string/line-comment/block-comment) that make the interior of strings and comments opaque to the outer loop.
`SparkSQLCLIDriver.splitSemiColon` now delegates to `StringUtils.splitSemiColonWithIndex` (single implementation, no duplication).
### Why are the changes needed?
The original bug: `splitSemiColon` drops the last SQL statement when it ends with a block comment. The incremental fix approach (adding ad-hoc string surgery) was fragile -- each patch introduced the next edge case. The structural scanner fixes all known cases by construction:
- Trailing block comment after last statement
- Nested block comments (`/* outer /* inner */ */`)
- Line comments preceding block comments
- Semicolons inside string literals and backtick-quoted identifiers
### Behavior change note
`SparkSQLCLIDriver.splitSemiColon` now treats backtick-quoted identifiers (`` ` ``) as string delimiters, matching ANSI identifier quoting and the existing `StringUtils.splitSemiColonWithIndex` behavior. Semicolons inside backtick-quoted identifiers no longer split.
### Does this PR introduce _any_ user-facing change?
Yes -- SQL statements ending with block comments are no longer silently dropped. Backtick-quoted identifiers containing semicolons are now handled correctly.
### How was this patch tested?
- All existing `StringUtilsSuite` tests pass (12 tests)
- All existing `CliSuite` SPARK-37906 tests pass
- New tests for: trailing block comment, nested block comments, line comment + block comment, backtick-quoted identifiers
### Was this patch authored or co-authored using generative AI tooling?
Yes.
Closes #55466 from yadavay-amzn/fix/SPARK-54876-split-semicolon.
Authored-by: Anupam Yadav <an...@amazon.com>
Signed-off-by: Wenchen Fan <we...@databricks.com>
(cherry picked from commit 3744250bc7bae17f29818831e0037035041a90a6)
Signed-off-by: Wenchen Fan <we...@databricks.com>
---
.../spark/sql/catalyst/util/StringUtils.scala | 179 +++++++--------------
.../spark/sql/catalyst/util/StringUtilsSuite.scala | 63 ++++++++
.../sql/hive/thriftserver/SparkSQLCLIDriver.scala | 113 +------------
.../spark/sql/hive/thriftserver/CliSuite.scala | 13 +-
4 files changed, 135 insertions(+), 233 deletions(-)
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
index b89bf7d6c6f4..87c55aaf1eaa 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
@@ -279,139 +279,68 @@ object StringUtils extends Logging {
pattern.matches(cleanText)
}
- private def containsNonWhiteSpaceCharacters(inputString: String): Boolean = {
- val pattern = "\\S".r
- pattern.findFirstIn(inputString).isDefined
- }
-
- // Implementation is grabbed from SparkSQLCLIDriver.splitSemiColon, the only difference is this
- // implementation handles backtick and treat it as single/double quote.
- // Below comments are from the source:
- // Adapted splitSemiColon from Hive 2.3's CliDriver.splitSemiColon.
- // Note: [SPARK-31595] if there is a `'` in a double quoted string, or a `"` in a single quoted
- // string, the origin implementation from Hive will not drop the trailing semicolon as expected,
- // hence we refined this function a little bit.
- // Note: [SPARK-33100] Ignore a semicolon inside a bracketed comment in spark-sql.
+ // Structural scanner for splitting SQL by semicolons.
+ // Handles quoted strings with escapes, line comments (--), and nested block comments (/* */).
+ // Semicolons inside strings or comments are not treated as delimiters.
+ // Note: [SPARK-31595], [SPARK-33100], [SPARK-54876]
def splitSemiColonWithIndex(line: String, enableSqlScripting: Boolean): List[String] = {
- var insideSingleQuote = false
- var insideDoubleQuote = false
- var insideBacktick = false
- var insideSimpleComment = false
- var bracketedCommentLevel = 0
- var escape = false
- var beginIndex = 0
- var leavingBracketedComment = false
- var hasPrecedingNonCommentString = false
- var isStatement = false
- val ret = new ArrayBuffer[String]()
-
lazy val insideSqlScript: Boolean = isSqlScript(line)
+ if (enableSqlScripting && insideSqlScript) return List(line)
- def insideBracketedComment: Boolean = bracketedCommentLevel > 0
- def insideComment: Boolean = insideSimpleComment || insideBracketedComment
- def statementInProgress(index: Int): Boolean =
- isStatement || (!insideComment &&
- index > beginIndex && !s"${line.charAt(index)}".trim.isEmpty)
-
- for (index <- 0 until line.length) {
- // Checks if we need to decrement a bracketed comment level; the last character '/' of
- // bracketed comments is still inside the comment, so `insideBracketedComment` must keep
- // true in the previous loop and we decrement the level here if needed.
- if (leavingBracketedComment) {
- bracketedCommentLevel -= 1
- leavingBracketedComment = false
+ val ret = new ArrayBuffer[String]()
+ val n = line.length
+ var i = 0
+ var chunkStart = 0
+ var chunkHasSql = false
+ var chunkHasUnclosed = false
+
+ def consumeString(start: Int, quote: Char): Int = {
+ var p = start + 1
+ while (p < n) {
+ val c = line.charAt(p)
+ if (c == '\\' && p + 1 < n) p += 2
+ else if (c == quote) return p + 1
+ else p += 1
}
+ chunkHasUnclosed = true; n
+ }
- if (line.charAt(index) == '\'' && !insideComment) {
- // take a look to see if it is escaped
- // See the comment above about SPARK-31595
- if (!escape && !insideDoubleQuote && !insideBacktick) {
- // flip the boolean variable
- insideSingleQuote = !insideSingleQuote
- }
- } else if (line.charAt(index) == '\"' && !insideComment) {
- // take a look to see if it is escaped
- // See the comment above about SPARK-31595
- if (!escape && !insideSingleQuote && !insideBacktick) {
- // flip the boolean variable
- insideDoubleQuote = !insideDoubleQuote
- }
- } else if (line.charAt(index) == '`' && !insideComment) {
- // take a look to see if it is escaped
- // See the comment above about SPARK-31595
- if (!escape && !insideSingleQuote && !insideDoubleQuote) {
- // flip the boolean variable
- insideBacktick = !insideBacktick
- }
- } else if (line.charAt(index) == '-') {
- val hasNext = index + 1 < line.length
- if (insideDoubleQuote || insideSingleQuote || insideBacktick || insideComment) {
- // Ignores '-' in any case of quotes or comment.
- // Avoids to start a comment(--) within a quoted segment or already in a comment.
- // Sample query: select "quoted value --"
- // ^^ avoids starting a comment if inside quotes.
- } else if (hasNext && line.charAt(index + 1) == '-') {
- // ignore quotes and ; in simple comment
- insideSimpleComment = true
- }
- } else if (line.charAt(index) == ';') {
- if (insideSingleQuote || insideDoubleQuote || insideBacktick || insideComment) {
- // do not split
- } else if (enableSqlScripting && insideSqlScript) {
- // do not split
- } else {
- if (isStatement) {
- // split, do not include ; itself
- ret.append(line.substring(beginIndex, index))
- }
- beginIndex = index + 1
- isStatement = false
- }
- } else if (line.charAt(index) == '\n') {
- // with a new line the inline simple comment should end.
- if (!escape) {
- insideSimpleComment = false
- }
- } else if (line.charAt(index) == '/' && !insideSimpleComment) {
- val hasNext = index + 1 < line.length
- if (insideSingleQuote || insideDoubleQuote || insideBacktick) {
- // Ignores '/' in any case of quotes
- } else if (insideBracketedComment && line.charAt(index - 1) == '*') {
- // Decrements `bracketedCommentLevel` at the beginning of the next loop
- leavingBracketedComment = true
- } else if (hasNext && line.charAt(index + 1) == '*') {
- bracketedCommentLevel += 1
- // Check if there's non-comment characters(non space, non newline characters) before
- // multiline comments.
- hasPrecedingNonCommentString = beginIndex != index && containsNonWhiteSpaceCharacters(
- line.substring(beginIndex, index)
- )
- }
- }
- // set the escape
- if (escape) {
- escape = false
- } else if (line.charAt(index) == '\\') {
- escape = true
- }
+ def consumeLineComment(start: Int): Int = {
+ var p = start + 2
+ while (p < n && line.charAt(p) != '\n') p += 1
+ p
+ }
- isStatement = statementInProgress(index)
+ def consumeBlockComment(start: Int): Int = {
+ var p = start + 2
+ var level = 1
+ while (p + 1 < n && level > 0) {
+ val c0 = line.charAt(p); val c1 = line.charAt(p + 1)
+ if (c0 == '/' && c1 == '*') { level += 1; p += 2 }
+ else if (c0 == '*' && c1 == '/') { level -= 1; p += 2 }
+ else p += 1
+ }
+ if (level > 0) { chunkHasUnclosed = true; n } else p
}
- // Check the last char is end of nested bracketed comment.
- val endOfBracketedComment = leavingBracketedComment && bracketedCommentLevel == 1 &&
- !hasPrecedingNonCommentString
- // Spark SQL support simple comment and nested bracketed comment in query body.
- // But if Spark SQL receives a comment alone, it will throw parser exception.
- // In Spark SQL CLI, if there is a completed comment in the end of whole query,
- // since Spark SQL CLL use `;` to split the query, CLI will pass the comment
- // to the backend engine and throw exception. CLI should ignore this comment,
- // If there is an uncompleted statement or an uncompleted bracketed comment in the end,
- // CLI should also pass this part to the backend engine, which may throw an exception
- // with clear error message (for incomplete statement, if there's non comment characters,
- // we would still append the string).
- if (!endOfBracketedComment && (isStatement || insideBracketedComment)) {
- ret.append(line.substring(beginIndex))
+
+ while (i < n) {
+ val c = line.charAt(i)
+ def peek(ch: Char): Boolean = i + 1 < n && line.charAt(i + 1) == ch
+ if (c == '\'' || c == '"' || c == '`') {
+ chunkHasSql = true; i = consumeString(i, c)
+ } else if (c == '-' && peek('-')) {
+ i = consumeLineComment(i)
+ } else if (c == '/' && peek('*')) {
+ i = consumeBlockComment(i)
+ } else if (c == ';') {
+ if (chunkHasSql) ret += line.substring(chunkStart, i)
+ chunkStart = i + 1; chunkHasSql = false; chunkHasUnclosed = false; i += 1
+ } else {
+ if (!Character.isWhitespace(c)) chunkHasSql = true
+ i += 1
+ }
}
+ if (chunkHasSql || chunkHasUnclosed) ret += line.substring(chunkStart)
ret.toList
}
}
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/StringUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/StringUtilsSuite.scala
index 77ae1a96ac45..470ace2bcdcd 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/StringUtilsSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/StringUtilsSuite.scala
@@ -287,5 +287,68 @@ class StringUtilsSuite extends SparkFunSuite with SQLHelper {
|END""".stripMargin
)
)
+
+ // SPARK-54876: statement after semicolon ending with block comment should not be dropped
+ assert(
+ splitSemiColonWithIndex(
+ "SELECT 1; SELECT 2 /* comment */",
+ enableSqlScripting = false) == Seq("SELECT 1", " SELECT 2 /* comment */")
+ )
+
+ // SPARK-54876: line comment followed by block comment should produce empty result
+ assert(
+ splitSemiColonWithIndex(
+ "-- foo\n/* bar */",
+ enableSqlScripting = false) == Seq()
+ )
+
+ // SPARK-54876: line comment before block comment after semicolon
+ assert(
+ splitSemiColonWithIndex(
+ "SELECT 1; -- foo\n /* bar */",
+ enableSqlScripting = false) == Seq("SELECT 1")
+ )
+
+ // SPARK-54876: nested block comments
+ assert(
+ splitSemiColonWithIndex(
+ "SELECT 1; /* outer /* inner */ */",
+ enableSqlScripting = false) == Seq("SELECT 1")
+ )
+
+ // SPARK-54876: preceding closed block comment + line comment (no SQL statement)
+ assert(
+ splitSemiColonWithIndex(
+ "/* a */ -- foo\n/* b */",
+ enableSqlScripting = false) == Seq()
+ )
+
+ // SPARK-54876: unterminated block comment at EOF
+ assert(
+ splitSemiColonWithIndex(
+ "SELECT 1; /* unterminated",
+ enableSqlScripting = false) == Seq("SELECT 1", " /* unterminated")
+ )
+
+ // SPARK-54876: unterminated string literal (single input, no semicolon)
+ assert(
+ splitSemiColonWithIndex(
+ "'unterminated",
+ enableSqlScripting = false) == Seq("'unterminated")
+ )
+
+ // SPARK-54876: unterminated string literal after semicolon
+ assert(
+ splitSemiColonWithIndex(
+ "SELECT 1; 'unterminated string",
+ enableSqlScripting = false) == Seq("SELECT 1", " 'unterminated string")
+ )
+
+ // SPARK-54876: unterminated block comment (single input, no semicolon)
+ assert(
+ splitSemiColonWithIndex(
+ "/* only a comment that never closes",
+ enableSqlScripting = false) == Seq("/* only a comment that never closes")
+ )
}
}
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 0a024fb10ee0..ebe5fca9f1c4 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
@@ -41,7 +41,7 @@ import org.apache.spark.deploy.SparkHadoopUtil
import org.apache.spark.internal.Logging
import org.apache.spark.internal.LogKeys._
import org.apache.spark.sql.catalyst.analysis.FunctionRegistry
-import org.apache.spark.sql.catalyst.util.SQLKeywordUtils
+import org.apache.spark.sql.catalyst.util.{SQLKeywordUtils, StringUtils}
import org.apache.spark.sql.hive.client.HiveClientImpl
import org.apache.spark.sql.hive.security.HiveDelegationTokenProvider
import org.apache.spark.sql.hive.thriftserver.SparkSQLCLIDriver.closeHiveSessionStateIfStarted
@@ -613,110 +613,9 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
}
}
- // Adapted splitSemiColon from Hive 2.3's CliDriver.splitSemiColon.
- // Note: [SPARK-31595] if there is a `'` in a double quoted string, or a `"` in a single quoted
- // string, the origin implementation from Hive will not drop the trailing semicolon as expected,
- // hence we refined this function a little bit.
- // Note: [SPARK-33100] Ignore a semicolon inside a bracketed comment in spark-sql.
- private[hive] def splitSemiColon(line: String): JList[String] = {
- var insideSingleQuote = false
- var insideDoubleQuote = false
- var insideSimpleComment = false
- var bracketedCommentLevel = 0
- var escape = false
- var beginIndex = 0
- var leavingBracketedComment = false
- var isStatement = false
- val ret = new JArrayList[String]
-
- def insideBracketedComment: Boolean = bracketedCommentLevel > 0
- def insideComment: Boolean = insideSimpleComment || insideBracketedComment
- def statementInProgress(index: Int): Boolean = isStatement || (!insideComment &&
- index > beginIndex && !s"${line.charAt(index)}".trim.isEmpty)
-
- for (index <- 0 until line.length) {
- // Checks if we need to decrement a bracketed comment level; the last character '/' of
- // bracketed comments is still inside the comment, so `insideBracketedComment` must keep true
- // in the previous loop and we decrement the level here if needed.
- if (leavingBracketedComment) {
- bracketedCommentLevel -= 1
- leavingBracketedComment = false
- }
-
- if (line.charAt(index) == '\'' && !insideComment) {
- // take a look to see if it is escaped
- // See the comment above about SPARK-31595
- if (!escape && !insideDoubleQuote) {
- // flip the boolean variable
- insideSingleQuote = !insideSingleQuote
- }
- } else if (line.charAt(index) == '\"' && !insideComment) {
- // take a look to see if it is escaped
- // See the comment above about SPARK-31595
- if (!escape && !insideSingleQuote) {
- // flip the boolean variable
- insideDoubleQuote = !insideDoubleQuote
- }
- } else if (line.charAt(index) == '-') {
- val hasNext = index + 1 < line.length
- if (insideDoubleQuote || insideSingleQuote || insideComment) {
- // Ignores '-' in any case of quotes or comment.
- // Avoids to start a comment(--) within a quoted segment or already in a comment.
- // Sample query: select "quoted value --"
- // ^^ avoids starting a comment if it's inside quotes.
- } else if (hasNext && line.charAt(index + 1) == '-') {
- // ignore quotes and ; in simple comment
- insideSimpleComment = true
- }
- } else if (line.charAt(index) == ';') {
- if (insideSingleQuote || insideDoubleQuote || insideComment) {
- // do not split
- } else {
- if (isStatement) {
- // split, do not include ; itself
- ret.add(line.substring(beginIndex, index))
- }
- beginIndex = index + 1
- isStatement = false
- }
- } else if (line.charAt(index) == '\n') {
- // with a new line the inline simple comment should end.
- if (!escape) {
- insideSimpleComment = false
- }
- } else if (line.charAt(index) == '/' && !insideSimpleComment) {
- val hasNext = index + 1 < line.length
- if (insideSingleQuote || insideDoubleQuote) {
- // Ignores '/' in any case of quotes
- } else if (insideBracketedComment && line.charAt(index - 1) == '*' ) {
- // Decrements `bracketedCommentLevel` at the beginning of the next loop
- leavingBracketedComment = true
- } else if (hasNext && line.charAt(index + 1) == '*') {
- bracketedCommentLevel += 1
- }
- }
- // set the escape
- if (escape) {
- escape = false
- } else if (line.charAt(index) == '\\') {
- escape = true
- }
-
- isStatement = statementInProgress(index)
- }
- // Check the last char is end of nested bracketed comment.
- val endOfBracketedComment = leavingBracketedComment && bracketedCommentLevel == 1
- // Spark SQL support simple comment and nested bracketed comment in query body.
- // But if Spark SQL receives a comment alone, it will throw parser exception.
- // In Spark SQL CLI, if there is a completed comment in the end of whole query,
- // since Spark SQL CLL use `;` to split the query, CLI will pass the comment
- // to the backend engine and throw exception. CLI should ignore this comment,
- // If there is an uncompleted statement or an uncompleted bracketed comment in the end,
- // CLI should also pass this part to the backend engine, which may throw an exception
- // with clear error message.
- if (!endOfBracketedComment && (isStatement || insideBracketedComment)) {
- ret.add(line.substring(beginIndex))
- }
- ret
- }
+ // Splits SQL into individual statements by top-level semicolons. See
+ // [[StringUtils.splitSemiColonWithIndex]] for the implementation.
+ // Note: [SPARK-31595], [SPARK-33100], [SPARK-54876]
+ private[hive] def splitSemiColon(line: String): JList[String] =
+ StringUtils.splitSemiColonWithIndex(line, enableSqlScripting = false).asJava
}
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 5f25ca02d2c4..5debfd168404 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
@@ -674,7 +674,18 @@ class CliSuite extends SparkFunSuite {
"SELECT /* comment */ 1;" -> Seq("SELECT /* comment */ 1"),
"-- comment " -> Seq(),
"-- comment \nSELECT 1" -> Seq("-- comment \nSELECT 1"),
- "/* comment */ " -> Seq()
+ "/* comment */ " -> Seq(),
+ // SPARK-54876: statement after semicolon ending with block comment should not be dropped
+ "SELECT 1; SELECT 2 /* comment */" -> Seq("SELECT 1", " SELECT 2 /* comment */"),
+ // SPARK-54876: line comment followed by block comment should produce empty result
+ "-- foo\n/* bar */" -> Seq(),
+ "SELECT 1; -- foo\n /* bar */" -> Seq("SELECT 1"),
+ // SPARK-54876: nested block comments
+ "SELECT 1; /* outer /* inner */ */" -> Seq("SELECT 1"),
+ // SPARK-54876: preceding closed block comment + line comment (no SQL statement)
+ "/* a */ -- foo\n/* b */" -> Seq(),
+ // SPARK-54876: semicolons inside backtick-quoted identifiers are not split points
+ "SELECT * FROM `t;a`; SELECT 1" -> Seq("SELECT * FROM `t;a`", " SELECT 1")
).foreach { case (query, ret) =>
assert(cli.splitSemiColon(query).asScala === ret)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org