viirya opened a new pull request, #7185:
URL: https://github.com/apache/datafusion-comet/pull/7185
## Which issue does this PR close?
Closes #7184.
## Rationale for this change
`CometNativeUDF.classifyNativeError` chooses the exception for a failed
driver-side library validation by matching the native message text. Rewording a
message on the native side therefore changes the exception class a caller sees.
The native side already decides which kind of failure it hit, so it can raise
the right class directly.
## What changes are included in this PR?
- Three new `CometError` variants, each mapped in `to_exception`:
- `NativeUdfNotAllowed` raises `CometNativeUdfNotAllowedException`.
- `NativeUdfAbi` raises `CometNativeUdfAbiException`.
- `NoSuchElement` raises `java.util.NoSuchElementException`.
Like `ShuffleSizeLimit` and `IllegalState`, they are listed in
`typed_jvm_exception` and in `From<CometError> for DataFusionError`, so they
keep their class through DataFusion's Context/Shared/External wrappers.
- `validate_native_udf_library(path, name, enabled, allowed_paths) ->
CometResult<()>` holds the logic that was inline in the JNI export, and the
export is now a thin wrapper. Its error mapping:
- a `PolicyError` becomes `NativeUdfNotAllowed`
- `LoaderError::AbiMismatch` and `NoDiscovery` become `NativeUdfAbi`
- a name the library does not export becomes `NoSuchElement`
- `LoaderError::Open` and `Discovery` stay `Internal`
- `classifyNativeError` is removed. `CometNativeUDF.validateLibrary`
rethrows the three typed exceptions and wraps any other failure in
`CometNativeUdfLoadException` with that failure as its cause, as before.
Besides `Open` and `Discovery`, this covers a panic or a JNI string conversion
error, which cannot carry a Java cause when raised natively.
Exception classes and messages are unchanged. The messages keep the `Comet
Internal Error: ` prefix they had when every failure crossed JNI as
`CometError::Internal`. The executor-side checks in the planner are not touched.
## How are these changes tested?
- New unit tests in `comet_native_udf_bridge.rs` need no JVM and assert the
variant and exact message for each case: an exported UDF, an unknown name,
loading disabled, a library outside `allowedPaths`, a missing library, an ABI
mismatch, a missing discovery symbol, and a failed discovery.
- New tests in `jni-bridge/src/errors.rs` check the class and message for
each new variant, including when it is wrapped in a DataFusion `Context`.
- `CometNativeUdfSuite`, unchanged, pins each failure mode to its exception
class and passes.
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: [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]
