Skip to content

test: add SQL error-class parity checks - #6650

Open
rich7420 wants to merge 3 commits into
apache:mainfrom
rich7420:feat/6617-sql-error-class-parity
Open

rich7420 wants to merge 3 commits into
apache:mainfrom
rich7420:feat/6617-sql-error-class-parity

Conversation

@rich7420

@rich7420 rich7420 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #6617.

Rationale for this change

Legacy expect_error checks message substrings and allows Spark fallback, so it cannot establish native execution or exact error-class parity. Add a strict SQL-fixture mode that uses the existing Scala error-checking contract.

What changes are included in this PR?

Add query expect_error_class(ERROR_CLASS) using checkSparkError to assert Comet operators, exact error class, exception-class and SQLSTATE parity, and no CometNativeException in either cause chain. Validate malformed directives with source locations and document the mode.

The strict mode can serve as a codegen error sentinel. Legacy message-only behavior remains unchanged, and query-context assertions stay in Scala.

How are these changes tested?

  • Current main-integrated head: 35 focused tests passed on Spark 4.1.3 / JDK 21, covering the parser, runner regressions and column-backed error fixture, with a freshly built matching native library.
  • Root-reactor compilation, Spotless and Scalastyle passed. Standalone parser checks passed on Scala 2.12 and 2.13, with unchanged parsing of all 538 existing main fixtures.
  • Current-head CI is running without reported failures. Additional Spark-profile runtime, Spark SQL and Iceberg suites have not run for this head.

@github-actions github-actions Bot added enhancement New feature or request test Testing related labels Oct 5, 2026
@rich7420
rich7420 marked this pull request as ready for review October 5, 2026 08:00

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

  • Prior state and problem: expect_error checks message substrings and permits Spark fallback, so it cannot establish Comet error-class parity.
  • Design approach: Add expect_error_class using the existing checkSparkError helper, preserving legacy behavior.
  • Correctness / compatibility analysis: The helper checks planned Comet operators, exact error class, exception type, SQLSTATE and cause chains. Spark source checks across 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 confirm the APIs and fixture error classes. No introduced P1/P2 issues found within this review.
  • Key design decisions: Reusing the helper avoids duplicate parity logic and version branches. Changes are confined to tests and contributor documentation, adding no production execution overhead.
  • Implementation sketch: Add the assertion mode, validate its directive syntax, dispatch to the helper, accept it as a codegen sentinel, and cover parser and runner behavior.
  • Behavioral changes worth calling out: Malformed directives fail with source locations. Operator checks inspect the initial plan under AQE. Query-context assertions remain in Scala. The touched existing files on latest release branch branch-1.1 match the PR base, so release-relative changes are the same intended framework additions.
  • Suggested improvements: None meeting the P1/P2 reporting threshold.

Reviewed both commits and all five changed files at 1aa87d1a7c97e41bff54c9173da3b4fa2015c654 against 93ce5c3471dbd89aa5cd5f6f9e26df27adab6a8a. Applied review-comet-pr; no sibling subsystem skill applies. Confirmed non-draft status. Existing reviews, issue comments, inline comments and review threads were empty.

Exact-head CI: 23 checks succeeded, 14 were skipped, and the Spark 4.1 expressions job remained in progress. No failures were reported. Spark SQL, Iceberg and macOS jobs were skipped.

Validation: All 16 Spark-independent parser tests passed in a disposable Scala 2.13.17 harness. Base/head parser outputs matched for all 510 existing SQL fixtures. git diff --check passed. The unchanged fixture-gating parser test and end-to-end Spark/Comet runner tests were not executed locally. This checkout has no built native library, and a full Maven/native build was not performed. The working tree remains unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request test Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: add a Comet SQL test error mode that checks native execution and error-class parity

2 participants