consolidate physical_optimizer tests into core/tests/physical_optimizer #14244
Conversation
| @@ -0,0 +1,306 @@ | |||
|
|
|||
| use datafusion_common::config::ConfigOptions; | |||
There was a problem hiding this comment.
Moved into its own file for consistency. If people like this pattern I can do it for the other tests too
| @@ -1236,3 +1239,828 @@ async fn test_not_replaced_with_partial_sort_for_bounded_input() -> Result<()> { | |||
| assert_optimized!(expected_input, expected_no_change, physical_plan, false); | |||
| Ok(()) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I moved all the tests for the EnforceSorting pass into the same module along side a bunch of existing tests. I think this will make evaluating the coverage of EnforceSorting easier to understand
| @@ -147,314 +147,4 @@ fn take_optimizable_value_from_statistics( | |||
| value.map(|val| (val, agg_expr.name().to_string())) | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
There was a problem hiding this comment.
I simply moved the tests around
| @@ -1,861 +0,0 @@ | |||
| // Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
This was consolidated with the exsiting enforce_sorting.rs tests (same for the others below)
|
If this is an acceptable change, as a follow on PR I will move the tests from
I was just trying to limit the size of this PR |
|
I shared my comments in #14243 |
Thanks @berkaysynnada -- I think your idea is great (see #14243 (comment)) I also think this PR is an incremental step towards your plan, so we could potentially:
Thanks again! |
|
Thanks @berkaysynnada |
Move tests
Which issue does this PR close?
EnforceDistributionintodatafusion-physical-optimizercrate #14190Part of - [EPIC] Extract remaining physical optimizer out of core #11502
Rationale for this change
Now that we have migrated the physical optimizer tests over, @berkaysynnada and @logan-keede made the modules smaller and easier to manage by putting the tests into separate integration test
Let's keep the rest of the tests following the same pattern and consolidate them with the existing files
What changes are included in this PR?
physical_optimizer_integration, which is consistent withoptimizer_integration,sql_integration, etcAre these changes tested?
Yes by CI. You can run them via
cargo test --test core_integrationAre there any user-facing changes?