clintropolis commented on a change in pull request #9638:
URL: https://github.com/apache/druid/pull/9638#discussion_r472489808
##########
File path:
processing/src/main/java/org/apache/druid/segment/column/RowSignature.java
##########
@@ -237,21 +235,14 @@ public Builder addDimensions(final List<DimensionSpec>
dimensions)
public Builder addAggregators(final List<AggregatorFactory> aggregators)
{
for (final AggregatorFactory aggregator : aggregators) {
- final ValueType type = GuavaUtils.getEnumIfPresent(
- ValueType.class,
- StringUtils.toUpperCase(aggregator.getTypeName())
- );
-
- // Use null instead of COMPLEX for nonnumeric types, since in that
case, the type depends on whether or not
- // the aggregator is finalized, and we don't know (a) if it will be
finalized, or even (b) what the type would
- // be if it were finalized. So null (i.e. unknown) is the proper thing
to do.
- //
- // Another note: technically, we don't know what the finalized type
will be even if the type here is numeric,
- // but we're assuming that it doesn't change upon finalization. All
builtin aggregators work this way.
+ final ValueType type = aggregator.getType();
- if (type != null && type.isNumeric()) {
+ if (type.equals(aggregator.getFinalizedType())) {
add(aggregator.getName(), type);
} else {
+ // Use null if the type depends on whether or not the aggregator is
finalized, since
Review comment:
I agree that this is a nice thing to do in the future so that we can
have complete/accurate `RowSignature` everywhere, as I alluded to in the PR
description, but it is a bit much for this PR and not especially needed at this
point.
----------------------------------------------------------------
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]
