Conversation
| .as_primitive() | ||
| .typed_value::<u64>() | ||
| .vortex_expect("count partial should not be null"); | ||
| *partial += val; |
There was a problem hiding this comment.
from nan_count i assume we don't really think this would happen in practice
There was a problem hiding this comment.
I think that's probably fair for a count to not exceed u64
| type Partial = u64; | ||
|
|
||
| fn id(&self) -> AggregateFnId { | ||
| AggregateFnId::new_ref("vortex.count") |
There was a problem hiding this comment.
are you sure (also @gatesn ) we want a global namespace of scalar-fns and aggregations?
Signed-off-by: blaginin <github@blaginin.me>
| fn try_accumulate( | ||
| &self, | ||
| _state: &mut Self::Partial, | ||
| _batch: &ArrayRef, | ||
| _ctx: &mut ExecutionCtx, | ||
| ) -> VortexResult<bool> { | ||
| Ok(false) | ||
| } |
There was a problem hiding this comment.
can you remove this. We want a different system to short cut decompression, similar to execute_parent
There was a problem hiding this comment.
i think then let's wait for that system to merge - any pr i should follow?
There was a problem hiding this comment.
Isn't this the equivalent of "reduce"? i.e. something that can operate without looking at data buffers.
So maybe we just need to rename this to fn reduce(...) and Joe will be happy!
Merging this PR will improve performance by 12.32%
Performance Changes
Comparing 
Footnotes
|
4 participants
Count from https://github.com/vortex-data/vortex/pull/7201/files