kgyrtkirk commented on code in PR #14748:
URL: https://github.com/apache/druid/pull/14748#discussion_r1301280476
##########
processing/src/main/java/org/apache/druid/query/aggregation/NullableNumericAggregatorFactory.java:
##########
@@ -85,6 +85,12 @@ public final int getMaxIntermediateSizeWithNulls()
return getMaxIntermediateSize() + (NullHandling.replaceWithDefault() ? 0 :
Byte.BYTES);
}
+ protected boolean canHandleNulls()
+ {
+ return NullHandling.replaceWithDefault();
+ }
Review Comment:
Yes; actually - I wanted a way to declare that this is `NullAware` :D
Earlier I kinda wanted to take a similar path as you suggested - just alter
the combiner - but that ended up as this change eventually; to my knowledge:
* `CombiningSequence` creates the accumulator and dispatches it thru
`Sequence#accumulate`
* as per what I've seen/remember the *first* element is not run thru the
combiner as there is nothing to combine it too...
* there is only a
[combine](https://github.com/apache/druid/blob/194a9c9abc1b6edf98c2652687515200c6cbd3e8/processing/src/main/java/org/apache/druid/query/aggregation/CountAggregatorFactory.java#L85)
method which has 2 arguments
* or the combining factory...
actually its interesting to consider a corner case (I don't have a testcase
for this - but it was covered by existing cases):
* `CombiningSequence` is run on an inputstream which produces only 1 output
element - which is produced via the `AggregateCominer` from a number of input
* The `AggregateCombiner` for `CountAggregatorFactory` is
`LongSumAggregateCombiner` originally - so it may have returned null ..and thus
by giving a sole input value could skip the combine method call
I was considering at some point also to extend the `Accumulator` interface
so that the it could communicate the initial value
Will investigate your suggestion/other alternatives - but iirc when there
are 0 elements aggregated those methods are not called/work....that's why I
resorted to the current solution.
--
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]
