cloud-fan commented on code in PR #58530:
URL: https://github.com/apache/spark/pull/58530#discussion_r3962199855
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:
##########
@@ -285,8 +370,67 @@ 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))
+ val lexer = new SqlBaseLexer(
+ new UpperCaseCharStream(CharStreams.fromString(candidate)))
+ lexer.removeErrorListeners()
+ val tokens = new CommonTokenStream(lexer)
+ tokens.fill()
+ val parser = new SqlBaseParser(tokens)
+ configureSplitterParser(parser, conf, bailOnError = false)
+ parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+ try {
+ val context = parser.singleCompoundStatement()
+ val end = context.END()
+ if (end != null && end.getSymbol.getTokenIndex >= 0 && tokens.LA(1) ==
Token.EOF) {
Review Comment:
**Blocking (P1):** `tokens.LA(1) == EOF` only shows where recovery finished;
it does not prove that `context.END()` matched the outer block. With `BEGIN IFF
TRUE THEN SELECT 1; END IF; SELECT 2; END; SELECT 3`, the recovering parser can
use the real `END` from `END IF` for the outer rule and then consume `IF` while
recovering the expected EOF, so both checks pass and the containing script is
split early. Please verify on the recovered candidate's default-channel tokens
that only the optional semicolon and the real EOF follow this `END`, and add a
nested malformed-control regression with a following top-level statement.
**Recommended change:** Accept a recovered compound boundary only when the
selected END is followed by the outer rule's optional semicolon and real EOF,
then cover nested control terminators.
**Why this works:** DefaultErrorStrategy may populate context.END with an
inner control END and skip the remaining control suffix while recovering the
EOF match; inspecting the original default-channel suffix distinguishes that
recovery artifact from a true outer boundary.
**Scope:** SqlStatementSplitter.findMalformedCompoundEnd plus focused
SqlStatementSplitterSuite and ParseSqlResultSuite coverage.
**Compatibility:** Preserve the flat malformed-block repair and parse_sql's
one-result-per-top-level-statement behavior while leaving generic splitter
extension fallback unchanged.
**Risks:** The suffix check must still accept the optional outer semicolon
and must not classify a synthetic or nested END as complete.
**Constraints:** Do not consume a following top-level statement. Keep
preserveMalformedCompoundBoundaries disabled by default for generic splitter
callers.
**Success:** A malformed nested control block followed by SELECT 3 produces
exactly two results: one error spanning the complete outer BEGIN ... END and
one successful SELECT with the correct source span.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:
##########
@@ -399,7 +548,48 @@ object SqlStatementSplitter {
parser.single_character_pipe_operator_enabled =
conf.singleCharacterPipeOperatorEnabled
parser.removeErrorListeners()
- parser.setErrorHandler(new BailErrorStrategy)
+ if (bailOnError) {
Review Comment:
**Nit (P3):** The method Scaladoc still says this helper installs a bail
strategy so failures throw immediately, but this new false arm intentionally
leaves `DefaultErrorStrategy` in place for malformed-compound recovery. Please
qualify that promise with `bailOnError` and document why the recovery caller
uses the default strategy.
--
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]
