You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "viirya (via GitHub)" <gi...@apache.org> on 2026/09/29 23:06:01 UTC

[PR] refactor: separate JNI-free core logic and error classification at the native boundary [datafusion-comet]

viirya opened a new pull request, #6435:
URL: https://github.com/apache/datafusion-comet/pull/6435

   ## Which issue does this PR close?
   
   Part of #6434.
   
   ## Rationale for this change
   
   The native JNI entry points mix JNI argument conversion, the native logic, and raising the JVM
   exception on failure in one function body. So the native logic cannot be unit-tested or
   benchmarked without a JVM, even where it never calls back into the JVM, and the mapping from
   `CometError` / `SparkError` to the JVM exception can only be tested through a JVM.
   
   This is the first step: the error classification plus a few entry points.
   
   ## What changes are included in this PR?
   
   - `native/jni-bridge/src/errors.rs`: `throw_exception` is split into
     `NativeError::from_comet_error`, which classifies an error into the JVM exception it surfaces
     as without using JNI, and `throw_native_error`, which throws it. Every branch of the previous
     code maps to one `JvmException` variant (`New`, `Spark`, `Rethrow`), so exception classes and
     messages are unchanged. All entry points go through it via `try_unwrap_or_throw`, whose
     signature is unchanged.
     - `NativeStatus` gives the outcome a stable code, and `NativeError::to_payload` serializes the
       classification as a versioned JSON document (exception class, exact message, parsed Spark
       error, backtrace, and the Rust `source()` chain for diagnostics).
     - `catch_native` is the panic-catching boundary without JNI.
   - `native/core/src/lib.rs`: `isFeatureEnabled` and `isObjectStoreSchemeSupported` call the new
     `is_feature_enabled(&str)` and `is_object_store_scheme_supported(&str)`.
   - `native/core/src/execution/jni_api.rs`:
     - `decodeShuffleBlock` and `decodeShuffleBlockWithValidation` call
       `decode_shuffle_block(&[u8], &[i64], &[i64], Option<&[DataType]>)`, and
       `createRemoteShuffleDecoder` calls `RemoteShuffleDecoder::try_new(&[u8])`.
     - `prepare_output` now only pins the address arrays and calls `export_batch`, which `executePlan`
       also uses through it.
   - `sql_error_propagation.md` is updated for the new classification step (the previous snippet also
     referred to an old path).
   
   The plan entry points (`createPlan`, `executePlan`, `releasePlan`, `setShufflePartitionPusher`)
   depend on JVM callbacks and are not changed here.
   
   ## How are these changes tested?
   
   - New Rust unit tests without a JVM for each classification path: Spark errors directly and
     through DataFusion wrappers, file-read classification, typed classes through `Context` / `Shared`
     wrappers, `to_exception` classes, the message-based fallbacks, panic backtrace formatting, the
     payload shape, and `catch_native`.
   - A new JVM-backed Rust test that a Java throwable captured during an upcall is rethrown as the
     same exception.
   - New Rust unit tests for `decode_shuffle_block` (local and remote blocks, no output columns,
     mismatched types, truncated blocks, invalid schema), `is_feature_enabled` and
     `is_object_store_scheme_supported`.
   - Existing JVM-backed tests in `errors.rs` (panics, typed exceptions through DataFusion wrappers,
     `NumberFormatException`, ...) pass unchanged.
   - Ran `CometNativeShuffleSuite`, `CometCelebornShuffleReaderSuite`,
     `ParquetReadFromFakeHadoopFsSuite`, `SparkErrorConverterSuite`, `CometNativeReaderSuite`,
     `CometRegExpJvmSuite` and `CometExecSuite` locally.
   
   This pull request and its description were written by Isaac.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] refactor: separate JNI-free core logic and error classification at the native boundary [datafusion-comet]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya merged PR #6435:
URL: https://github.com/apache/datafusion-comet/pull/6435


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] refactor: separate JNI-free core logic and error classification at the native boundary [datafusion-comet]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #6435:
URL: https://github.com/apache/datafusion-comet/pull/6435#issuecomment-5924456115

   Thanks @sunchao 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org