Skip to content

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

"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.

❤️

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