[SPARK-59255][SQL] Extend parse_sql to parse SQL batches

[srielau]

What changes were proposed in this pull request?

Extend the experimental parse_sql function so it can parse a batch of SQL statements ('select 1; select 2') instead of a single statement.

parse_sql now:

• Splits the input with SqlStatementSplitter (the same splitter used by SparkSqlParser.splitStatements).
• Parses each statement independently and returns a JSON array of statement objects.
• Adds start (1-based offset in the original batch) and length (trimmed statement text, excluding surrounding whitespace and the terminating semicolon) on every statement object.
• Continues after a parse failure so later statements are still described.

Well-formed BEGIN ... END scripts remain a single array element. Nested error locations stay statement-relative; start is relative to the original batch. Empty or comment-only input returns []. NULL still returns SQL NULL.

The splitter now records source positions internally so spans are taken from token offsets rather than reconstructed with indexOf (which would mis-bind when a dropped comment repeats later statement text).

Why are the changes needed?

Users of the experimental parse_sql function asked to parse batches such as 'select 1; select 2'. Source spans are needed so consumers can highlight each sub-statement in the original text.

JIRA: https://issues.apache.org/jira/browse/SPARK-59255

Does this PR introduce any user-facing change?

Yes, behind spark.sql.function.parseSql.enabled (still off by default; the JSON contract is documented as evolving).

Previously a successful parse returned one JSON object:

{"parse_success":true,"statement_identifier":"SELECT","statement_code":21,"select_list":[{"name":[]}]}

Now the same input is wrapped in an array and includes source spans:

[{"start":1,"length":8,"parse_success":true,"statement_identifier":"SELECT","statement_code":21,"select_list":[{"name":[]}]}]

A two-statement batch:

SELECT parse_sql('select 1; select 2')

[
  {"start":1,"length":8,"parse_success":true,"statement_identifier":"SELECT","statement_code":21,"select_list":[{"name":[]}]},
  {"start":11,"length":8,"parse_success":true,"statement_identifier":"SELECT","statement_code":21,"select_list":[{"name":[]}]}
]

JSON paths such as $.statement_identifier become $[0].statement_identifier. Empty SQL that previously produced a parse-failure object now returns [].

How was this patch tested?

• SqlStatementSplitterSuite (including comment / empty-; span recovery)
• ParseSqlResultSuite and ParseSqlSuite
• SQLQueryTestSuite -- -z parse-sql.sql (goldens regenerated)
• ExpressionsSchemaSuite and ExpressionInfoSuite example-output check
• catalyst/scalastyle and sql/scalastyle

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Cursor Grok 4.6

[cloud-fan]

Review summary

The new batch result shape, per-statement error handling, valid compound-script handling, documentation, and broad ASCII test coverage are consistent. The remaining issue is the source-position boundary: supplementary Unicode characters can corrupt both the advertised spans and the splitter's candidate validation, so the positional conversion and a focused regression should be fixed before merge. The pinned Build check was still in progress; no test failures were available to assess.

Findings

1 total: 0 P0, 1 P1, 0 P2, 0 P3.

Blocking (P1)

• Convert ANTLR offsets before using them as String spans — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:66 — see inline.

[cloud-fan]

Review summary

The batch parsing design and the prior supplementary-Unicode correctness fix are sound, but three source-span issues remain. Repeated from-zero offset conversions make ordinary multi-statement batches quadratic; String.trim leaves lexer-recognized Unicode whitespace in the returned spans; and clients are not told that the public coordinates count UTF-16 code units. These should be addressed before relying on the new batch contract.

Findings

3 total: 0 P0, 0 P1, 3 P2, 0 P3.

Non-blocking (P2)

• Document the UTF-16 unit used by source spans — sql/core/src/main/scala/org/apache/spark/sql/catalyst/expressions/ParseSql.scala:42 — see inline.
• Avoid rescanning the SQL prefix for every statement position — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:197 — see inline.
• Trim spans with Spark SQL whitespace semantics — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:210 — see inline.

Re-review status

Prior AI findings: 1 addressed, 0 still present; additional unresolved findings in this review: 3.

New attribution: 1 newly introduced, 2 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

Verification

• The previous supplementary-Unicode correctness failure is resolved at the pinned head; the retained performance issue is a distinct consequence of the new conversion strategy.

PR metadata suggestions

• Define the coordinate unit for start and length in the PR description; the current implementation and tests use UTF-16 code units, while the examples only cover ASCII.

[cloud-fan]

Review summary

The positioned batch design, UTF-16 spans, normal error isolation, and valid compound handling are coherent, but malformed compound blocks cross a critical recovery boundary. An inner typo currently changes one top-level BEGIN ... END statement into three unrelated parse_sql elements, so the splitter recovery and focused regression coverage need correction before the batch contract is ready.

