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]

Reply via email to