Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow use of ? to return DataFusionErrors in methods that returnArrowErrors #1642

Closed
alamb opened this issue Jan 22, 2022 · 0 comments · Fixed by #1643
Closed

Allow use of ? to return DataFusionErrors in methods that returnArrowErrors #1642

alamb opened this issue Jan 22, 2022 · 0 comments · Fixed by #1643
Assignees
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Jan 22, 2022

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
Improve DataFusion error conversions

Now that @matthewmturner made a DataFusionError::External #1541 , the story for converting back and forth to ArrowErrors is much better ❤️

However, I really want to be able to use ? in functions that return an ArrowError or a DataFusionError]k

Specifically, I want the following two cases to work (

    /// Model what happens when implementing SendableRecrordBatchStream:
    /// DataFusion code needs to return an ArrowError
    fn return_arrow_error() -> arrow::error::Result<()> {
        // Expect the '?' to work
        let _foo = Err(DataFusionError::Internal("foo".to_string()))?;
        Ok(())
    }


    /// Model what happens when using arrow kernels in DataFusion
    /// code: need to turn an ArrowError into a DataFusionError
    fn return_datafusion_error() -> crate::error::Result<()> {
        // Expect the '?' to work
        let _bar = Err(ArrowError::SchemaError("bar".to_string()))?;
        Ok(())
    }

Unfortunately, the first example (converting a DataFusionError to an ArrowError) produces a compilation error:

error[E0277]: `?` couldn't convert the error to `arrow::error::ArrowError`
   --> datafusion/src/error.rs:184:69
    |
182 |     fn return_arrow_error() -> arrow::error::Result<()> {
    |                                ------------------------ expected `arrow::error::ArrowError` because of this
183 |         // Expect the '?' to work
184 |         let _foo = Err(DataFusionError::Internal("foo".to_string()))?;
    |                                                                     ^ the trait `From<error::DataFusionError>` is not implemented for `arrow::error::ArrowError`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following implementations were found:
              <arrow::error::ArrowError as From<FromUtf8Error>>
              <arrow::error::ArrowError as From<IntoInnerError<W>>>
              <arrow::error::ArrowError as From<csv::error::Error>>
              <arrow::error::ArrowError as From<serde_json::error::Error>>
              <arrow::error::ArrowError as From<std::io::Error>>
    = note: required because of the requirements on the impl of `FromResidual<std::result::Result<Infallible, error::DataFusionError>>` for `std::result::Result<(), arrow::error::ArrowError>`

Describe the solution you'd like
Add the appropriate conversion method

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant