sreemanamala commented on code in PR #16402:
URL: https://github.com/apache/druid/pull/16402#discussion_r1596845657
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/GroupingSqlAggregator.java:
##########
@@ -90,7 +91,17 @@ public Aggregation toDruidAggregation(
}
}
}
- AggregatorFactory factory = new GroupingAggregatorFactory(name, arguments);
+ AggregatorFactory factory;
+ try {
+ factory = new GroupingAggregatorFactory(name, arguments);
+ }
+ catch (Exception e) {
Review Comment:
@rohangarg Thanks for the comment.
I have updated the exception message to remove not supported. I think the
exception here would only be caused by the Precondition checks and that could
be easily understood by the exception message
Made the exception look similar to the one we have in
`EarliestLatestBySqlAggregator` and return null.
anyways, In the end `DruidQuery` would throw `CannotBuildQueryException`. So
it made sense to not throw in the aggregator class.
Does this look good to you now?
--
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]
