Implement retract_batch for AvgAccumulator #4846
Conversation
Add avg to custom window frame tests
github-actions
bot
added
core
ozankabak
left a comment
ozankabak
left a comment
There was a problem hiding this comment.
LGTM, thank you for the contribution!
| let delta = sum_batch(values, &self.sum.get_datatype())?; | ||
| self.sum = self.sum.sub(&delta)?; |
There was a problem hiding this comment.
I like giving a name to the change (i.e. delta). I suggest doing the same in update_batch.
|
@mustafasrepo, PTAL |
|
Thank you for contribution @jonmmease and for the review @ozankabak ❤️ |
|
LGTM thanks @jonmmease. |
|
Can this be merged? Thanks! |
|
Sorry for the delay @jonmmease -- thanks again for the contributions! |
|
Benchmark runs are scheduled for baseline = 42f7dd5 and contender = 13fb42e. 13fb42e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
5 participants
Which issue does this PR close?
Closes #4845.
What changes are included in this PR?
Adds an implementation of
retract_batchtoAvgAccumulatorthat is identical to the implementation forSumAccumulator.Are these changes tested?
I added
AVGto a couple of existing tests that previously failed with"Retractable Accumulator hasn't been implemented..."error.Are there any user-facing changes?
AVGcan now be used in window expressions with custom frames.