imply-cheddar commented on a change in pull request #12251:
URL: https://github.com/apache/druid/pull/12251#discussion_r803300900
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/AvgSqlAggregator.java
##########
@@ -115,9 +115,13 @@ public Aggregation toDruidAggregation(
project,
Iterables.getOnlyElement(aggregateCall.getArgList())
);
- String vc = virtualColumnRegistry.getVirtualColumnByExpression(arg,
resolutionArg.getType());
- fieldName = vc != null ? vc : null;
- expression = vc != null ? null : arg.getExpression();
+ if (arg.getType() == DruidExpression.NodeType.LITERAL) {
Review comment:
This doesn't really feel like somethign that each implementation should
have to do to me... Could we not like pass in the virtualColumnRegistry to the
DruidExpression and ask it to tell us the expression to use? Where, like, in
the literal case, it would give us an expression that's just a literal and in
the non-literal case it would register the thing and then return the name to
use for it?
--
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]