Skip to content
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.

Sign up for GitHub

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

Chore: Move aggregate statistics optimizer test from core to optimizer crate #12783

Merged
merged 7 commits into from
Oct 9, 2024
Merged

Chore: Move aggregate statistics optimizer test from core to optimizer crate #12783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
db10c10
move test from core to optimizer crate
jayzhan211 Oct 7, 2024
93275f9
cleanup
jayzhan211 Oct 7, 2024
63186e5
Merge branch 'main' of https://github.com/apache/datafusion into aggr…
jayzhan211 Oct 8, 2024
963981f
upd
jayzhan211 Oct 8, 2024
0a220ae
clippy
jayzhan211 Oct 8, 2024
55c4b85
Merge branch 'main' of https://github.com/apache/datafusion into aggr…
jayzhan211 Oct 9, 2024
b9d1c40
fmt
jayzhan211 Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions datafusion-cli/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

325 changes: 0 additions & 325 deletions datafusion/core/tests/physical_optimizer/aggregate_statistics.rs
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion datafusion/core/tests/physical_optimizer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
// specific language governing permissions and limitations
// under the License.

mod aggregate_statistics;
mod combine_partial_final_agg;
mod limit_pushdown;
mod limited_distinct_aggregation;
Expand Down
6 changes: 6 additions & 0 deletions datafusion/physical-optimizer/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ rust-version = { workspace = true }
workspace = true

[dependencies]
arrow = { workspace = true }
arrow-schema = { workspace = true }
datafusion-common = { workspace = true, default-features = true }
datafusion-execution = { workspace = true }
datafusion-expr-common = { workspace = true, default-features = true }
datafusion-physical-expr = { workspace = true }
datafusion-physical-plan = { workspace = true }
itertools = { workspace = true }

[dev-dependencies]
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.

Would it be possible to avoid this dependency? (I think we could remove it as a follow on PR, but I didn't quite understand why it was needed in the first place)

Looks like it is just count_udaf (which we could perhaps mock up) 🤔

Copy link
Contributor Author

@jayzhan211 jayzhan211 Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think we can't use function that hasn't implement accumulator here, for physical optimizer, I think we should use the real function

Error if I switch to stub

test aggregate_statistics::tests::test_count_partial_with_nulls_indirect_child ... FAILED
test aggregate_statistics::tests::test_count_partial_direct_child ... FAILED
test aggregate_statistics::tests::test_count_partial_with_nulls_direct_child ... FAILED
test aggregate_statistics::tests::test_count_partial_indirect_child ... FAILED
test aggregate_statistics::tests::test_count_inexact_stat ... FAILED
test aggregate_statistics::tests::test_count_with_nulls_inexact_stat ... FAILED

successes:

successes:

failures:

---- aggregate_statistics::tests::test_count_partial_with_nulls_indirect_child stdout ----
Error: NotImplemented("no impl for stub")

---- aggregate_statistics::tests::test_count_partial_direct_child stdout ----
Error: NotImplemented("no impl for stub")

---- aggregate_statistics::tests::test_count_partial_with_nulls_direct_child stdout ----
Error: NotImplemented("no impl for stub")

---- aggregate_statistics::tests::test_count_partial_indirect_child stdout ----
Error: NotImplemented("no impl for stub")

---- aggregate_statistics::tests::test_count_inexact_stat stdout ----
Error: NotImplemented("no impl for stub")

---- aggregate_statistics::tests::test_count_with_nulls_inexact_stat stdout ----
Error: NotImplemented("no impl for stub")

tokio = { workspace = true }
Loading