From b0ca06f8cd900f03e0b39a306322975f814f498a Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 2 Apr 2024 12:59:48 -0700 Subject: [PATCH] Fix name of combining filtered aggregator factory. (#16224) The name of the combining filtered aggregator factory should be the same as the name of the original factory. However, it wasn't the same in the case where the original factory's name and the original delegate aggregator were inconsistently named. In this scenario, we should use the name of the original filtered aggregator, not the name of the original delegate aggregator. --- .../FilteredAggregatorFactory.java | 9 ++++++++- .../FilteredAggregatorFactoryTest.java | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FilteredAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FilteredAggregatorFactory.java index 3aa1d855945..9b8c0d20381 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FilteredAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FilteredAggregatorFactory.java @@ -139,7 +139,14 @@ public class FilteredAggregatorFactory extends AggregatorFactory @Override public AggregatorFactory getCombiningFactory() { - return delegate.getCombiningFactory(); + final AggregatorFactory delegateCombiningFactory = delegate.getCombiningFactory(); + final String myName = getName(); + + if (myName.equals(delegateCombiningFactory.getName())) { + return delegateCombiningFactory; + } else { + return delegateCombiningFactory.withName(myName); + } } @Override diff --git a/processing/src/test/java/org/apache/druid/query/aggregation/FilteredAggregatorFactoryTest.java b/processing/src/test/java/org/apache/druid/query/aggregation/FilteredAggregatorFactoryTest.java index 1f26547c8a2..1c691db6d1e 100644 --- a/processing/src/test/java/org/apache/druid/query/aggregation/FilteredAggregatorFactoryTest.java +++ b/processing/src/test/java/org/apache/druid/query/aggregation/FilteredAggregatorFactoryTest.java @@ -48,6 +48,26 @@ public class FilteredAggregatorFactoryTest extends InitializedNullHandlingTest ).getName()); } + @Test + public void testNameOfCombiningFactory() + { + Assert.assertEquals("overrideName", new FilteredAggregatorFactory( + new CountAggregatorFactory("foo"), + TrueDimFilter.instance(), + "overrideName" + ).getCombiningFactory().getName()); + Assert.assertEquals("delegateName", new FilteredAggregatorFactory( + new CountAggregatorFactory("delegateName"), + TrueDimFilter.instance(), + "" + ).getCombiningFactory().getName()); + Assert.assertEquals("delegateName", new FilteredAggregatorFactory( + new CountAggregatorFactory("delegateName"), + TrueDimFilter.instance(), + null + ).getCombiningFactory().getName()); + } + @Test public void testRequiredFields() {