rohangarg commented on code in PR #16402:
URL: https://github.com/apache/druid/pull/16402#discussion_r1598297012
##########
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:
Thanks for the fixups :)
A couple of things I would check are :
1. How does the whole user message look when the exception is thrown - does
that make sense?
2. Are all the exceptions thrown by the constructor distinguishable via the
message - so that we aren't left with ambiguity when an exception arrives?
If both those things look fine, then it is good to go from my side too
--
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]
