[GitHub] nishantmonu51 commented on a change in pull request #6219: Add optional `name` to top level of FilteredAggregatorFactory

[GitBox <gi...@apache.org>]

nishantmonu51 commented on a change in pull request #6219: Add optional `name` to top level of FilteredAggregatorFactory
URL: https://github.com/apache/incubator-druid/pull/6219#discussion_r212316259
 
 

 ##########
 File path: processing/src/main/java/io/druid/query/aggregation/FilteredAggregatorFactory.java
 ##########
 @@ -112,7 +127,11 @@ public Object finalizeComputation(Object object)
   @Override
   public String getName()
   {
-    return delegate.getName();
+    String name = delegate.getName();
+    if (Strings.isNullOrEmpty(name)) {
+      name = this.name;
+    }
+    return name;
 
 Review comment:
   It would be wierd to ignore the name set in FilteredAggregator when user explicitly sets it.
    
   IMO, the default should be - If user has set a name for FilteredAggregatorFactory - use that, otherwise use the name of the delegate. For backwards compatible case user would not have specified name at filtered aggregator level and will get the old behavior of getting delegate name. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org