srielau commented on code in PR #58530:
URL: https://github.com/apache/spark/pull/58530#discussion_r3994768963
##########
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
Review Comment:
Follow-up in 87610602065: the private linear boundary parser and its claim
were removed. parse_sql now reuses the existing generic splitter, whose O(k^2)
compound-prefix worst case is already documented directly on
SqlStatementSplitter.
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -87,6 +89,181 @@ 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
+ | parseSqlBatchMalformedEmptyCompoundBlock
+ | parseSqlBatchMalformedBeginStatement
+ | parseSqlBatchLeafStatement
+ ;
+
+parseSqlBatchMalformedBeginStatement
+ : BEGIN
+ ;
+
+parseSqlBatchPartialCompoundStatement
+ : BEGIN .*?
+ ;
+
+parseSqlBatchCompoundStatement
+ : BEGIN (NOT ATOMIC)? parseSqlBatchCompoundBody? END
+ ;
+
+parseSqlBatchBeginEndCompoundBlock
+ : beginLabel? BEGIN (NOT ATOMIC)? parseSqlBatchCompoundBody? END endLabel?
+ ;
+
+parseSqlBatchMalformedEmptyCompoundBlock
+ : beginLabel? BEGIN (NOT ATOMIC)? SEMICOLON END endLabel?
+ ;
+
+parseSqlBatchMalformedBodyBeginStatement
+ : BEGIN (~(SEMICOLON | PARSE_SQL_BATCH_DELIMITER))+
+ ;
+
+parseSqlBatchCompoundBody
+ : (parseSqlBatchCompoundBodyStatement SEMICOLON)+
+ ;
+
+parseSqlBatchCompoundBodyStatement
+ : parseSqlBatchNestedStatement
+ | parseSqlBatchOrphanControlEndStatement
+ | parseSqlBatchBodyLeafStatement
+ ;
+
+parseSqlBatchNestedStatement
+ : parseSqlBatchBeginEndCompoundBlock
+ | parseSqlBatchMalformedEmptyCompoundBlock
+ | parseSqlBatchDeclareHandlerStatement
+ | parseSqlBatchIfElseStatement
+ | parseSqlBatchCaseStatement
+ | parseSqlBatchWhileStatement
+ | parseSqlBatchRepeatStatement
+ | parseSqlBatchLoopStatement
+ | parseSqlBatchForStatement
+ | parseSqlBatchMalformedBodyBeginStatement
+ | parseSqlBatchMalformedBeginStatement
+ ;
+
+parseSqlBatchOrphanControlEndStatement
+ : END (IF | WHILE | LOOP | REPEAT | FOR | CASE)
+ ;
+
+parseSqlBatchDeclareHandlerStatement
+ : DECLARE (CONTINUE | EXIT) HANDLER FOR conditionValues
+ (parseSqlBatchBeginEndCompoundBlock
+ | parseSqlBatchMalformedEmptyCompoundBlock
+ | parseSqlBatchMalformedBodyBeginStatement
+ | parseSqlBatchBodyLeafStatement)
+ ;
+
+parseSqlBatchWhileStatement
+ : beginLabel? WHILE booleanExpression DO parseSqlBatchCompoundBody
Review Comment:
Follow-up in 87610602065: after stepping back, I removed the private
parse_sql batch grammar entirely. parse_sql again uses the existing positioned
SqlStatementSplitter path, including its existing `${...}` validation
preprocessor, and then stock-parses each emitted segment. Thus there is no
separate WHILE/REPEAT/FOR boundary grammar to keep synchronized.
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -87,6 +89,181 @@ 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
+ | parseSqlBatchMalformedEmptyCompoundBlock
+ | parseSqlBatchMalformedBeginStatement
+ | parseSqlBatchLeafStatement
+ ;
+
+parseSqlBatchMalformedBeginStatement
+ : BEGIN
+ ;
+
+parseSqlBatchPartialCompoundStatement
+ : BEGIN .*?
Review Comment:
Follow-up in 87610602065: I removed the parse_sql-specific
malformed-compound recovery rather than extending it further. This PR now
deliberately inherits the existing SqlStatementSplitter behavior for malformed
SQL; valid BEGIN ... END scripts remain atomic, while malformed input uses the
splitter existing best-effort semicolon fallback. Stronger malformed-script
recovery should be a separate improvement to the shared splitter, not a second
policy inside parse_sql.
--
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]
