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

Improve optimizer performance by not using Errors in the happy path #7552

Closed
2 tasks done
alamb opened this issue Sep 13, 2023 · 5 comments
Closed
2 tasks done

Improve optimizer performance by not using Errors in the happy path #7552

alamb opened this issue Sep 13, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Sep 13, 2023

Is your feature request related to a problem or challenge?

We have had a few issues / reports where the creation of DataFusionError during query planning has contributed significantly to the overall planning time. Specifically, #5309 and more recently #7522

At the core of the issue is that creating a DataFusionError is a fairly slow operation (as it requires allocating a new String to hold the error message, and sometimes, as in #5309, the construction of the message itself is significant.

Describe the solution you'd like

I would like to update the DataFusion codebase so it does not use Errors on the "happy path"

Tasks:

Describe alternatives you've considered

We could potentially make it faster / cheaper to create DataFusion errors

Additional context

No response

@alamb
Copy link
Contributor Author

alamb commented Sep 13, 2023

Here is an example of using Errors in the happy path (aka ignoring an error rather than fixing the underlying issue)

https://github.com/apache/arrow-datafusion/blob/c47ae0d13dbc9474897609f3dea4f80a93cf4de0/datafusion/physical-plan/src/filter.rs#L206-L209

@comphead
Copy link
Contributor

I wanna check that if no other volunteers

@comphead
Copy link
Contributor

comphead commented Jul 9, 2024

I would also close this as a lot of job was done for planner to improve the performance

@comphead
Copy link
Contributor

comphead commented Jul 9, 2024

@alamb I'm closing this, please feel free to reopen if needed

@comphead comphead closed this as completed Jul 9, 2024
@alamb
Copy link
Contributor Author

alamb commented Jul 9, 2024

I agree -- thanks for cleaning these up @comphead -- we can file new tickets if/when we have actionable changes to make

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

No branches or pull requests

2 participants