From c55b37d7ec8dfdecaa5a6349692ed34895d32edb Mon Sep 17 00:00:00 2001 From: Charles Allen Date: Thu, 11 Oct 2018 11:56:53 -0700 Subject: [PATCH] Add optional `name` to top level of FilteredAggregatorFactory (#6219) * Add optional `name` to top level of FilteredAggregatorFactory * Add compat constructor for tests * Address comments * Add equals and hash code updates * Rename test * Fix imports and code style --- .../FilteredAggregatorFactory.java | 64 +++++++++++-------- .../FilteredAggregatorFactoryTest.java | 47 ++++++++++++++ 2 files changed, 86 insertions(+), 25 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/query/aggregation/FilteredAggregatorFactoryTest.java 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 313bec1dfce..62ee3ea91c7 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 @@ -19,8 +19,10 @@ package org.apache.druid.query.aggregation; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import org.apache.druid.query.PerSegmentQueryOptimizationContext; import org.apache.druid.query.filter.DimFilter; import org.apache.druid.query.filter.IntervalDimFilter; @@ -34,15 +36,28 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.Objects; public class FilteredAggregatorFactory extends AggregatorFactory { private final AggregatorFactory delegate; private final DimFilter filter; + private final String name; + // Constructor for backwards compat only + public FilteredAggregatorFactory( + AggregatorFactory delegate, + DimFilter filter + ) + { + this(delegate, filter, null); + } + + @JsonCreator public FilteredAggregatorFactory( @JsonProperty("aggregator") AggregatorFactory delegate, - @JsonProperty("filter") DimFilter filter + @JsonProperty("filter") DimFilter filter, + @JsonProperty("name") String name ) { Preconditions.checkNotNull(delegate); @@ -50,6 +65,7 @@ public class FilteredAggregatorFactory extends AggregatorFactory this.delegate = delegate; this.filter = filter; + this.name = name; } @Override @@ -108,11 +124,16 @@ public class FilteredAggregatorFactory extends AggregatorFactory return delegate.finalizeComputation(object); } + // See https://github.com/apache/incubator-druid/pull/6219#pullrequestreview-148919845 @JsonProperty @Override public String getName() { - return delegate.getName(); + String name = this.name; + if (Strings.isNullOrEmpty(name)) { + name = delegate.getName(); + } + return name; } @Override @@ -198,7 +219,8 @@ public class FilteredAggregatorFactory extends AggregatorFactory intervalDimFilter.getDimension(), effectiveFilterIntervals, intervalDimFilter.getExtractionFn() - ) + ), + this.name ); } else { return this; @@ -223,15 +245,6 @@ public class FilteredAggregatorFactory extends AggregatorFactory return delegate.getRequiredColumns(); } - @Override - public String toString() - { - return "FilteredAggregatorFactory{" + - "delegate=" + delegate + - ", filter=" + filter + - '}'; - } - @Override public boolean equals(Object o) { @@ -241,24 +254,25 @@ public class FilteredAggregatorFactory extends AggregatorFactory if (o == null || getClass() != o.getClass()) { return false; } - FilteredAggregatorFactory that = (FilteredAggregatorFactory) o; - - if (delegate != null ? !delegate.equals(that.delegate) : that.delegate != null) { - return false; - } - if (filter != null ? !filter.equals(that.filter) : that.filter != null) { - return false; - } - - return true; + return Objects.equals(delegate, that.delegate) && + Objects.equals(filter, that.filter) && + Objects.equals(name, that.name); } @Override public int hashCode() { - int result = delegate != null ? delegate.hashCode() : 0; - result = 31 * result + (filter != null ? filter.hashCode() : 0); - return result; + return Objects.hash(delegate, filter, name); + } + + @Override + public String toString() + { + return "FilteredAggregatorFactory{" + + "delegate=" + delegate + + ", filter=" + filter + + ", name='" + name + '\'' + + '}'; } } 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 new file mode 100644 index 00000000000..4db7b0e5c54 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/aggregation/FilteredAggregatorFactoryTest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.query.aggregation; + +import org.apache.druid.query.filter.TrueDimFilter; +import org.junit.Assert; +import org.junit.Test; + +public class FilteredAggregatorFactoryTest +{ + @Test + public void testSimpleNaming() + { + Assert.assertEquals("overrideName", new FilteredAggregatorFactory( + new CountAggregatorFactory("foo"), + new TrueDimFilter(), + "overrideName" + ).getName()); + Assert.assertEquals("delegateName", new FilteredAggregatorFactory( + new CountAggregatorFactory("delegateName"), + new TrueDimFilter(), + "" + ).getName()); + Assert.assertEquals("delegateName", new FilteredAggregatorFactory( + new CountAggregatorFactory("delegateName"), + new TrueDimFilter(), + null + ).getName()); + } +}