JingsongLi commented on code in PR #999:
URL: https://github.com/apache/paimon-rust/pull/999#discussion_r4164983889
##########
bindings/python/src/error.rs:
##########
@@ -30,3 +44,31 @@ pub fn to_py_err(err: paimon::Error) -> PyErr {
pub fn df_to_py_err(err: datafusion::error::DataFusionError) -> PyErr {
PyValueError::new_err(err.to_string())
Review Comment:
[P2] Preserve ForkSafetyError through the Python SQL entry point
The new fork classification also needs to cover `df_to_py_err()`:
`SQLContext.sql()` uses this mapper for both planning and collection, and
DataFusion preserves the Paimon error inside `DataFusionError::External`. An
inherited Jindo-backed SQL query therefore raises `ValueError` here, so callers
handling `ForkSafetyError` cannot recognize the failure. I reproduced this by
passing
`DataFusionError::External(Box::new(paimon::Error::ProcessForkUnsupported {
message: "fork is not supported".into() }))` to `df_to_py_err()`; the returned
exception was `ValueError: External error: fork is not supported`, and the
ForkSafetyError assertion failed. The two existing direct/wrapped `to_py_err()`
tests passed. Please inspect the DataFusion error chain for the fork-specific
cause before constructing the Python exception and add this conversion
regression test.
--
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]
