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.

Sign up for GitHub

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

Jump to bottom

External Error prefix is repeated multiple times #14080

Open
Tracked by #14008
gz opened this issue Jan 10, 2025 · 7 comments · May be fixed by #14371
Open
Tracked by #14008

External Error prefix is repeated multiple times #14080

gz opened this issue Jan 10, 2025 · 7 comments · May be fixed by #14371
Assignees
@getChan
Labels
bug Something isn't working

Comments

@gz
Copy link

gz commented Jan 10, 2025 •
edited
Loading

Describe the bug

I'm getting errors in the form of DataFusionError::External(External(External(External(External(External(xyz)))) when throwing them from https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.ExecutionPlan.html#tymethod.execute

To Reproduce

Implement a TableProvider that throws an error and call it

    fn execute(
        &self,
        partition: usize,
        _context: Arc<TaskContext>,
    ) -> datafusion::common::Result<SendableRecordBatchStream> {
            return Err(DataFusionError::Execution(
                format!("error"),
            ));
    }

Expected behavior

I'd expect the External(External(External(External(External flattened to just one External

Additional context
@gz gz added the bug Something isn't working label Jan 10, 2025
@gz gz mentioned this issue Jan 10, 2025
Closed
gz added a commit to feldera/feldera that referenced this issue Jan 10, 2025
@gz
Fix display of datafusion errors.
27611db 
Until apache/datafusion#14080 is fixed, we'll
remove the `DataFusionError::External` strings from the error message
displayed to clients.

Signed-off-by: Gerd Zellweger <[email protected]>
@gz gz mentioned this issue Jan 10, 2025
Merged
github-merge-queue bot pushed a commit to feldera/feldera that referenced this issue Jan 11, 2025
@gz
Fix display of datafusion errors.
377409f 
Until apache/datafusion#14080 is fixed, we'll
remove the `DataFusionError::External` strings from the error message
displayed to clients.

Signed-off-by: Gerd Zellweger <[email protected]>
@Omega359
Copy link
Contributor

I was seeing this with the sqlite tests, thx for creating a ticket for it.

@alamb
Copy link
Contributor

alamb commented Jan 13, 2025

I agree this would be nice to fix.

We can potentially do the unwrapping in this From impl:

datafusion/datafusion/common/src/error.rs

Lines 282 to 286 in 63b94c8

impl From<GenericError> for DataFusionError {
fn from(err: GenericError) -> Self {
DataFusionError::External(err)
}
}

But not sure if there are other places too

@alamb alamb mentioned this issue Jan 13, 2025
Open
32 tasks
@getChan
Copy link
Contributor

getChan commented Jan 29, 2025

Take

@Omega359
Copy link
Contributor

Note that when this is fixed the sqlite test files will need to be regenerated. See #14290 for a PR to do that.

@getChan getChan linked a pull request Jan 30, 2025 that will close this issue
Open
@getChan
Copy link
Contributor

getChan commented Jan 31, 2025 •
edited
Loading

@Omega359 @gz
Can you provide me with code that can reproduce a bug? I want to use it for testing.

@alamb
Copy link
Contributor

alamb commented Jan 31, 2025

@Omega359 @gz Can you provide me with code that can reproduce a bug? I want to use it for testing.

I think the tests @Omega359 is referring to are the ones in

The challenge is updating the expected results

@Omega359
Copy link
Contributor

https://github.com/apache/datafusion-testing/blob/5b424aefd7f6bf198220c37f59d39dbb25b47695/data/sqlite/random/groupby/slt_good_12.slt#L19568 is an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@getChan getChan

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix: Avoid recursive external error wrapping getChan/datafusion
4 participants
@gz @Omega359 @alamb @getChan