neilconway opened a new pull request, #23874:
URL: https://github.com/apache/datafusion/pull/23874

   ## Which issue does this PR close?
   
   - Closes #23872 
   
   ## Rationale for this change
   
   `SlidingMinAccumulator::update_batch` skipped NULL values. This meant that 
if all the non-NULL values in a window frame were retracted, the window frame 
would not be empty but the `MovingMin` data structure by the 
`SlidingMinAccumulator` would not contain any values. This resulted in 
incorrectly returning a stale non-NULL value for a sliding `min()` over a 
window frame consisting of only NULL values.
   
   Along the way, optimize the min and max sliding window accumulators to make 
them both more efficient and more symmetric with one another. In the original 
coding, `SlidingMaxAccumulator` included NULL values but 
`SlidingMinAccumulator` omitted them, in part because omitting NULLs made the 
original `retract_batch` implementation more expensive. This PR optimizes 
`retract_batch`, so we can now use the same scheme for both the min and max 
sliding accumulators:
   
   * Omit NULLs on `update_batch` (this improves on the prior behavior of `max`)
   * Efficiently account for NULLs in `retract_batch` (this improves on the 
prior behavior of `min`)
   * Ensure correct results for all-NULL window frames (this fixes the prior 
bug in `min`).
   
   ## What changes are included in this PR?
   
   * Fix bug in `min()` over all-NULL window frames
   * Optimize `SlidingMinAccumulator::retract_batch` (avoid materializing 
values just to count NULLs)
   * Optimize `SlidingMinAccumulator::update_batch` (omit NULLs), also 
improving symmetry with `min`
   
   ## Are these changes tested?
   
   Yes, new tests added.
   
   ## Are there any user-facing changes?
   
   No, aside from the bug fix.


-- 
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]

Reply via email to