Pass return_type to AccumulatorFunctionImplementation for user defined aggregates
#3428
Conversation
github-actions
bot
added
core
alamb
changed the title
return_type to AccumulatorFunctionImplementation for user defined aggregates
| @@ -99,7 +100,7 @@ impl AggregateExpr for AggregateFunctionExpr { | |||
| } | |||
|
|
|||
| fn create_accumulator(&self) -> Result<Box<dyn Accumulator>> { | |||
| (self.fun.accumulator)() | |||
| (self.fun.accumulator)(&self.data_type) | |||
There was a problem hiding this comment.
The is the key change -- pass the return type when instantiating the accumulator
| pub type AccumulatorFunctionImplementation = | ||
| Arc<dyn Fn() -> Result<Box<dyn Accumulator>> + Send + Sync>; | ||
| Arc<dyn Fn(&DataType) -> Result<Box<dyn Accumulator>> + Send + Sync>; |
There was a problem hiding this comment.
Here is the signature change
|
This will likely conflict with #3425 logically |
Codecov Report
@@ Coverage Diff @@
## master #3428 +/- ##
==========================================
- Coverage 85.70% 85.68% -0.02%
==========================================
Files 298 298
Lines 54612 54644 +32
==========================================
+ Hits 46803 46824 +21
- Misses 7809 7820 +11
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Benchmark runs are scheduled for baseline = 1c8797d and contender = 447141c. 447141c is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3427
Rationale for this change
See #3427
What changes are included in this PR?
return_typeinAccumulatorFunctionImplementationAre there any user-facing changes?
Yes, if you use user defined aggregate functions -- one of the type signatures has changed