Findings

1 total: 0 P0, 1 P1, 0 P2, 0 P3.

Blocking (P1)

• Preserve malformed compound statements as one batch element — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:315 — see inline.

Re-review status

Prior AI findings: 3 addressed, 0 still present; additional unresolved findings in this review: 1.

New attribution: 0 newly introduced, 1 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

Verification

• The malformed balanced compound is reachable through the public parse_sql path and is emitted as three result objects by the checked-in golden output.

[cloud-fan]

Review summary

The batch parsing design and the prior Unicode/source-span fixes are sound, but malformed balanced compound statements still cross the splitter's recovery boundary. When an inner statement fails, FailedNonEof recovery splits at compound-body semicolons, so one BEGIN ... END statement becomes three errors; appending SELECT 3 yields four array elements instead of the two top-level statements. Compound-aware recovery should retain the matching outer END, preserve the ordinary-invalid and parser-extension fallbacks, add a malformed-block-plus-following-statement regression, and regenerate the SQL golden. This is the same blocking issue already present in the current inline thread, so the review should not post a duplicate inline comment.

Findings

1 total: 0 P0, 1 P1, 0 P2, 0 P3.

Blocking (P1)

• Preserve malformed compound statements as one batch element — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:315 — already raised in an existing discussion.

Re-review status

Prior AI findings: 0 addressed, 1 still present; additional unresolved findings in this review: 0.

New attribution: 0 newly introduced, 0 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

• Preserve malformed compound statements as one batch element — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:315

Existing discussions

• Suppressed duplicate: Preserve malformed compound statements as one batch element — P1 at sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:315 — existing discussion

Verification

• The pinned SQL golden records three error objects for one balanced malformed BEGIN ... END input, matching the FailedNonEof branch that emits at the first internal semicolon.

[cloud-fan]

Review summary

The flat malformed-BEGIN fix is progress, but the recovery proof is still incomplete: DefaultErrorStrategy can bind the outer rule's END to an earlier control terminator and recover across its suffix to EOF, splitting one balanced script into several parse_sql results. The PR should validate the token suffix after the recovered END and cover a malformed nested-control case before it is ready. The existing BailErrorStrategy Scaladoc also needs to describe the new recovery arm. Separately, the documented O(k^2) compound-prefix algorithm now runs per DataFrame row; that accepted design boundary should be reconsidered for this scalar-function consumer. The pinned Build check was still queued, and no local test command was run during this review.

Findings

2 total: 0 P0, 1 P1, 0 P2, 1 P3.

Blocking (P1)

• Require the recovered END to be the outer terminator — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:425 — see inline.

Nit (P3)

• Document the non-bailing parser configuration arm — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/SqlStatementSplitter.scala:551 — see inline.

Scope and assumption challenges

• Reconsider the quadratic splitter path for DataFrame evaluation — The accepted splitter boundary allows O(k^2) growing-prefix parsing for compound scripts, but parse_sql now pays that cost independently on every DataFrame row and then parses the completed script again. Since this expression is documented for batch evaluation, please reconsider that tradeoff and use a boundary-specific compound path that identifies the outer END in one pass while retaining EOF-anchored handling for ordinary wildcard statements.

Re-review status

Prior AI findings: 1 addressed, 0 still present; additional unresolved findings in this review: 2.

New attribution: 2 newly introduced, 0 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

Existing discussions

• existing discussion — The flat malformed-block case is fixed, but the error-recovering parser can select an earlier real inner END and skip its control suffix to EOF.

Verification

• The nested recovery defect is reachable only on the parse_sql opt-in path; generic split callers keep preserveMalformedCompoundBoundaries=false.

[cloud-fan]

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.

[srielau]

Fixed in afbd833. Recovery now requires a real recovered END and independently verifies the recovered candidate default-channel suffix as a real outer END followed only by the optional semicolon and EOF. This prevents an inner END IF from terminating the outer block. Added focused SqlStatementSplitterSuite and ParseSqlResultSuite regressions for BEGIN IFF TRUE THEN SELECT 1; END IF; SELECT 2; END; SELECT 3; they verify exactly one failed outer block plus one successful SELECT with correct spans. The full splitter, ParseSqlResult, and parse_sql suites and Catalyst/SQL Scalastyle checks pass.

[cloud-fan]

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.

[srielau]

Fixed in afbd833. The Scaladoc now qualifies bail-strategy installation with bailOnError and explains that malformed-compound recovery intentionally retains DefaultErrorStrategy so it can inspect the recovered outer END boundary.