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

Add DataFusionError -> ArrowError conversion #1643

Merged
merged 4 commits into from
Jan 22, 2022

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jan 22, 2022

Which issue does this PR close?

Resolves: #1642

Rationale for this change

Please see #1642

What changes are included in this PR?

  1. Add impl From<DataFusionError> for ArrowError
  2. Tests

Are there any user-facing changes?

Possible to use ? now

@github-actions github-actions bot added the datafusion Changes in the datafusion crate label Jan 22, 2022
@@ -74,6 +74,8 @@ pub enum DataFusionError {

impl DataFusionError {
/// Wraps this [DataFusionError] as an [arrow::error::ArrowError].
///
/// TODO this can be removed in favor if the conversion below
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #1645

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// TODO this can be removed in favor if the conversion below
/// TODO this can be removed in favor of the conversion below

Why not remove this now? If we want to keep it, then it would be better to add a deprecated attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry -- I removed it in #1645 (probably wasn't worth leaving a TODO) 🤔

@alamb alamb merged commit 15af24a into apache:master Jan 22, 2022
@alamb alamb deleted the alamb/error-cleanup branch January 22, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow use of ? to return DataFusionErrors in methods that returnArrowErrors
2 participants