gianm commented on a change in pull request #9638:
URL: https://github.com/apache/druid/pull/9638#discussion_r417755990
##########
File path:
processing/src/main/java/org/apache/druid/query/aggregation/AggregatorFactory.java
##########
@@ -214,19 +216,42 @@ public AggregatorFactory
getMergingFactory(AggregatorFactory other) throws Aggre
* {@link #deserialize} and the type accepted by {@link #combine}. However,
it is *not* necessarily the same type
* returned by {@link #finalizeComputation}.
*
- * If the type is complex (i.e. not a simple, numeric {@link
org.apache.druid.segment.column.ValueType}) then there
+ * If the type is complex (i.e. not a simple, numeric {@link ValueType})
then there
* must be a corresponding {@link
org.apache.druid.segment.serde.ComplexMetricSerde} which was registered with
* {@link org.apache.druid.segment.serde.ComplexMetrics#registerSerde} using
this type name.
*
- * If you need a ValueType enum corresponding to this aggregator, a good way
to do that is:
- *
- * <pre>
- * Optional.ofNullable(GuavaUtils.getEnumIfPresent(ValueType.class,
aggregator.getTypeName()))
- * .orElse(ValueType.COMPLEX);
- * </pre>
+ * If you need a ValueType enum corresponding to this aggregator, use {@link
#getTypeName} instead.
*/
public abstract String getTypeName();
+ /**
+ * Get the type name for the 'finalized' type for this aggregator, i.e. the
type of the value returned by
+ * {@link #finalizeComputation}. This may be the same as or different than
the types expected in {@link #deserialize}
+ * and {@link #combine}.
+ *
+ * If you need a ValueType enum corresponding to this aggregator, use {@link
#getFinalizedType} instead.
+ */
+ public String getFinalizedTypeName()
+ {
+ return getTypeName();
Review comment:
Fwiw, I'm in favor of requiring aggregators to explicitly implement
these functions — especially if 90% of people would be fine with the default!
That means it would be really easy for the 10% that need to override it to
forget to do so. In general it's good to make it easy to do things right, not
easy to forget important stuff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
