From 6d1ef6e2afe8fefdd025ffaa390218b614ad4b07 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Wed, 7 Oct 2020 12:27:52 -0500
Subject: [PATCH 01/12] Add logic to rename aggregation to avoid name collision

---
 CHANGELOG.md                                  |  3 +
 .../makers/AggregationAverageMaker.java       | 13 ++++
 .../config/metric/makers/ArithmeticMaker.java | 13 ++++
 .../makers/BaseProtocolMetricMaker.java       | 64 ++++++++++++++++++-
 .../makers/AggregationAverageMakerSpec.groovy | 16 +++--
 5 files changed, 100 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7e2a121a7..db9a0f343d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,9 @@ pull request if there was one.
 Current
 -------
 ### Fixed:
+- [Fix: Add logic to rename aggregation to avoid name collision](https://github.com/yahoo/fili/issues/1095)
+    * Add renameIfConflicting logic for aggregations in BaseProtocolMetricMaker.
+
 - [Fix: Bad serialization of AllGranularity](https://github.com/yahoo/fili/issues/1093)
     * Change to rollup formatter broke serialization of all timegrain.
 
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
index 275bcd07f5..1f80569b45 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
@@ -55,6 +55,8 @@ public class AggregationAverageMaker extends BaseProtocolMetricMaker {
 
     private static final int DEPENDENT_METRICS_REQUIRED = 1;
 
+    private static final String RENAMED_AVERAGER_PREFIX = "__averager_renamed_";
+
     public static final String PROTOCOL_NAME = ReaggregationProtocol.REAGGREGATION_CONTRACT_NAME;
 
     public static final PostAggregation COUNT_INNER = new ConstantPostAggregation("one", 1);
@@ -97,6 +99,17 @@ public AggregationAverageMaker(
         this.innerGrain = innerGrain;
     }
 
+    /**
+     * Abstract method to build new renamed aggregation name.
+     *
+     * @param name The dependent metric name need rename.=
+     * @return Renamed metric name with specified prefix
+     */
+    @Override
+    protected String getRenamedMetricNameWithPrefix(String name) {
+        return RENAMED_AVERAGER_PREFIX + name;
+    }
+
     @Override
     protected TemplateDruidQuery makePartialQuery(
             final LogicalMetricInfo logicalMetricInfo,
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
index ab02e633d4..90a0357950 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
@@ -36,6 +36,8 @@ public class ArithmeticMaker extends BaseProtocolMetricMaker {
 
     private final Function<String, ResultSetMapper> resultSetMapperSupplier;
 
+    private static final String RENAMED_ARITHMETIC_PREFIX = "__airthmetic_renamed_";
+
     /**
      * Constructor.
      *
@@ -92,6 +94,17 @@ public ArithmeticMaker(MetricDictionary metricDictionary, ArithmeticPostAggregat
         );
     }
 
+    /**
+     * Abstract method to build new aggregation name.
+     *
+     * @param name The dependent metric name need rename.=
+     * @return Renamed metric name with specified prefix
+     */
+    @Override
+    protected String getRenamedMetricNameWithPrefix(String name) {
+        return RENAMED_ARITHMETIC_PREFIX + name;
+    }
+
     @Override
     public ResultSetMapper makeCalculation(LogicalMetricInfo logicalMetricInfo, List<LogicalMetric> dependentMetric) {
         return resultSetMapperSupplier.apply(logicalMetricInfo.getName());
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index 99fc1e49e9..069a43491d 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -8,10 +8,12 @@
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
 import com.yahoo.bard.webservice.data.metric.mappers.ResultSetMapper;
 import com.yahoo.bard.webservice.data.metric.protocol.DefaultSystemMetricProtocols;
+import com.yahoo.bard.webservice.data.metric.protocol.GeneratedMetricInfo;
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolMetric;
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolSupport;
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolMetricImpl;
 
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -59,12 +61,68 @@ public LogicalMetric makeInnerWithResolvedDependencies(
             LogicalMetricInfo logicalMetricInfo,
             List<LogicalMetric> dependentMetrics
     ) {
-        TemplateDruidQuery partialQuery = makePartialQuery(logicalMetricInfo, dependentMetrics);
-        ResultSetMapper calculation = makeCalculation(logicalMetricInfo, dependentMetrics);
-        ProtocolSupport protocolSupport = makeProtocolSupport(logicalMetricInfo, dependentMetrics);
+        LogicalMetric dependentMetric = dependentMetrics.get(0);
+        LogicalMetric renamedDependentMetric = renameIfConflicting(logicalMetricInfo.getName(), dependentMetric);
+        List<LogicalMetric> renamedDependentMetrics =
+                dependentMetrics.size() == 1 ? Collections.singletonList(renamedDependentMetric) : dependentMetrics;
+        TemplateDruidQuery partialQuery = makePartialQuery(logicalMetricInfo, renamedDependentMetrics);
+        ResultSetMapper calculation = makeCalculation(logicalMetricInfo, renamedDependentMetrics);
+        ProtocolSupport protocolSupport = makeProtocolSupport(logicalMetricInfo, renamedDependentMetrics);
         return new ProtocolMetricImpl(logicalMetricInfo, partialQuery, calculation, protocolSupport);
     }
 
+    /**
+     * Renames the provided logical metric if there is a name conflict between it and the final desired output name.
+     * If there is no name conflict the input metric is returned unchanged. Otherwise, the input metric is renamed to
+     * not conflict with the final output name.
+     * <p>
+     * This method specifically the {@link GeneratedMetricInfo} typing on the input metric if it has to be renamed. All
+     * other subclasses of {@link LogicalMetricInfo} are lost during the renaming. If maintaining more LogicalMetricInfo
+     * types is required, a withName method must «should be added to the LogicalMetricInfo interface.
+     *
+     * @param finalOutputName  The name to check for conflicts on
+     * @param dependentMetric  The dependent metric to check for a conflicting name
+     * @return  The metric without a name conflicting with finalOutputName
+     */
+     protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetric dependentMetric) {
+        LogicalMetricInfo dependentMetricInfo = dependentMetric.getLogicalMetricInfo();
+        //if no name conflict ,  return the original dependentMetric
+        if (!java.util.Objects.equals(finalOutputName, dependentMetricInfo.getName())) {
+            return dependentMetric;
+        }
+        String newName = getRenamedMetricNameWithPrefix(dependentMetricInfo.getName());
+        while (ifConflicting(newName, dependentMetric)) {
+            newName = getRenamedMetricNameWithPrefix(dependentMetricInfo.getName());
+        }
+        LogicalMetricInfo resultInfo;
+        if (dependentMetricInfo instanceof GeneratedMetricInfo) {
+            GeneratedMetricInfo generatedMetricInfo = (GeneratedMetricInfo) dependentMetricInfo;
+            resultInfo = new GeneratedMetricInfo(newName, generatedMetricInfo.getBaseMetricName());
+        } else {
+            resultInfo = new LogicalMetricInfo(newName);
+        }
+
+        return dependentMetric.withLogicalMetricInfo(resultInfo);
+    }
+
+    /**
+     * Abstract method to build new aggregation name.
+     * @param name The dependent metric name need rename.
+     * @return Renamed metric name with specified prefix
+     */
+    abstract protected String getRenamedMetricNameWithPrefix(String name);
+
+    /**
+     * Method to determine if there is name conflict.
+     * @param newName The dependent metric new name.
+     * @param dependentMetric The dependent LogicalMetric.
+     * @return Returns true if newName is already been used by the aggregations. otherwise false.
+     */
+    protected boolean ifConflicting(String newName, LogicalMetric dependentMetric) {
+        LogicalMetricInfo dependentMetricInfo = dependentMetric.getLogicalMetricInfo();
+        return java.util.Objects.equals(dependentMetricInfo.getName(), newName);
+    }
+
     /**
      * Create the post processing mapper for this LogicalMetric.
      *
diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
index d82a71978a..d20b3743ad 100644
--- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
+++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
@@ -30,18 +30,22 @@ import spock.lang.Specification
 class AggregationAverageMakerSpec extends Specification{
 
     static final String NAME = "users"
+    static final String NAME_RENAMED = "__averager_renamed_users"
     static final String DESCRIPTION  = NAME
     static final String ESTIMATE_NAME = "users_estimate"
-    static final String ESTIMATE_SUM_NAME = "users_estimate_sum"
+    static final String ESTIMATE_NAME_RENAMED = "__averager_renamed_users_estimate"
+    static final String ESTIMATE_SUM_NAME_RENAMED = "__averager_renamed_users_estimate_sum"
     static final int SKETCH_SIZE = 16000
     static final ZonelessTimeGrain INNER_GRAIN = DAY
 
     @Shared
     FieldConverters converter = FieldConverterSupplier.sketchConverter
     Aggregation sketchMerge
+    Aggregation sketchMergeRenamed
 
     def setup(){
         sketchMerge = new ThetaSketchAggregation(NAME, NAME, SKETCH_SIZE)
+        sketchMergeRenamed = new ThetaSketchAggregation(NAME_RENAMED, NAME, SKETCH_SIZE)
         //Initializing the Sketch field converter
         FieldConverterSupplier.sketchConverter = new ThetaSketchFieldConverter();
     }
@@ -67,7 +71,7 @@ class AggregationAverageMakerSpec extends Specification{
         and: """a test-specific inner post aggregation and the expected metric. The test-specific inner post aggregation
                 estimates the size of userSketchCount."""
         PostAggregation sketchEstimate = new ThetaSketchEstimatePostAggregation(
-                ESTIMATE_NAME,
+                ESTIMATE_NAME_RENAMED,
                 new FieldAccessorPostAggregation(userSketchCount)
         )
         LogicalMetric expectedMetric = buildExpectedMetric(sketchEstimate)
@@ -81,7 +85,7 @@ class AggregationAverageMakerSpec extends Specification{
         given: """A logical metric for counting the number of users each day, using a sketch merge and sketch
             estimate rather than a sketch count."""
         PostAggregation sketchEstimate = new ThetaSketchEstimatePostAggregation(
-                ESTIMATE_NAME,
+                ESTIMATE_NAME_RENAMED,
                 new FieldAccessorPostAggregation(sketchMerge)
         )
         TemplateDruidQuery sketchMergeAndEstimateQuery = new TemplateDruidQuery(
@@ -118,7 +122,7 @@ class AggregationAverageMakerSpec extends Specification{
         and: """the expected metric. Note that a sketch estimate is expected to be added automatically by the
                 AggregationAverageMaker."""
         PostAggregation sketchEstimate = new ThetaSketchEstimatePostAggregation(
-                ESTIMATE_NAME,
+                ESTIMATE_NAME_RENAMED,
                 new FieldAccessorPostAggregation(sketchMerge)
         )
         LogicalMetric expectedMetric = buildExpectedMetric(sketchEstimate)
@@ -162,7 +166,7 @@ class AggregationAverageMakerSpec extends Specification{
      * @return The LogicalMetric expected by the tests
      */
     LogicalMetric buildExpectedMetric(PostAggregation innerPostAggregation){
-        Set<Aggregation> innerAggregations = [sketchMerge] as LinkedHashSet
+        Set<Aggregation> innerAggregations = [sketchMergeRenamed] as LinkedHashSet
         Set<PostAggregation> innerPostAggregations = [innerPostAggregation, AggregationAverageMaker.COUNT_INNER]
         TemplateDruidQuery innerQueryTemplate = new TemplateDruidQuery(
                 innerAggregations,
@@ -170,7 +174,7 @@ class AggregationAverageMakerSpec extends Specification{
                 DAY
         )
 
-        Aggregation outerSum = new DoubleSumAggregation(ESTIMATE_SUM_NAME, ESTIMATE_NAME)
+        Aggregation outerSum = new DoubleSumAggregation(ESTIMATE_SUM_NAME_RENAMED, ESTIMATE_NAME_RENAMED)
         FieldAccessorPostAggregation outerSumLookup = new FieldAccessorPostAggregation(outerSum)
         PostAggregation average = new ArithmeticPostAggregation(
                 NAME,

From b67417314547d9324569b1536d796e72c13c8601 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Wed, 7 Oct 2020 14:48:54 -0500
Subject: [PATCH 02/12] fix tests

---
 .../metric/makers/AggregationAverageMakerSpec.groovy | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
index d20b3743ad..524c735c51 100644
--- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
+++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
@@ -57,6 +57,7 @@ class AggregationAverageMakerSpec extends Specification{
     def "Build a correct LogicalMetric when passed a sketch count aggregation."(){
         given: "a logical metric for counting the number of users each day"
         Aggregation userSketchCount = new ThetaSketchAggregation(NAME, NAME, 16000)
+        Aggregation userSketchCountRenamed = new ThetaSketchAggregation(NAME_RENAMED, NAME, 16000)
         TemplateDruidQuery sketchQuery = new TemplateDruidQuery(
                 [userSketchCount] as Set,
                 [] as Set
@@ -72,7 +73,7 @@ class AggregationAverageMakerSpec extends Specification{
                 estimates the size of userSketchCount."""
         PostAggregation sketchEstimate = new ThetaSketchEstimatePostAggregation(
                 ESTIMATE_NAME_RENAMED,
-                new FieldAccessorPostAggregation(userSketchCount)
+                new FieldAccessorPostAggregation(userSketchCountRenamed)
         )
         LogicalMetric expectedMetric = buildExpectedMetric(sketchEstimate)
         LogicalMetric actual = maker.make(NAME, NAME)
@@ -88,6 +89,10 @@ class AggregationAverageMakerSpec extends Specification{
                 ESTIMATE_NAME_RENAMED,
                 new FieldAccessorPostAggregation(sketchMerge)
         )
+        PostAggregation sketchEstimateRenamed = new ThetaSketchEstimatePostAggregation(
+                ESTIMATE_NAME_RENAMED,
+                new FieldAccessorPostAggregation(sketchMergeRenamed)
+        )
         TemplateDruidQuery sketchMergeAndEstimateQuery = new TemplateDruidQuery(
                 [sketchMerge] as Set,
                 [sketchEstimate] as Set
@@ -100,7 +105,7 @@ class AggregationAverageMakerSpec extends Specification{
 
         and: """The expected metric. Identical to the expected metric from the previous test, except that the
             sketchEstimate post aggregation is accessing a sketch merge, rather than a sketch count aggregation."""
-        LogicalMetric expectedMetric = buildExpectedMetric(sketchEstimate)
+        LogicalMetric expectedMetric = buildExpectedMetric(sketchEstimateRenamed)
 
         expect:
         maker.make(NAME, NAME).equals(expectedMetric)
@@ -109,6 +114,7 @@ class AggregationAverageMakerSpec extends Specification{
     def "Build a correct LogicalMetric when passed only a sketch merge."(){
         given: "A Logical Metric containing only a sketch estimate"
         Aggregation sketchMerge = new ThetaSketchAggregation(NAME, NAME, SKETCH_SIZE)
+        Aggregation sketchMergeRenamed = new ThetaSketchAggregation(NAME_RENAMED, NAME, SKETCH_SIZE)
         TemplateDruidQuery sketchEstimateQuery = new TemplateDruidQuery(
                 [sketchMerge] as Set,
                 [] as Set
@@ -123,7 +129,7 @@ class AggregationAverageMakerSpec extends Specification{
                 AggregationAverageMaker."""
         PostAggregation sketchEstimate = new ThetaSketchEstimatePostAggregation(
                 ESTIMATE_NAME_RENAMED,
-                new FieldAccessorPostAggregation(sketchMerge)
+                new FieldAccessorPostAggregation(sketchMergeRenamed)
         )
         LogicalMetric expectedMetric = buildExpectedMetric(sketchEstimate)
 

From a8fa6b05e12d814171f4324600ecfdbec726a160 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Wed, 7 Oct 2020 15:20:55 -0500
Subject: [PATCH 03/12] fix typo

---
 .../data/config/metric/makers/AggregationAverageMaker.java      | 2 +-
 .../webservice/data/config/metric/makers/ArithmeticMaker.java   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
index 1f80569b45..8adc65e6de 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
@@ -102,7 +102,7 @@ public AggregationAverageMaker(
     /**
      * Abstract method to build new renamed aggregation name.
      *
-     * @param name The dependent metric name need rename.=
+     * @param name The dependent metric name need rename.
      * @return Renamed metric name with specified prefix
      */
     @Override
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
index 90a0357950..7045af1e9b 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
@@ -97,7 +97,7 @@ public ArithmeticMaker(MetricDictionary metricDictionary, ArithmeticPostAggregat
     /**
      * Abstract method to build new aggregation name.
      *
-     * @param name The dependent metric name need rename.=
+     * @param name The dependent metric name need rename.
      * @return Renamed metric name with specified prefix
      */
     @Override

From 1c16b48f16fb44d06bdcb36c579f01e262e42b9f Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Thu, 8 Oct 2020 11:00:39 -0500
Subject: [PATCH 04/12] Review comments

---
 CHANGELOG.md                                  |  6 ++--
 .../makers/AggregationAverageMaker.java       |  6 ----
 .../config/metric/makers/ArithmeticMaker.java |  6 ----
 .../makers/BaseProtocolMetricMaker.java       | 29 ++++++++++++++-----
 .../makers/AggregationAverageMakerSpec.groovy | 24 +++++++++++++++
 5 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index db9a0f343d..fee5c4b45a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,8 +8,6 @@ pull request if there was one.
 Current
 -------
 ### Fixed:
-- [Fix: Add logic to rename aggregation to avoid name collision](https://github.com/yahoo/fili/issues/1095)
-    * Add renameIfConflicting logic for aggregations in BaseProtocolMetricMaker.
 
 - [Fix: Bad serialization of AllGranularity](https://github.com/yahoo/fili/issues/1093)
     * Change to rollup formatter broke serialization of all timegrain.
@@ -65,6 +63,10 @@ Current
     * Created `LegacyGenerator` as a bridge interface from the existing constructor based api request impls and the factory based value object usage.
 
 ### Added:
+- [Add logic to rename aggregation to avoid name collision](https://github.com/yahoo/fili/issues/1095)
+    * Add renameIfConflicting logic for aggregations in BaseProtocolMetricMaker.
+        - Subclasses of `BaseProtocolMetricMaker` must implement `getRenamedMetricNameWithPrefix` method to have unique rename prefix for corresponding maker. 
+    
 - [Add ability in fili-sql to translate FilteredAggregation into SQL](https://github.com/yahoo/fili/pull/1083)
    * Translate a Druid query with `n` FilteredAggregation into SQL using `(n + 1)` subquery unions. 
        - See PR description for details  
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
index 8adc65e6de..216ccc50c5 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMaker.java
@@ -99,12 +99,6 @@ public AggregationAverageMaker(
         this.innerGrain = innerGrain;
     }
 
-    /**
-     * Abstract method to build new renamed aggregation name.
-     *
-     * @param name The dependent metric name need rename.
-     * @return Renamed metric name with specified prefix
-     */
     @Override
     protected String getRenamedMetricNameWithPrefix(String name) {
         return RENAMED_AVERAGER_PREFIX + name;
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
index 7045af1e9b..ad4b6ee29b 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/ArithmeticMaker.java
@@ -94,12 +94,6 @@ public ArithmeticMaker(MetricDictionary metricDictionary, ArithmeticPostAggregat
         );
     }
 
-    /**
-     * Abstract method to build new aggregation name.
-     *
-     * @param name The dependent metric name need rename.
-     * @return Renamed metric name with specified prefix
-     */
     @Override
     protected String getRenamedMetricNameWithPrefix(String name) {
         return RENAMED_ARITHMETIC_PREFIX + name;
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index 069a43491d..94019a5dd5 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -12,9 +12,18 @@
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolMetric;
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolSupport;
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolMetricImpl;
+import com.yahoo.bard.webservice.druid.model.MetricField;
+import com.yahoo.bard.webservice.druid.model.aggregation.Aggregation;
+import com.yahoo.bard.webservice.druid.model.postaggregation.PostAggregation;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static com.yahoo.bard.webservice.util.StreamUtils.not;
 
 /**
  * An expansion on the the base {@link MetricMaker} contract that leverages the functionality of {@link ProtocolMetric}.
@@ -75,13 +84,10 @@ public LogicalMetric makeInnerWithResolvedDependencies(
      * Renames the provided logical metric if there is a name conflict between it and the final desired output name.
      * If there is no name conflict the input metric is returned unchanged. Otherwise, the input metric is renamed to
      * not conflict with the final output name.
-     * <p>
-     * This method specifically the {@link GeneratedMetricInfo} typing on the input metric if it has to be renamed. All
-     * other subclasses of {@link LogicalMetricInfo} are lost during the renaming. If maintaining more LogicalMetricInfo
-     * types is required, a withName method must «should be added to the LogicalMetricInfo interface.
      *
      * @param finalOutputName  The name to check for conflicts on
      * @param dependentMetric  The dependent metric to check for a conflicting name
+     *
      * @return  The metric without a name conflicting with finalOutputName
      */
      protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetric dependentMetric) {
@@ -92,7 +98,7 @@ protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetri
         }
         String newName = getRenamedMetricNameWithPrefix(dependentMetricInfo.getName());
         while (ifConflicting(newName, dependentMetric)) {
-            newName = getRenamedMetricNameWithPrefix(dependentMetricInfo.getName());
+            newName = getRenamedMetricNameWithPrefix(newName);
         }
         LogicalMetricInfo resultInfo;
         if (dependentMetricInfo instanceof GeneratedMetricInfo) {
@@ -106,8 +112,10 @@ protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetri
     }
 
     /**
-     * Abstract method to build new aggregation name.
+     * Method to build new renamed metric name by adding prefix to it.
+     * This method would make sure all maker subclasses use unqiue prefix to build renamed name to avoid naming collision.
      * @param name The dependent metric name need rename.
+     *
      * @return Renamed metric name with specified prefix
      */
     abstract protected String getRenamedMetricNameWithPrefix(String name);
@@ -116,11 +124,16 @@ protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetri
      * Method to determine if there is name conflict.
      * @param newName The dependent metric new name.
      * @param dependentMetric The dependent LogicalMetric.
+     *
      * @return Returns true if newName is already been used by the aggregations. otherwise false.
      */
     protected boolean ifConflicting(String newName, LogicalMetric dependentMetric) {
-        LogicalMetricInfo dependentMetricInfo = dependentMetric.getLogicalMetricInfo();
-        return java.util.Objects.equals(dependentMetricInfo.getName(), newName);
+        Set<Aggregation> aggregations = dependentMetric.getTemplateDruidQuery().getAggregations();
+        Set<PostAggregation> postAggregations = dependentMetric.getTemplateDruidQuery().getPostAggregations();
+        Set<String> allNames = Stream.concat(aggregations.stream(), postAggregations.stream())
+                .map(MetricField::getName)
+                .collect(Collectors.toSet());
+        return !allNames.add(newName);
     }
 
     /**
diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
index 524c735c51..20c1d64444 100644
--- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
+++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
@@ -2,6 +2,9 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.data.config.metric.makers
 
+import com.yahoo.bard.webservice.data.metric.LogicalMetricImpl
+import com.yahoo.bard.webservice.druid.model.aggregation.LongSumAggregation
+
 import static com.yahoo.bard.webservice.data.metric.protocol.protocols.ReaggregationProtocol.REAGGREGATION_CONTRACT_NAME
 import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.DAY
 
@@ -37,6 +40,7 @@ class AggregationAverageMakerSpec extends Specification{
     static final String ESTIMATE_SUM_NAME_RENAMED = "__averager_renamed_users_estimate_sum"
     static final int SKETCH_SIZE = 16000
     static final ZonelessTimeGrain INNER_GRAIN = DAY
+    AggregationAverageMaker averageMaker
 
     @Shared
     FieldConverters converter = FieldConverterSupplier.sketchConverter
@@ -137,6 +141,26 @@ class AggregationAverageMakerSpec extends Specification{
         maker.make(NAME, NAME).equals(expectedMetric)
     }
 
+    def "When output name collides with dependent metric name, dependent metric must be renamed"() {
+        setup:
+        String metricName = "inputMetric"
+        String finalMetricName = "inputMetric"
+        LogicalMetricInfo inputMetricInfo = new LogicalMetricInfo(metricName)
+        LogicalMetric inputMetric = new LogicalMetricImpl(
+                inputMetricInfo,
+                new TemplateDruidQuery([new LongSumAggregation(metricName, "unused")], []),
+                new NoOpResultSetMapper()
+        )
+        MetricMaker maker = new AggregationAverageMaker(new MetricDictionary(), INNER_GRAIN)
+        maker.metrics.add(inputMetric)
+
+        when:
+        LogicalMetric result = maker.renameIfConflicting(finalMetricName, inputMetric)
+
+        then:
+        result.getName() == AggregationAverageMaker.RENAMED_AVERAGER_PREFIX + metricName
+    }
+
     LogicalMetric buildDependentMetric(TemplateDruidQuery dependentQuery){
         return new ProtocolMetricImpl(
                 new LogicalMetricInfo(NAME, DESCRIPTION),

From ca31a5344f26b3467f7fb33333d3e00e42c98c2a Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Thu, 8 Oct 2020 11:56:03 -0500
Subject: [PATCH 05/12] fix build failure

---
 .../data/config/metric/makers/BaseProtocolMetricMaker.java | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index 94019a5dd5..d3717d38c4 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -17,14 +17,12 @@
 import com.yahoo.bard.webservice.druid.model.postaggregation.PostAggregation;
 
 import java.util.Collections;
-import java.util.HashSet;
+
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.yahoo.bard.webservice.util.StreamUtils.not;
-
 /**
  * An expansion on the the base {@link MetricMaker} contract that leverages the functionality of {@link ProtocolMetric}.
  *
@@ -113,7 +111,8 @@ protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetri
 
     /**
      * Method to build new renamed metric name by adding prefix to it.
-     * This method would make sure all maker subclasses use unqiue prefix to build renamed name to avoid naming collision.
+     * This method would make sure all maker subclasses use unqiue prefix to build renamed name
+     * to avoid naming collision.
      * @param name The dependent metric name need rename.
      *
      * @return Renamed metric name with specified prefix

From 1051d46a39e5906a44cd41b78d8b0d8611d7f901 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Mon, 12 Oct 2020 15:58:45 -0500
Subject: [PATCH 06/12] addressed review comments

---
 CHANGELOG.md                                    |  2 +-
 .../metric/makers/BaseProtocolMetricMaker.java  | 15 ++++++++-------
 .../makers/AggregationAverageMakerSpec.groovy   | 17 +++++++++++------
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fee5c4b45a..f8b70ab1ab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -65,7 +65,7 @@ Current
 ### Added:
 - [Add logic to rename aggregation to avoid name collision](https://github.com/yahoo/fili/issues/1095)
     * Add renameIfConflicting logic for aggregations in BaseProtocolMetricMaker.
-        - Subclasses of `BaseProtocolMetricMaker` must implement `getRenamedMetricNameWithPrefix` method to have unique rename prefix for corresponding maker. 
+        - Subclasses of `BaseProtocolMetricMaker` can implement `getRenamedMetricNameWithPrefix` method to have unique rename prefix for corresponding maker. 
     
 - [Add ability in fili-sql to translate FilteredAggregation into SQL](https://github.com/yahoo/fili/pull/1083)
    * Translate a Druid query with `n` FilteredAggregation into SQL using `(n + 1)` subquery unions. 
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index d3717d38c4..8e38d92195 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -36,6 +36,8 @@ public abstract class BaseProtocolMetricMaker extends MetricMaker implements Mak
      */
     protected final ProtocolSupport baseProtocolSupport;
 
+    private static final String DEFAULT_RENAMED_PREFIX = "__renamed_";
+
     /**
      * Construct a fully specified MetricMaker.
      *
@@ -117,7 +119,9 @@ protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetri
      *
      * @return Renamed metric name with specified prefix
      */
-    abstract protected String getRenamedMetricNameWithPrefix(String name);
+    protected String getRenamedMetricNameWithPrefix(String name) {
+        return DEFAULT_RENAMED_PREFIX + name;
+    }
 
     /**
      * Method to determine if there is name conflict.
@@ -127,12 +131,9 @@ protected LogicalMetric renameIfConflicting(String finalOutputName, LogicalMetri
      * @return Returns true if newName is already been used by the aggregations. otherwise false.
      */
     protected boolean ifConflicting(String newName, LogicalMetric dependentMetric) {
-        Set<Aggregation> aggregations = dependentMetric.getTemplateDruidQuery().getAggregations();
-        Set<PostAggregation> postAggregations = dependentMetric.getTemplateDruidQuery().getPostAggregations();
-        Set<String> allNames = Stream.concat(aggregations.stream(), postAggregations.stream())
-                .map(MetricField::getName)
-                .collect(Collectors.toSet());
-        return !allNames.add(newName);
+        Set<String> aggregations = dependentMetric.getTemplateDruidQuery().getAggregations().stream().map(MetricField::getName).collect(Collectors.toSet());
+        Set<String> postAggregations = dependentMetric.getTemplateDruidQuery().getPostAggregations().stream().map(MetricField::getName).collect(Collectors.toSet());
+        return aggregations.contains(newName) || postAggregations.contains(newName);
     }
 
     /**
diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
index 20c1d64444..ffaeea04ac 100644
--- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
+++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
@@ -3,6 +3,7 @@
 package com.yahoo.bard.webservice.data.config.metric.makers
 
 import com.yahoo.bard.webservice.data.metric.LogicalMetricImpl
+import com.yahoo.bard.webservice.data.metric.protocol.GeneratedMetricInfo
 import com.yahoo.bard.webservice.druid.model.aggregation.LongSumAggregation
 
 import static com.yahoo.bard.webservice.data.metric.protocol.protocols.ReaggregationProtocol.REAGGREGATION_CONTRACT_NAME
@@ -145,20 +146,24 @@ class AggregationAverageMakerSpec extends Specification{
         setup:
         String metricName = "inputMetric"
         String finalMetricName = "inputMetric"
-        LogicalMetricInfo inputMetricInfo = new LogicalMetricInfo(metricName)
-        LogicalMetric inputMetric = new LogicalMetricImpl(
-                inputMetricInfo,
-                new TemplateDruidQuery([new LongSumAggregation(metricName, "unused")], []),
+        LogicalMetric inputMetric1 = new LogicalMetricImpl(
+                info,
+                new TemplateDruidQuery([new LongSumAggregation("inputMetric", "unused")], []),
                 new NoOpResultSetMapper()
         )
         MetricMaker maker = new AggregationAverageMaker(new MetricDictionary(), INNER_GRAIN)
-        maker.metrics.add(inputMetric)
+        maker.metrics.add(inputMetric1)
 
         when:
-        LogicalMetric result = maker.renameIfConflicting(finalMetricName, inputMetric)
+        LogicalMetric result = maker.renameIfConflicting(finalMetricName, inputMetric1)
 
         then:
         result.getName() == AggregationAverageMaker.RENAMED_AVERAGER_PREFIX + metricName
+
+        where:
+        info                                             | infotype
+        new LogicalMetricInfo("inputMetric")                | "Logical Metric Info"
+        new GeneratedMetricInfo("inputMetric", "baseName")  | "Generated Metric Info"
     }
 
     LogicalMetric buildDependentMetric(TemplateDruidQuery dependentQuery){

From aacc06aaa06806cdc65b89fbfc961fea0e6b8f57 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Mon, 12 Oct 2020 16:43:13 -0500
Subject: [PATCH 07/12] fix style errors

---
 .../config/metric/makers/BaseProtocolMetricMaker.java    | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index 8e38d92195..506394b25b 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -13,15 +13,12 @@
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolSupport;
 import com.yahoo.bard.webservice.data.metric.protocol.ProtocolMetricImpl;
 import com.yahoo.bard.webservice.druid.model.MetricField;
-import com.yahoo.bard.webservice.druid.model.aggregation.Aggregation;
-import com.yahoo.bard.webservice.druid.model.postaggregation.PostAggregation;
 
 import java.util.Collections;
 
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 /**
  * An expansion on the the base {@link MetricMaker} contract that leverages the functionality of {@link ProtocolMetric}.
@@ -131,8 +128,10 @@ protected String getRenamedMetricNameWithPrefix(String name) {
      * @return Returns true if newName is already been used by the aggregations. otherwise false.
      */
     protected boolean ifConflicting(String newName, LogicalMetric dependentMetric) {
-        Set<String> aggregations = dependentMetric.getTemplateDruidQuery().getAggregations().stream().map(MetricField::getName).collect(Collectors.toSet());
-        Set<String> postAggregations = dependentMetric.getTemplateDruidQuery().getPostAggregations().stream().map(MetricField::getName).collect(Collectors.toSet());
+        Set<String> aggregations = dependentMetric.getTemplateDruidQuery().getAggregations().stream()
+                .map(MetricField::getName).collect(Collectors.toSet());
+        Set<String> postAggregations = dependentMetric.getTemplateDruidQuery().getPostAggregations()
+                .stream().map(MetricField::getName).collect(Collectors.toSet());
         return aggregations.contains(newName) || postAggregations.contains(newName);
     }
 

From 42feaf1f7b3fd3c907a08bf13e34ff1d7284b724 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Mon, 12 Oct 2020 20:09:35 -0500
Subject: [PATCH 08/12] update changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f8b70ab1ab..c1b50b293c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -66,6 +66,7 @@ Current
 - [Add logic to rename aggregation to avoid name collision](https://github.com/yahoo/fili/issues/1095)
     * Add renameIfConflicting logic for aggregations in BaseProtocolMetricMaker.
         - Subclasses of `BaseProtocolMetricMaker` can implement `getRenamedMetricNameWithPrefix` method to have unique rename prefix for corresponding maker. 
+        - Default implementation will add Prefix `__renamed_` whenever there is aggregation name collision. 
     
 - [Add ability in fili-sql to translate FilteredAggregation into SQL](https://github.com/yahoo/fili/pull/1083)
    * Translate a Druid query with `n` FilteredAggregation into SQL using `(n + 1)` subquery unions. 

From 8918ee77ab9be6e614acdf3bcbe4d03afca54e34 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Tue, 13 Oct 2020 10:34:38 -0500
Subject: [PATCH 09/12] retry build.

---
 .../config/metric/makers/AggregationAverageMakerSpec.groovy | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
index ffaeea04ac..d545b48349 100644
--- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
+++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/metric/makers/AggregationAverageMakerSpec.groovy
@@ -146,16 +146,16 @@ class AggregationAverageMakerSpec extends Specification{
         setup:
         String metricName = "inputMetric"
         String finalMetricName = "inputMetric"
-        LogicalMetric inputMetric1 = new LogicalMetricImpl(
+        LogicalMetric inputMetric = new LogicalMetricImpl(
                 info,
                 new TemplateDruidQuery([new LongSumAggregation("inputMetric", "unused")], []),
                 new NoOpResultSetMapper()
         )
         MetricMaker maker = new AggregationAverageMaker(new MetricDictionary(), INNER_GRAIN)
-        maker.metrics.add(inputMetric1)
+        maker.metrics.add(inputMetric)
 
         when:
-        LogicalMetric result = maker.renameIfConflicting(finalMetricName, inputMetric1)
+        LogicalMetric result = maker.renameIfConflicting(finalMetricName, inputMetric)
 
         then:
         result.getName() == AggregationAverageMaker.RENAMED_AVERAGER_PREFIX + metricName

From 85748d0c11a4e7cb42853de25135bcae19be149a Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Tue, 13 Oct 2020 11:24:45 -0500
Subject: [PATCH 10/12] retry build

---
 .../data/config/metric/makers/BaseProtocolMetricMaker.java       | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index 506394b25b..3cca87ec0e 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -74,6 +74,7 @@ public LogicalMetric makeInnerWithResolvedDependencies(
         TemplateDruidQuery partialQuery = makePartialQuery(logicalMetricInfo, renamedDependentMetrics);
         ResultSetMapper calculation = makeCalculation(logicalMetricInfo, renamedDependentMetrics);
         ProtocolSupport protocolSupport = makeProtocolSupport(logicalMetricInfo, renamedDependentMetrics);
+        
         return new ProtocolMetricImpl(logicalMetricInfo, partialQuery, calculation, protocolSupport);
     }
 

From a5dbecf1f990ae58edf0d376d6bb5c4f415beed7 Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Tue, 13 Oct 2020 11:38:10 -0500
Subject: [PATCH 11/12] fix checkstyle error

---
 .../data/config/metric/makers/BaseProtocolMetricMaker.java      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
index 3cca87ec0e..dbea54ae7f 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/data/config/metric/makers/BaseProtocolMetricMaker.java
@@ -74,7 +74,7 @@ public LogicalMetric makeInnerWithResolvedDependencies(
         TemplateDruidQuery partialQuery = makePartialQuery(logicalMetricInfo, renamedDependentMetrics);
         ResultSetMapper calculation = makeCalculation(logicalMetricInfo, renamedDependentMetrics);
         ProtocolSupport protocolSupport = makeProtocolSupport(logicalMetricInfo, renamedDependentMetrics);
-        
+
         return new ProtocolMetricImpl(logicalMetricInfo, partialQuery, calculation, protocolSupport);
     }
 

From ec1eb06345959106c447718ea904de872b2ea66e Mon Sep 17 00:00:00 2001
From: Bharat Motwani <>
Date: Tue, 13 Oct 2020 11:50:05 -0500
Subject: [PATCH 12/12] fix constructor

---
 .../table/resolver/QueryPlanningConstraint.java        | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/table/resolver/QueryPlanningConstraint.java b/fili-core/src/main/java/com/yahoo/bard/webservice/table/resolver/QueryPlanningConstraint.java
index 19d5fa02ed..90f7654771 100644
--- a/fili-core/src/main/java/com/yahoo/bard/webservice/table/resolver/QueryPlanningConstraint.java
+++ b/fili-core/src/main/java/com/yahoo/bard/webservice/table/resolver/QueryPlanningConstraint.java
@@ -49,11 +49,11 @@ public class QueryPlanningConstraint extends BaseDataSourceConstraint {
      * @param requestGranularity  The requested granularity of on the requested table
      */
     public QueryPlanningConstraint(
-            Set<Dimension> requestDimensions,
-            Set<Dimension> filterDimensions,
-            Set<Dimension> metricDimensions,
-            Set<String> metricNames,
-            ApiFilters apiFilters,
+            @NotNull Set<Dimension> requestDimensions,
+            @NotNull Set<Dimension> filterDimensions,
+            @NotNull Set<Dimension> metricDimensions,
+            @NotNull Set<String> metricNames,
+            @NotNull ApiFilters apiFilters,
             LogicalTable logicalTable,
             List<Interval> intervals,
             Set<LogicalMetric> logicalMetrics,