Skip to content

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Sep 28, 2025

Which issue does this PR close?

Rationale for this change

I am trying to run some benchmarks with @pepijnve on
#17419 but the benchmark queries are
now failing.

In order to unblock testing, I would like to disable the panic'ing query in the benchmark code until we fix the underlying issue.

What changes are included in this PR?

Skip the failing test

Are these changes tested?

I tested this manually that the previously failing code

cargo bench --profile dev --bench sql_planner -- physical_plan_tpcds_all

Now runs to completion

Are there any user-facing changes?

No, this is a benchmarking change only

@github-actions github-actions bot added the core Core DataFusion crate label Sep 28, 2025
@alamb alamb changed the title Disable failing sql_planner benchmark query Temporarily disable failing sql_planner benchmark query Sep 28, 2025
@alamb alamb marked this pull request as ready for review September 28, 2025 12:20
@alamb
Copy link
Contributor Author

alamb commented Sep 28, 2025

This may not be necessary given @pepijnve is working on a real fix: #17801 (comment)

Copy link
Member

@xudong963 xudong963 left a comment

Choose a reason for hiding this comment

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

I give an approval here, in case we still want this.

@pepijnve
Copy link
Contributor

pepijnve commented Sep 29, 2025

Proposed fix is in #17813. It uncovers the same issue that the benchmark is hitting in one of the tpc-ds tests by removing a double logical plan optimisation and then fixes the problem by introducing a small refinement to the nullability reporting of case expressions.

@alamb
Copy link
Contributor Author

alamb commented Sep 29, 2025

Since I had some non trivial comments on

I'll also prepare a revert PR so we can back it out

Thanks for bearing with us @pepijnve

@alamb alamb added this pull request to the merge queue Sep 29, 2025
@alamb
Copy link
Contributor Author

alamb commented Sep 29, 2025

Thank you for the approval @xudong963

Merged via the queue into apache:main with commit 5cc0be5 Sep 29, 2025
29 checks passed
alamb added a commit to alamb/datafusion that referenced this pull request Sep 29, 2025
@alamb
Copy link
Contributor Author

alamb commented Sep 29, 2025

@alamb alamb deleted the alamb/disable_failing_test branch September 29, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants