Skip to content

Temporarily fix bug in dynamic top-k optimization #16465

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

Merged
merged 2 commits into from
Jun 19, 2025

Conversation

AdamGS
Copy link
Contributor

@AdamGS AdamGS commented Jun 19, 2025

Which issue does this PR close?

See #16452.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added core Core DataFusion crate physical-plan Changes to the physical-plan crate labels Jun 19, 2025
@AdamGS AdamGS force-pushed the adamg/investigate-16452 branch from 5becc5b to f915110 Compare June 19, 2025 21:15
@AdamGS AdamGS changed the title Fix bug in dynamic topk optimization Temporarily fix bug in dynamic top-k optimization Jun 19, 2025
Copy link
Contributor

@adriangb adriangb left a comment

Choose a reason for hiding this comment

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

Some minor nits


let random_seed = 1u64; // Use a fixed seed to ensure consistent behavior

println!("Creating test generator with same config as original runner...");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan on merging the test? If so maybe let's clean this up?

@adriangb adriangb merged commit 5ca4ff0 into apache:main Jun 19, 2025
27 checks passed
@adriangb
Copy link
Contributor

Sad to have to do this but I agree keeping CI green is the top priority cc @Dandandan

@alamb
Copy link
Contributor

alamb commented Jun 20, 2025

Thanks @adriangb and @blaginin and @AdamGS for sorting this out

alamb added a commit to alamb/datafusion that referenced this pull request Jun 22, 2025
@alamb
Copy link
Contributor

alamb commented Jun 22, 2025

We think we have fixed the real cause in

Here is a PR to restore the related code in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate physical-plan Changes to the physical-plan crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants