HyukjinKwon commented on PR #54946: URL: https://github.com/apache/spark/pull/54946#issuecomment-4887841149
**Reviewed — 0 blocking, 2 nits.** Solid, well-tested redesign; the placeholder-validation hybrid cleanly resolves the prior round's substitution-ordering finding and the splitter's failure-mode handling is exhaustive. Two comment nits: - `SqlStatementSplitter.scala:101` — the performance-note Scaladoc says `tryParseSegment`; the method is `tryParseRegion`. - `SparkSQLCLIDriver.scala:245` — "This matches the pre-PR behavior" reads relative to this PR's history and loses its meaning once merged; better to state the invariant directly (e.g. "Line-level buffering keeps multi-line input accumulating until the user signals end-of-statement with `;`"). Verification: enumerated `tryParseRegion`'s ANTLR parse state space (SLL success, SLL-bail→LL success, LL-bail-at-EOF vs not-at-EOF, `StackOverflowError` under either stage) — all handled at `SqlStatementSplitter.scala:355-374`, and an unclosed bracketed comment is surfaced via the lexer flag rather than thrown (`:287-290`). So a well-formed `BEGIN..END` block is always confirmed whole and never split at an internal `;`, and malformed input always reaches the backend for a proper error. <!-- ai-code-review --> -- 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]
