Skip to content

Conversation

@andygrove
Copy link
Contributor

@andygrove andygrove commented Sep 21, 2022

We haven't been running cargo test in CI and some regressions crept in.

The goal of this PR is just to get us back to a stable point and enable Rust tests in CI.

Changes in this PR:

  • Run cargo test in CI
  • Revert to an earlier version of the EliminateAggDistinct rule and ignore 2 failing tests

Proof that this works (from the logs):

running 9 tests
test sql::optimizer::eliminate_agg_distinct::tests::test_count_and_distinct_no_group_by ... ok
test sql::optimizer::eliminate_agg_distinct::tests::test_count_and_distinct_no_group_by_with_alias ... ok
test sql::optimizer::eliminate_agg_distinct::tests::test_multiple_distinct ... ok
test sql::optimizer::eliminate_agg_distinct::tests::test_count_and_distinct_group_by ... ok
test sql::optimizer::eliminate_agg_distinct::tests::test_multiple_distinct_with_aliases ... ok
test sql::optimizer::eliminate_agg_distinct::tests::test_single_distinct_group_by ... ignored
test sql::optimizer::eliminate_agg_distinct::tests::test_single_distinct_group_by_with_alias ... ignored
test sql::optimizer::eliminate_agg_distinct::tests::test_single_distinct_no_group_by ... ok
test sql::optimizer::eliminate_agg_distinct::tests::test_single_distinct_no_group_by_with_alias ... ok

test result: ok. 7 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.01s

@andygrove andygrove force-pushed the fix-distinct-optimization branch from 940074d to 2982803 Compare September 21, 2022 17:45
@andygrove andygrove changed the title [DF] WIP: Fix alias issues in eliminate_agg_distinct rule [DF] Fix regressions in EliminateAggDistinct Sep 21, 2022
@andygrove andygrove changed the title [DF] Fix regressions in EliminateAggDistinct [DF] Fix regressions in EliminateAggDistinct, run cargo test in CI Sep 21, 2022
@andygrove andygrove marked this pull request as ready for review September 21, 2022 18:03
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2022

Codecov Report

❗ No coverage uploaded for pull request base (datafusion-sql-planner@528108c). Click here to learn what that means.
The diff coverage is n/a.

@@                    Coverage Diff                    @@
##             datafusion-sql-planner     #782   +/-   ##
=========================================================
  Coverage                          ?   75.28%           
=========================================================
  Files                             ?       73           
  Lines                             ?     3678           
  Branches                          ?      765           
=========================================================
  Hits                              ?     2769           
  Misses                            ?      773           
  Partials                          ?      136           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ayushdg ayushdg merged commit 0570b4b into dask-contrib:datafusion-sql-planner Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants