gianm commented on a change in pull request #11818:
URL: https://github.com/apache/druid/pull/11818#discussion_r734176292
##########
File path:
processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java
##########
@@ -189,20 +192,31 @@ public String getName()
}
@Override
- public ColumnType getType()
+ public ColumnType getType(ColumnInspector signature)
{
- // computed by decorate
- return outputType;
+ if (outputType != null) {
+ // computed by decorate
+ return outputType;
+ }
+ final ExpressionType type = parsed.get().getOutputType(signature);
+ if (type == null) {
+ return null;
+ } else {
+ return ExpressionType.toColumnType(type);
+ }
}
@Override
public ExpressionPostAggregator decorate(final Map<String,
AggregatorFactory> aggregators)
{
+ final ColumnInspector aggInspector =
AggregatorUtil.inspectorForAggregatorFactoryMap(aggregators);
Review comment:
This doesn't look necessary… how about dropping it and also the new
function in AggregatorUtil?
##########
File path:
processing/src/main/java/org/apache/druid/segment/column/RowSignature.java
##########
@@ -277,7 +277,8 @@ public Builder addPostAggregators(final
List<PostAggregator> postAggregators)
);
// unlike aggregators, the type we see here is what we get, no further
finalization will occur
- add(name, postAggregator.getType());
+ // feed it the existing RowSignature for PostAggregator
implementations whose output is dependent on input types
Review comment:
Alternate comment:
```
// It's OK to call getType in the order that post-aggregators
appear, because post-aggregators are only
// allowed to refer to *earlier* post-aggregators (not later ones;
the order is meaningful).
```
##########
File path:
processing/src/main/java/org/apache/druid/query/aggregation/post/FieldAccessPostAggregator.java
##########
@@ -89,9 +91,16 @@ public String getName()
}
@Override
- public ColumnType getType()
+ public ColumnType getType(ColumnInspector signature)
{
- return type;
+ if (type != null) {
+ return type;
+ }
+ final ColumnCapabilities capabilities =
signature.getColumnCapabilities(fieldName);
Review comment:
Offtopic: it'd be nice to have `signature.getColumnType(fieldName)`.
--
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]
