apache / apache/datafusion-comet
Re-enable the two ignored CometAggregateSuite metric tests after the DataFusion 55 peak_mem_used change
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
### Describe the bug
Two tests in `CometAggregateSuite` are disabled on the DataFusion 55 branch with only a prose TODO and no tracking issue:
```scala
// TODO: To be addressed after DF 55 migration
ignore("grouped aggregate metrics are forwarded without fabricating global metrics") {
...
// TODO: To be addressed after DF 55 migration
ignore("range sampling does not report grouped aggregate metrics") {
```
They assert on `spill_count`, `spilled_bytes`, `spilled_rows` and `peak_mem_used`. `peak_mem_used` accounting changed in apache/datafusion#22710, which is what broke them.
### Steps to reproduce
Un-`ignore` either test on the `df55` branch.
### Expected behavior
Either the assertions are adapted to DataFusion 55's accounting, or the metric is reported the way the tests expect. Either way the skips should point at this issue rather than a prose TODO, which is easy to lose once the branch merges.
### Additional context
@sunchao offered to take this as a follow-up after #5262 merges. Related: the `CometTPCHQuerySuite` off-heap bump tracked separately.
Contributor guide
Research direction
Start with the two ignored tests in CometAggregateSuite on the df55 branch and review the DataFusion 55 peak_mem_used change referenced in apache/datafusion#22710. Run the suite with each test enabled, then adapt the assertions or metric reporting so spill_count, spilled_bytes, spilled_rows, and peak_mem_used match the intended behavior; replace the prose TODO with this issue reference.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, scala, spark
- Domain
- data-engineering, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100