clintropolis commented on code in PR #14924:
URL: https://github.com/apache/druid/pull/14924#discussion_r1323478479
##########
processing/src/main/java/org/apache/druid/query/aggregation/any/NumericNilVectorAggregator.java:
##########
@@ -69,6 +69,11 @@ public static NumericNilVectorAggregator
longNilVectorAggregator()
@Nullable
private final Object returnValue;
+ public static NumericNilVectorAggregator of(Object returnValue)
Review Comment:
side note: I wonder if we should rename `NumericNilVectorAggregator` if we
are going to be using it for non-numbers, maybe just `NilVectorAggregator`. I
guess this doesn't need to be done in this PR, but it does seem kind of funny
exposing it for other purposes
##########
processing/src/main/java/org/apache/druid/query/aggregation/first/DoubleFirstAggregatorFactory.java:
##########
@@ -149,7 +150,7 @@ public VectorAggregator factorizeVector(
timeColumn);
return new DoubleFirstVectorAggregator(timeSelector, valueSelector);
}
- return NumericNilVectorAggregator.doubleNilVectorAggregator();
+ return NumericNilVectorAggregator.of(new SerializablePair<>(0L,
NullHandling.defaultDoubleValue()));
Review Comment:
this could be saved as a static variable instead of making a new one each
time since its not going to change over the lifetime of the service, same for
all the other impls
--
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]
