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
This commit is contained in:
Charles Allen 2018-10-11 11:56:53 -07:00 committed by Jonathan Wei
parent ab7b4798cc
commit c55b37d7ec
2 changed files with 86 additions and 25 deletions

View File

@ -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 + '\'' +
'}';
}
}

View File

@ -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());
}
}