Skip to content

Move All Physical Optimizer Tests to core/tests and Remove functions-aggregate Dependency #14298

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 1 commit into from
Jan 26, 2025

Conversation

berkaysynnada
Copy link
Contributor

@berkaysynnada berkaysynnada commented Jan 25, 2025

Which issue does this PR close?

Rationale for this change

Rather than keeping physical optimizer tests separate—which may lead to the duplication of utils and tests—we prefer consolidating them into a single file. This file should reside in the core module, as many optimizer tests depend on core tools. These changes also remove the functions-aggregate dependency on the physical-optimizer crate, which was previously required only for tests.

What changes are included in this PR?

just test migration

Are these changes tested?

Are there any user-facing changes?

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Awesome -- thank you so much @berkaysynnada

As this is just moving code around (not changing behavior) I will merge this now to avoid it accumulating conflicts

@@ -1593,7 +1593,6 @@ dependencies = [
"datafusion-execution",
"datafusion-expr",
"datafusion-expr-common",
"datafusion-functions-aggregate",
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

mod limited_distinct_aggregation;
mod replace_with_order_preserving_variants;
mod sanity_checker;

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@@ -41,7 +41,6 @@ datafusion-common = { workspace = true, default-features = true }
datafusion-execution = { workspace = true }
datafusion-expr = { workspace = true }
datafusion-expr-common = { workspace = true, default-features = true }
datafusion-functions-aggregate = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@alamb alamb merged commit d460bb4 into apache:main Jan 26, 2025
29 checks passed
@alamb
Copy link
Contributor

alamb commented Jan 26, 2025

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate optimizer Optimizer rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependency on physical-optimizer on functions-aggregates
2 participants