srielau commented on code in PR #58530:
URL: https://github.com/apache/spark/pull/58530#discussion_r3994713357
##########
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:
Fixed in 6a76d8114e4. WHILE, REPEAT UNTIL, and FOR now use boundary-only
header rules with permissive delimiter-bounded fallbacks, while retaining the
full booleanExpression/query alternatives for ordinary valid headers. The
emitted text remains unchanged and SparkSqlParser still performs substitution
and semantic parsing. Added splitter regressions for raw `${...}` in all three
loop forms and result-level tests proving substituted scripts plus a following
SELECT parse as two successful results with original spans.
##########
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:
Fixed in 6a76d8114e4. The performance note now qualifies the linear claim as
the normal parse_sql path and explicitly states that a parser stack overflow
falls back to the generic path.
##########
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:
Fixed in 6a76d8114e4. The grammar-owned outer-compound rule now recognizes a
malformed suffix after its structural END through that statement semicolon, and
separately recovers a final body leaf missing its semicolon before END. `BEGIN
SELECT 1; END bad; SELECT 2` and `BEGIN SELECT 1 END; SELECT 2` now each
produce a failed first result followed by the successful SELECT with separate
spans. Existing nested and malformed compound regressions remain green.
--
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]
