[I] feat: typed `DataFusionException` hierarchy across the JNI boundary [datafusion-java]

["LantaoJin (via GitHub)" <gi...@apache.org>]

LantaoJin opened a new issue, #79:
URL: https://github.com/apache/datafusion-java/issues/79

   ### Is your feature request related to a problem or challenge?
   
   Every error from the native side currently surfaces as a single `java.lang.RuntimeException`. `try_unwrap_or_throw` in `native/src/errors.rs` calls `env.throw_new("java/lang/RuntimeException", message)` for every `Err(...)` it sees, so a `DataFusionError::Plan` and a `DataFusionError::ResourcesExhausted` and a `DataFusionError::IoError` are indistinguishable on the Java side. Callers that want to retry on transient resource pressure but not on a malformed query plan have only the message string to discriminate on — fragile and brittle to upstream wording changes.
   
   A representative pain point: a service wrapper around `df.collect()` wants to bubble plan errors up as 4xx and resource-exhausted as 5xx-with-retry, and today that has to be done with `e.getMessage().contains("ResourcesExhausted")`-style sniffing.
   
   ### Describe the solution you'd like
   
   A small typed hierarchy under `org.apache.datafusion`, plus a routing pass in `try_unwrap_or_throw` that picks the right subclass based on the originating `DataFusionError` variant.
   
   ```
   DataFusionException        extends RuntimeException
   ├── PlanException          // DataFusionError::Plan, ::SQL, ::SchemaError
   ├── ExecutionException     // DataFusionError::Execution, ::ExecutionJoin
   ├── ResourcesExhaustedException
   ├── IoException            // DataFusionError::IoError, ::ObjectStore, ::ArrowError, ::ParquetError, ::AvroError
   ├── NotImplementedException
   └── ConfigurationException
   ```
   
   Mapping (one Java class per "error category that matters to a caller"; multiple Rust variants fold into one Java class when they're the same kind of problem from the caller's standpoint):
   
   | `DataFusionError` variant | Java exception |
   |---|---|
   | `Plan`, `SQL`, `SchemaError` | `PlanException` |
   | `Execution`, `ExecutionJoin`, `External`, `Ffi` | `ExecutionException` |
   | `ResourcesExhausted` | `ResourcesExhaustedException` |
   | `IoError`, `ObjectStore`, `ArrowError`, `ParquetError`, `AvroError` | `IoException` |
   | `NotImplemented` | `NotImplementedException` |
   | `Configuration` | `ConfigurationException` |
   | `Internal`, `Substrait`, `Diagnostic`, `Collection`, `Shared`, `Context` | `DataFusionException` (parent; no good caller-facing category) |
   | Rust panic (caught by `try_unwrap_or_throw`) | `DataFusionException` (parent) with a `panic: ` prefix |
   
   Every subclass is a `RuntimeException`, so existing callers that `catch (RuntimeException)` keep working unchanged. The mapping is deliberately conservative — the hierarchy is shallow, and "I don't know what to do with this" stays as the parent class rather than inventing a class per Rust variant.
   
   Each subclass has the standard pair of constructors:
   
   ```java
   public PlanException(String message) { super(message); }
   public PlanException(String message, Throwable cause) { super(message, cause); }
   ```
   
   The `(String, Throwable)` constructor is the seam for a follow-up that propagates Java stack traces from upcalls (issue **#55**) — when a Java-implemented UDF or table-provider scan throws, the resulting `DataFusionException` can carry the original `Throwable` as its `cause` instead of stringifying it. This PR doesn't actually wire that path — it just makes sure the constructor exists so #55 doesn't have to change the public API surface.
   
   ### Describe alternatives you've considered
   
   **Mirror every `DataFusionError` variant 1:1.** Maximally faithful, but exposes implementation details (`SchemaError`, `Diagnostic`, `Shared`, `Collection`, `Context`) that aren't useful to a Java caller and that upstream itself may renumber. The proposed mapping is a stable contract; the underlying variant set is not.
   
   ### Additional context
   
   This issue lands before #55 . Reasons:
   1. #55 needs a Java parent class to carry the original `Throwable` as `cause`. This PR creates it. Otherwise #55 has to invent its own and renames it later.
   2. This issue is mechanical (one match arm per variant); #55 has a real design decision (stringify the Java stack trace, or keep the `Throwable` alive across the JNI via a registry handle so it can become `cause`). That decision is easier with the parent already merged.


-- 
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.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: [I] feat: typed `DataFusionException` hierarchy across the JNI boundary [datafusion-java]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove closed issue #79: feat: typed `DataFusionException` hierarchy across the JNI boundary
URL: https://github.com/apache/datafusion-java/issues/79


-- 
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