Akshat-Jain commented on code in PR #16682:
URL: https://github.com/apache/druid/pull/16682#discussion_r1675629281
##########
processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactory.java:
##########
@@ -122,25 +118,43 @@ public BufferAggregator
factorizeBuffered(ColumnSelectorFactory metricFactory)
if (selector instanceof NilColumnValueSelector) {
return NoopBufferAggregator.instance();
}
- final Class classOfObject = selector.classOfObject();
- if (classOfObject.equals(Object.class) ||
HyperLogLogCollector.class.isAssignableFrom(classOfObject)) {
- return new HyperUniquesBufferAggregator(selector);
- }
-
- throw new IAE("Incompatible type for metric[%s], expected a HyperUnique,
got a %s", fieldName, classOfObject);
+ validateInputs(metricFactory.getColumnCapabilities(fieldName));
+ return new HyperUniquesBufferAggregator(selector);
}
@Override
public VectorAggregator factorizeVector(final VectorColumnSelectorFactory
selectorFactory)
{
- final ColumnCapabilities capabilities =
selectorFactory.getColumnCapabilities(fieldName);
- if (!Types.is(capabilities, ValueType.COMPLEX)) {
+ final ColumnCapabilities columnCapabilities =
selectorFactory.getColumnCapabilities(fieldName);
+ if (!Types.is(columnCapabilities, ValueType.COMPLEX)) {
return NoopVectorAggregator.instance();
} else {
+ validateInputs(columnCapabilities);
return new
HyperUniquesVectorAggregator(selectorFactory.makeObjectSelector(fieldName));
}
}
+ /**
+ * Validates whether the aggregator supports the input column type.
+ * Supported column types are complex types of hyperUnique,
preComputedHyperUnique, as well as UNKNOWN_COMPLEX.
+ * @param capabilities
+ */
+ private void validateInputs(@Nullable ColumnCapabilities capabilities)
+ {
+ if (capabilities != null) {
+ final ColumnType type = capabilities.toColumnType();
+ if (!(ColumnType.UNKNOWN_COMPLEX.equals(type) || TYPE.equals(type) ||
PRECOMPUTED_TYPE.equals(type))) {
Review Comment:
> but why allow everything in case capabilities == null ?
What should be the expectation when capabilities is null?
--
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]
