srielau commented on code in PR #58530:
URL: https://github.com/apache/spark/pull/58530#discussion_r3980207918
##########
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:
Fixed in e7f6b1a09a3. The batch grammar now has a semicolon-terminated
malformed-BEGIN item, so `BEGIN; SELECT 1;` yields a failed BEGIN result
followed by the successful SELECT. Balanced malformed compounds continue
through their grammar-owned outer END and then resume the batch. Added
splitter- and ParseSqlResult-level order/span regressions.
##########
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:
Fixed in e7f6b1a09a3. Compound-body leaves now exclude a leading bare END,
reserving it for the enclosing grammar rule. Orphan `END
IF/WHILE/LOOP/REPEAT/FOR/CASE` forms remain explicit grammar-owned malformed
body statements, and IF/CASE clause bodies reserve their own clause markers.
The counterexample now produces the valid compound, stray END error, and SELECT
2 with correct spans.
##########
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:
Fixed in e7f6b1a09a3. The Scaladoc and ExpressionDescription now say that
empty or closed-comment-only batches return `[]`. An unterminated block comment
remains a single `parse_success:false` result, and a result-level regression
pins that behavior.
##########
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:
Fixed in e7f6b1a09a3 without adding a broad exception fallback. The boundary
grammar now directly owns malformed `BEGIN; END` blocks (including labels, NOT
ATOMIC, handlers, and nested controls), malformed BEGIN-led body statements,
empty malformed IF/CASE conditions, and premature END forms paired with their
real control closer. `BEGIN BEGIN; END; END; SELECT 3` now returns one failed
outer-script result plus the successful SELECT instead of throwing. Added
no-throw splitter and ParseSqlResult regressions, plus related
IF/CASE/WHILE/LOOP/FOR/REPEAT/handler cases.
--
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]
