cool9850311 opened a new pull request, #26485:
URL: https://github.com/apache/datafusion/pull/26485
## Which issue does this PR close?
- Closes #26470.
## Rationale for this change
`bit_xor` over a sliding window frame returns `0` instead of `NULL` once
every non-NULL value has left the frame:
```sql
SELECT x,
bit_xor(x) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND CURRENT ROW) AS
sliding,
bit_xor(x) OVER (PARTITION BY i) AS
whole
FROM (VALUES (1, 5), (2, NULL), (3, 6)) t(i, x)
ORDER BY i;
+------+---------+-------+
| x | sliding | whole |
+------+---------+-------+
| 5 | 5 | 5 |
| NULL | 0 | NULL | <-- sliding should be NULL, as `whole` is
| 6 | 6 | 6 |
+------+---------+-------+
```
An aggregate with no non-NULL input returns NULL, and `bit_xor` is
documented as computing "the bitwise exclusive OR of all non-null input
values". The `0` is the XOR identity leaking out of the accumulator's internal
state. DuckDB returns NULL here.
The cause is that `BitXorAccumulator` keeps a single `Option<T::Native>`
whose `None` doubles as "has not seen a non-NULL value yet". `retract_batch`
can undo the value, since XOR is its own inverse, but it can never move the
state back from `Some` to `None`. So after the last non-NULL value leaves the
frame the accumulator is left holding `Some(0)`. Over a whole partition the
same aggregate is correct, because that path never retracts.
## What changes are included in this PR?
`BitXorAccumulator` now tracks the number of non-NULL values currently
accumulated and returns NULL when it is zero, the same way `avg` does (#22139).
`retract_batch` no longer delegates to `update_batch`, since the count has to
move the other way.
A genuine zero is preserved: `bit_xor` of `[5, 5]` still returns `0`.
`state_fields` is deliberately unchanged, so the state schema and its
serialization are untouched. The count never has to be serialized: `state()`
already yields NULL when there is no non-NULL input, and `merge_batch` goes
through `update_batch`, which adds nothing for a NULL state and one for a
non-NULL state.
`bit_and`, `bit_or` and `bit_xor(DISTINCT ..)` do not implement
`retract_batch`, so they cannot reach this state machine and are unaffected.
## What is the testing strategy for this PR?
Two layers:
- Four `sqllogictest` cases in
`datafusion/sqllogictest/test_files/window.slt`, placed next to the analogous
`avg` regression tests: the issue's own reproduction (the sliding frame
compared against the whole partition), a narrow frame that drains to empty on
every row, all-NULL input, and a case pinning down that a legitimate `0`
survives and that NULL follows it.
- Two accumulator unit tests in
`datafusion/functions-aggregate/src/bit_and_or_xor.rs`: the retract-to-empty
state machine, including that retracting NULLs does not move the count and that
the accumulator recovers when a non-NULL value re-enters, and the
legitimate-zero case.
I checked the tests fail without the fix: reverting the change to the
accumulator and keeping only the new tests turns three of the four slt blocks
red, each reporting `0` where NULL is expected. The all-NULL block passes
either way, so it is a guard rather than a reproduction.
I also verified the partial/final merge path directly, since `GROUP BY` uses
`PrimitiveGroupsAccumulator` and does not exercise this accumulator's
`state()`/`merge_batch()`: a partial over `[5, 5]` emits `0` as its state and
the final phase returns `0`, while merging only NULL states returns NULL.
The full test suite passes (`cargo test --profile ci --workspace --lib
--tests --bins` with the extended feature set), as do `cargo fmt --all` and
`ci/scripts/rust_clippy.sh`.
## Are there any user-facing changes?
Yes, a bug fix: `bit_xor` over a sliding window frame that contains no
non-NULL values now returns NULL instead of `0`. No public API changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
