cloud-fan commented on code in PR #58530:
URL: https://github.com/apache/spark/pull/58530#discussion_r3991402233
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -87,6 +89,97 @@ compoundOrSingleStatement
| singleCompoundStatement
;
+// Boundary-only grammar for parse_sql batches. Leaf statements deliberately
accept arbitrary
+// tokens: ParseSqlResult parses each emitted segment with the full grammar
and records any error.
+// BEGIN is excluded from the terminated fallback, so only a grammar context
can own the
+// semicolons inside a compound statement. BEGIN and END remain unrestricted
inside leaf
+// statements. The caller appends PARSE_SQL_BATCH_DELIMITER; the trailing
BEGIN fallback consumes
+// a structurally unclosed compound through that token without synthesizing an
END token.
+parseSqlBatch
+ : SEMICOLON* (items+=parseSqlBatchItem SEMICOLON*)*
+ PARSE_SQL_BATCH_DELIMITER? EOF
+ ;
+
+parseSqlBatchItem
+ : batchStatement=parseSqlBatchStatement
+ terminator=(SEMICOLON | PARSE_SQL_BATCH_DELIMITER)
+ | partialStatement=parseSqlBatchPartialCompoundStatement
+ terminator=PARSE_SQL_BATCH_DELIMITER
+ ;
+
+parseSqlBatchStatement
+ : parseSqlBatchCompoundStatement
+ | parseSqlBatchLeafStatement
+ ;
+
+parseSqlBatchPartialCompoundStatement
+ : BEGIN .*?
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3979010251","thread_id":"inline:3979010251","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -87,6 +89,97 @@ compoundOrSingleStatement
| singleCompoundStatement
;
+// Boundary-only grammar for parse_sql batches. Leaf statements deliberately
accept arbitrary
+// tokens: ParseSqlResult parses each emitted segment with the full grammar
and records any error.
+// BEGIN is excluded from the terminated fallback, so only a grammar context
can own the
+// semicolons inside a compound statement. BEGIN and END remain unrestricted
inside leaf
+// statements. The caller appends PARSE_SQL_BATCH_DELIMITER; the trailing
BEGIN fallback consumes
+// a structurally unclosed compound through that token without synthesizing an
END token.
+parseSqlBatch
+ : SEMICOLON* (items+=parseSqlBatchItem SEMICOLON*)*
+ PARSE_SQL_BATCH_DELIMITER? EOF
+ ;
+
+parseSqlBatchItem
+ : batchStatement=parseSqlBatchStatement
+ terminator=(SEMICOLON | PARSE_SQL_BATCH_DELIMITER)
+ | partialStatement=parseSqlBatchPartialCompoundStatement
+ terminator=PARSE_SQL_BATCH_DELIMITER
+ ;
+
+parseSqlBatchStatement
+ : parseSqlBatchCompoundStatement
+ | parseSqlBatchLeafStatement
+ ;
+
+parseSqlBatchPartialCompoundStatement
+ : BEGIN .*?
+ ;
+
+parseSqlBatchCompoundStatement
+ : BEGIN (NOT ATOMIC)? parseSqlBatchCompoundBody? END
+ ;
+
+parseSqlBatchBeginEndCompoundBlock
+ : beginLabel? BEGIN (NOT ATOMIC)? parseSqlBatchCompoundBody? END endLabel?
+ ;
+
+parseSqlBatchCompoundBody
+ : (parseSqlBatchCompoundBodyStatement SEMICOLON)+
+ ;
+
+parseSqlBatchCompoundBodyStatement
+ : parseSqlBatchBeginEndCompoundBlock
+ | parseSqlBatchDeclareHandlerStatement
+ | parseSqlBatchIfElseStatement
+ | parseSqlBatchCaseStatement
+ | parseSqlBatchWhileStatement
+ | parseSqlBatchRepeatStatement
+ | parseSqlBatchLoopStatement
+ | parseSqlBatchForStatement
+ | parseSqlBatchLeafStatement
+ ;
+
+parseSqlBatchDeclareHandlerStatement
+ : DECLARE (CONTINUE | EXIT) HANDLER FOR conditionValues
+ (parseSqlBatchBeginEndCompoundBlock | parseSqlBatchLeafStatement)
+ ;
+
+parseSqlBatchWhileStatement
+ : beginLabel? WHILE booleanExpression DO parseSqlBatchCompoundBody END
WHILE endLabel?
+ ;
+
+parseSqlBatchIfElseStatement
+ : IF booleanExpression THEN parseSqlBatchCompoundBody
+ (ELSEIF booleanExpression THEN parseSqlBatchCompoundBody)*
+ (ELSE parseSqlBatchCompoundBody)? END IF
+ ;
+
+parseSqlBatchRepeatStatement
+ : beginLabel? REPEAT parseSqlBatchCompoundBody UNTIL booleanExpression END
REPEAT endLabel?
+ ;
+
+parseSqlBatchCaseStatement
+ : CASE (WHEN booleanExpression THEN parseSqlBatchCompoundBody)+
+ (ELSE parseSqlBatchCompoundBody)? END CASE
+ | CASE expression (WHEN expression THEN parseSqlBatchCompoundBody)+
+ (ELSE parseSqlBatchCompoundBody)? END CASE
+ ;
+
+parseSqlBatchLoopStatement
+ : beginLabel? LOOP parseSqlBatchCompoundBody END LOOP endLabel?
+ ;
+
+parseSqlBatchForStatement
+ : beginLabel? FOR (strictIdentifier AS)? query DO
+ parseSqlBatchCompoundBody END FOR endLabel?
+ ;
+
+parseSqlBatchLeafStatement
+ : {_input.LA(1) != BEGIN}?
+ (~(SEMICOLON | PARSE_SQL_BATCH_DELIMITER))+
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3979010255","thread_id":"inline:3979010255","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/expressions/ParseSql.scala:
##########
@@ -27,38 +27,46 @@ import org.apache.spark.sql.types.{AbstractDataType,
DataType, StringType}
import org.apache.spark.unsafe.types.UTF8String
/**
- * Parses a SQL statement string and returns a compact JSON description of the
- * unresolved statement (identifier/code, lineage references, select-list
names,
- * parameters), or a STANDARD-format error object when the statement does not
- * parse.
+ * Parses a SQL batch string and returns a compact JSON array describing its
+ * unresolved statements (source position, identifier/code, lineage references,
+ * select-list names, parameters). A statement that does not parse is
represented
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3971952404","thread_id":"inline:3971952404","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseSqlResult.scala:
##########
@@ -43,12 +43,18 @@ import
org.apache.spark.sql.execution.datasources.CreateTempViewUsing
* executors without a session, so only the stock parser is available under
* distributed eval.
*
- * On success the JSON always includes `parse_success`, the statement
+ * Every statement object includes its 1-based UTF-16 code-unit `start` in the
+ * original batch and its UTF-16 code-unit `length`, excluding surrounding
+ * whitespace and the terminating semicolon. On success it also includes
+ * `parse_success`, the statement
* identifier/code (ISO/IEC 9075-2:2023 Table 39), and omits unused optional
* fields (`target_table_references`, `source_table_references`,
- * `function_references`, `select_list`, `parameter_markers`) when empty. On
parse
- * failure it returns `parse_success: false` with source location and a nested
- * STANDARD-format error object, and does not throw. Only [[ParseException]] /
+ * `function_references`, `select_list`, `parameter_markers`) when empty. On
+ * parse failure the statement object contains `parse_success: false` with
+ * source location and a nested STANDARD-format error object, and parsing
+ * continues with later statements. Nested error locations are relative to the
+ * individual statement, while `start` is relative to the original batch. An
+ * empty or comment-only batch produces an empty array. Only
[[ParseException]] /
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3979010263","thread_id":"inline:3979010263","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:
##########
@@ -97,21 +117,116 @@ case class SqlStatementSplitResult(
* for real at execution time. When `validationPreprocess` is `identity`
* (the default), the splitter behaves as a pure original-text splitter.
*
- * Performance note: for a single `BEGIN ... END` block with k internal `;`,
- * the splitter calls `tryParseRegion` O(k) times on growing prefixes -- an
+ * Performance note: the generic splitter calls `tryParseRegion` O(k) times on
+ * growing prefixes for a single `BEGIN ... END` block with k internal `;` --
an
* O(k^2) cost in the worst case (incomplete block on every keystroke in
* interactive mode). Ordinary non-scripting SQL is O(n). A non-EOF terminated
* single-statement rule (read `ctx.getStop` once per region) would make this
* O(n), but Spark's `setResetStatement` has `SET .*?` / `RESET .*?` wildcards
* that need an EOF anchor to terminate deterministically, so such a
* single-statement rule-rewrite does not drop in cleanly. Tracked as a
- * follow-up.
+ * follow-up. The parse_sql-only path uses [[splitForParseSql]] and performs
one
+ * linear boundary parse instead.
*/
object SqlStatementSplitter {
/** Split the given SQL text into individual statements at `;` boundaries. */
def split(sqlText: String): SqlStatementSplitResult =
- split(sqlText, identity)
+ splitWithPositions(sqlText, identity).withoutPositions
+
+ /**
+ * Split a parse_sql batch in one grammar-owned pass while retaining source
positions.
+ * Unlike the generic splitter, this boundary-only grammar accepts malformed
leaf statements
+ * and uses scripting grammar contexts to assign internal semicolons to
compound statements.
+ */
+ private[sql] def splitForParseSql(sqlText: String):
PositionedSqlStatementSplitResult = {
+ require(sqlText != null, "sqlText must not be null")
+
+ val toUtf16 = utf16Offsets(sqlText)
+ val sourceLexer = new SqlBaseLexer(new
UpperCaseCharStream(CharStreams.fromString(sqlText)))
+ sourceLexer.removeErrorListeners()
+ val sourceTokens = new CommonTokenStream(sourceLexer)
+ sourceTokens.fill()
+ val boundaryTokens = new java.util.ArrayList[Token](sourceTokens.size() +
1)
+ var sourceIndex = 0
+ while (sourceIndex < sourceTokens.size() - 1) {
+ boundaryTokens.add(sourceTokens.get(sourceIndex))
+ sourceIndex += 1
+ }
+ val boundary = new CommonToken(SqlBaseParser.PARSE_SQL_BATCH_DELIMITER, "")
+ boundary.setStartIndex(toUtf16.length - 1)
+ boundary.setStopIndex(toUtf16.length - 2)
+ boundaryTokens.add(boundary)
+ boundaryTokens.add(sourceTokens.get(sourceTokens.size() - 1))
+ val tokens = new CommonTokenStream(new ListTokenSource(boundaryTokens))
+ tokens.fill()
+ val parser = new SqlBaseParser(tokens)
+ configureSplitterParser(parser, SqlApiConf.get)
+ parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+ val batch = try {
+ parser.parseSqlBatch()
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3979010260","thread_id":"inline:3979010260","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:
##########
@@ -285,8 +370,155 @@ object SqlStatementSplitter {
val unclosed = lexer.has_unclosed_bracketed_comment
val partial =
- if (bufferHasContent || unclosed) buffer.toString.trim else ""
- SqlStatementSplitResult(completeStatements.toSeq, partial, unclosed &&
partial.nonEmpty)
+ if (bufferHasContent || unclosed) positionedStatement("") else None
+ PositionedSqlStatementSplitResult(
+ completeStatements.toSeq,
+ partial,
+ unclosed && partial.nonEmpty)
+ }
+
+ /**
+ * Returns the delimiter-array index and ending token index of a real outer
END for a malformed
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3971952394","thread_id":"inline:3971952394","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:
##########
@@ -285,8 +370,155 @@ object SqlStatementSplitter {
val unclosed = lexer.has_unclosed_bracketed_comment
val partial =
- if (bufferHasContent || unclosed) buffer.toString.trim else ""
- SqlStatementSplitResult(completeStatements.toSeq, partial, unclosed &&
partial.nonEmpty)
+ if (bufferHasContent || unclosed) positionedStatement("") else None
+ PositionedSqlStatementSplitResult(
+ completeStatements.toSeq,
+ partial,
+ unclosed && partial.nonEmpty)
+ }
+
+ /**
+ * Returns the delimiter-array index and ending token index of a real outer
END for a malformed
+ * compound statement. Error recovery may repair the body, but a missing END
is synthetic and
+ * has token index -1.
+ */
+ private def findMalformedCompoundEnd(
+ sqlText: String,
+ toUtf16: Array[Int],
+ stream: CommonTokenStream,
+ startIdx: Int,
+ delimiterPositions: Array[Int],
+ fromDelimiter: Int,
+ validationPreprocess: String => String,
+ conf: SqlApiConf): Option[(Int, Int)] = {
+ if (stream.get(startIdx).getType != SqlBaseLexer.BEGIN) {
+ return None
+ }
+
+ var delimiter = fromDelimiter
+ while (delimiter <= delimiterPositions.length) {
+ val endIdx = if (delimiter < delimiterPositions.length) {
+ delimiterPositions(delimiter)
+ } else {
+ stream.size() - 1
+ }
+ val firstTok = stream.get(startIdx)
+ val lastTok = stream.get(endIdx)
+ val regionStart = toUtf16(firstTok.getStartIndex)
+ val regionEnd = if (lastTok.getType == Token.EOF) {
+ sqlText.length
+ } else {
+ toUtf16(lastTok.getStopIndex + 1)
+ }
+ val candidate = validationPreprocess(sqlText.substring(regionStart,
regionEnd))
Review Comment:
Confirmed in the pinned head that this exact issue is resolved. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3971952386","thread_id":"inline:3971952386","verdict_sha256":"1c873ce5350b0afbb843eeabb7da0c9a30fa6a9bfbebf518124010218135d0ad"}
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
