apache / apache/datafusion

Some aggregation spilling unit tests don't cover spilling to disk logic

Open
#12,059 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Describe the bug

When I debug the failed tests in #11943 , I finally found that such tests not conver the `spilling to disk logic` in `main` branch actually(if convered, they may fail as #11943...)

And in #11943, the memory usage in test are changed, and trigger the `spilling to disk`, and lead to test failures.

### To Reproduce

- add a `panic` at the beginning of `update_merged_stream` function in `row_hash.rs`.
- run `aggregate_grouping_sets_source_not_yielding_with_spill` and `aggregate_grouping_sets_with_yielding_with_spill` tests.
- you will found them passed.

### Expected behavior

They should go the disk spilling path and pass.

### Additional context

_No response_

Contributor guide

Open the contributing guide

Research direction

Start with row_hash.rs and the update_merged_stream function, then run aggregate_grouping_sets_source_not_yielding_with_spill and aggregate_grouping_sets_with_yielding_with_spill. Verify that the tests actually exercise the disk-spilling path rather than passing without entering it, and that they still pass when that path is used.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data-engineering
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.