Shoryamishra61 opened a new pull request, #25871:
URL: https://github.com/apache/datafusion/pull/25871
## Which issue does this PR close?
- Closes #25850.
## Rationale for this change
Issue #25850 reported that `TopKAggregation` dropped the `NULL` group under
single-key `ORDER BY ... LIMIT` queries (e.g. `SELECT DISTINCT x FROM t ORDER
BY x LIMIT 10`, `SELECT x FROM t GROUP BY x ORDER BY x DESC LIMIT 10`).
Root-cause analysis revealed that in DataFusion 53.1.0,
`GroupedTopKAggregateStream::intern` ignored NULL keys during aggregation (`if
has_nulls && vals.is_null(row_idx) { continue; }`), which omitted the NULL
group for group-by-only queries. This was addressed in PR #22571 (`e7a2e052c`,
released in 55.0.0) by tracking `null_group_seen` and calling
`append_null_group`.
However, `aggregates_topk.slt` lacked a dedicated regression test covering
this exact query pattern with NULL groups, negative integer values, and both
`ASC` and `DESC` orderings with default and explicit `NULLS FIRST` / `NULLS
LAST` specifications.
This PR adds an extensive regression test matrix to lock in coverage on
`main` and prevent future regressions.
## What changes are included in this PR?
- Adds regression tests to
`datafusion/sqllogictest/test_files/aggregates_topk.slt` verifying:
1. `DISTINCT ... ORDER BY x ASC LIMIT 10` (default `NULLS LAST`) retains
NULL.
2. `GROUP BY x ORDER BY x DESC LIMIT 10` (default `NULLS FIRST`) retains
NULL.
3. `DISTINCT ... ORDER BY x DESC LIMIT 1` returns `NULL`.
4. `DISTINCT ... ORDER BY x DESC LIMIT 2` returns `NULL` and `500`.
5. `DISTINCT ... ORDER BY x DESC NULLS LAST LIMIT 1` returns `500`.
6. `DISTINCT ... ORDER BY x DESC NULLS LAST LIMIT 4` places NULL last.
7. `DISTINCT ... ORDER BY x ASC NULLS FIRST LIMIT 1` returns `NULL`.
8. `DISTINCT ... ORDER BY x ASC NULLS FIRST LIMIT 2` returns `NULL` and
`-301`.
9. `DISTINCT ... ORDER BY x LIMIT 3` excludes NULL when truncated by limit.
10. Table with all NULL values (`CREATE TABLE t(x BIGINT) VALUES (NULL),
(NULL)`).
11. Verifies plan and result equivalence between
`optimizer.enable_topk_aggregation = true` and
`optimizer.enable_topk_aggregation = false`.
## What is the testing strategy for this PR?
Tested via `sqllogictests`:
`cargo test -p datafusion-sqllogictest --test sqllogictests --
aggregates_topk`
## Are there any user-facing changes?
No user-facing changes or breaking API changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
