From 3c15734f3136947ed61c0dc5bdea5b35e07eae99 Mon Sep 17 00:00:00 2001 From: Gary Luo Date: Tue, 30 Jan 2018 12:32:44 -0600 Subject: [PATCH 1/4] Add extraction function to selector dimensional filter --- CHANGELOG.md | 6 +++ .../druid/model/filter/DimensionalFilter.java | 37 +++++++++++++-- .../druid/model/filter/ExtractionFilter.java | 24 ++++------ .../druid/model/filter/SelectorFilter.java | 26 ++++++++++- .../model/filter/SelectorFilterSpec.groovy | 45 +++++++++++++++++++ 5 files changed, 118 insertions(+), 20 deletions(-) create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy diff --git a/CHANGELOG.md b/CHANGELOG.md index 66dde68a71..e9fb047aff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ Current ### Added: +- [Extraction Function on selector filter](https://github.com/yahoo/fili/pull/617) + * Added extraction function on dimensional filter, defaults to extraction function on dimension if it exists. + - [Implement TimeFormatExtractionFunction](https://github.com/yahoo/fili/pull/611) * Enable [`TimeFormatExtractionFunction`](http://druid.io/docs/0.10.1/querying/dimensionspecs.html#time-format-extraction-function) in Fili so that API users could interact with Druid using `TimeFormatExtractionFunction` through Fili. @@ -239,6 +242,9 @@ Current ### Deprecated: +- [Extraction Function on selector filter](https://github.com/yahoo/fili/pull/617) + * Deprecated `ExtractionFilter` since it is deprecated in druid, use selector filter with extraction function instead + - [Rename filter variables and methods in DataApiRequest](https://github.com/yahoo/fili/pull/507) * Deprecated `getFilters` in favor of `getApiFilters` and `getFilter` in favor of `getDruidFilter` diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java index c24f9a2de7..336f3d3a2e 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java @@ -3,8 +3,12 @@ package com.yahoo.bard.webservice.druid.model.filter; import com.yahoo.bard.webservice.data.dimension.Dimension; +import com.yahoo.bard.webservice.data.dimension.impl.ExtractionFunctionDimension; +import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.ExtractionFunction; import com.yahoo.bard.webservice.druid.serializers.DimensionToNameSerializer; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.util.Objects; @@ -14,19 +18,40 @@ * * @param a DimensionalFilter */ +@JsonInclude(JsonInclude.Include.NON_NULL) public abstract class DimensionalFilter> extends Filter { private final Dimension dimension; + private final ExtractionFunction extractionFunction; /** - * Constructor. + * Constructor, default extraction function to the one on dimension if it has one. * * @param dimension Dimension to filter * @param type Type of the filter */ protected DimensionalFilter(Dimension dimension, FilterType type) { super(type); + + this.dimension = dimension; + if (dimension instanceof ExtractionFunctionDimension) { + extractionFunction = ((ExtractionFunctionDimension) dimension).getExtractionFunction().get(); + } else { + extractionFunction = null; + } + } + + /** + * Constructor, with explicit extraction function provided. + * + * @param dimension Dimension to filter + * @param type Type of the filter + * @param extractionFunction Extraction function to be applied on dimension + */ + protected DimensionalFilter(Dimension dimension, FilterType type, ExtractionFunction extractionFunction) { + super(type); this.dimension = dimension; + this.extractionFunction = extractionFunction; } @JsonSerialize(using = DimensionToNameSerializer.class) @@ -34,6 +59,11 @@ public Dimension getDimension() { return dimension; } + @JsonProperty(value = "extractionFn") + public ExtractionFunction getExtractionFunction() { + return extractionFunction; + } + /** * Get a new instance of this filter with the given Dimension. * @@ -45,7 +75,7 @@ public Dimension getDimension() { @Override public int hashCode() { - return Objects.hash(super.hashCode(), dimension); + return Objects.hash(super.hashCode(), dimension, extractionFunction); } @Override @@ -56,6 +86,7 @@ public boolean equals(Object obj) { return super.equals(obj) && - Objects.equals(dimension, other.dimension); + Objects.equals(dimension, other.dimension) && + Objects.equals(extractionFunction, other.extractionFunction); } } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java index 576ac6281e..ac2a1b989c 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java @@ -5,19 +5,18 @@ import com.yahoo.bard.webservice.data.dimension.Dimension; import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.ExtractionFunction; -import com.fasterxml.jackson.annotation.JsonProperty; - import java.util.Objects; /** * Filter for matching a dimension using some specific Extraction function. + * + * @deprecated Use other dimensional filters with extractionFn specified instead */ +@Deprecated public class ExtractionFilter extends DimensionalFilter { private final String value; - private final ExtractionFunction extractionFunction; - /** * Constructor. * @@ -26,23 +25,17 @@ public class ExtractionFilter extends DimensionalFilter { * @param extractionFn Function to do the extraction */ public ExtractionFilter(Dimension dimension, String value, ExtractionFunction extractionFn) { - super(dimension, DefaultFilterType.EXTRACTION); + super(dimension, DefaultFilterType.EXTRACTION, extractionFn); this.value = value; - this.extractionFunction = extractionFn; } public String getValue() { return value; } - @JsonProperty(value = "extractionFn") - public ExtractionFunction getExtractionFunction() { - return extractionFunction; - } - @Override public ExtractionFilter withDimension(Dimension dimension) { - return new ExtractionFilter(dimension, value, extractionFunction); + return new ExtractionFilter(dimension, value, getExtractionFunction()); } /** @@ -53,7 +46,7 @@ public ExtractionFilter withDimension(Dimension dimension) { * @return a new instance of this filter with the given value */ public ExtractionFilter withValue(String value) { - return new ExtractionFilter(getDimension(), value, extractionFunction); + return new ExtractionFilter(getDimension(), value, getExtractionFunction()); } /** @@ -69,7 +62,7 @@ public ExtractionFilter withExtractionFunction(ExtractionFunction extractionFunc @Override public int hashCode() { - return Objects.hash(super.hashCode(), value, extractionFunction); + return Objects.hash(super.hashCode(), value); } @Override @@ -80,7 +73,6 @@ public boolean equals(Object obj) { return super.equals(obj) && - Objects.equals(value, other.value) && - Objects.equals(extractionFunction, other.extractionFunction); + Objects.equals(value, other.value); } } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java index 5e973f00a3..fd1a2ff8e7 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java @@ -3,6 +3,7 @@ package com.yahoo.bard.webservice.druid.model.filter; import com.yahoo.bard.webservice.data.dimension.Dimension; +import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.ExtractionFunction; import java.util.Objects; @@ -20,7 +21,19 @@ public class SelectorFilter extends DimensionalFilter { * @param value Value of the filter */ public SelectorFilter(Dimension dimension, String value) { - super(dimension, DefaultFilterType.SELECTOR); + this(dimension, value, null); + } + + /** + * Constructor. + * + * @param dimension Dimension to apply the extraction to + * @param value Value of the filter + * @param extractionFn Extraction function to be applied on dimension + * + */ + public SelectorFilter(Dimension dimension, String value, ExtractionFunction extractionFn) { + super(dimension, DefaultFilterType.SELECTOR, extractionFn); this.value = value; } @@ -44,6 +57,17 @@ public SelectorFilter withValue(String value) { return new SelectorFilter(getDimension(), value); } + /** + * Get a new instance of this filter with the given value. + * + * @param extractionFn Extraction function to be applied on dimension + * + * @return a new instance of this filter with the given value + */ + public SelectorFilter withExtractionFn(ExtractionFunction extractionFn) { + return new SelectorFilter(getDimension(), value, extractionFn); + } + @Override public int hashCode() { return Objects.hash(super.hashCode(), value); diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy new file mode 100644 index 0000000000..0c9cc37afa --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy @@ -0,0 +1,45 @@ +package com.yahoo.bard.webservice.druid.model.filter + +import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.RegisteredLookupExtractionFunction + +import com.fasterxml.jackson.databind.ObjectMapper + +import spock.lang.Specification + +/** + * Test selector filter serialization. + */ +class SelectorFilterSpec extends Specification{ + + ObjectMapper objectMapper + + // Dimension missing due to lack of proper way to inject mock dimension value and therefore null is given + String expectedSerialization = + """ + { + "type":"selector", + "extractionFn":{ + "type":"registeredLookup", + "lookup":"lookup", + "retainMissingValue":false, + "replaceMissingValueWith":"none", + "injective":false, + "optimize":false + }, + "value":"value" + } + """ + + def setup() { + objectMapper = new ObjectMapper() + } + + def "Test dimensional filter serialize as expected by druid"() { + given: + SelectorFilter filter = new SelectorFilter(null, "value", new RegisteredLookupExtractionFunction("lookup", false, "none", false, false)) + String serializedFilter = objectMapper.writeValueAsString(filter) + + expect: + objectMapper.readTree(serializedFilter) == objectMapper.readTree(expectedSerialization) + } +} From b40cdce39cf3fa1318e4e10dd9406e329dc5cf5f Mon Sep 17 00:00:00 2001 From: Gary Luo Date: Tue, 30 Jan 2018 13:37:11 -0600 Subject: [PATCH 2/4] Modified according to jiaqi's comments --- .../druid/model/filter/DimensionalFilter.java | 12 ++++++++---- .../druid/model/filter/SelectorFilter.java | 4 ++-- .../druid/model/filter/SelectorFilterSpec.groovy | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java index 336f3d3a2e..7ab80d2ac3 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/DimensionalFilter.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.util.Objects; +import java.util.Optional; /** * Filter for matching a dimension. @@ -34,10 +35,13 @@ protected DimensionalFilter(Dimension dimension, FilterType type) { super(type); this.dimension = dimension; + if (dimension instanceof ExtractionFunctionDimension) { - extractionFunction = ((ExtractionFunctionDimension) dimension).getExtractionFunction().get(); + Optional optionalExtractionFunction = ((ExtractionFunctionDimension) dimension) + .getExtractionFunction(); + this.extractionFunction = optionalExtractionFunction.isPresent() ? optionalExtractionFunction.get() : null; } else { - extractionFunction = null; + this.extractionFunction = null; } } @@ -86,7 +90,7 @@ public boolean equals(Object obj) { return super.equals(obj) && - Objects.equals(dimension, other.dimension) && - Objects.equals(extractionFunction, other.extractionFunction); + Objects.equals(dimension, other.dimension) && + Objects.equals(extractionFunction, other.extractionFunction); } } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java index fd1a2ff8e7..4a09ade729 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java @@ -21,7 +21,8 @@ public class SelectorFilter extends DimensionalFilter { * @param value Value of the filter */ public SelectorFilter(Dimension dimension, String value) { - this(dimension, value, null); + super(dimension, DefaultFilterType.SELECTOR); + this.value = value; } /** @@ -30,7 +31,6 @@ public SelectorFilter(Dimension dimension, String value) { * @param dimension Dimension to apply the extraction to * @param value Value of the filter * @param extractionFn Extraction function to be applied on dimension - * */ public SelectorFilter(Dimension dimension, String value, ExtractionFunction extractionFn) { super(dimension, DefaultFilterType.SELECTOR, extractionFn); diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy index 0c9cc37afa..734857b804 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy @@ -1,3 +1,5 @@ +// Copyright 2017 Yahoo Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. package com.yahoo.bard.webservice.druid.model.filter import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.RegisteredLookupExtractionFunction @@ -9,7 +11,7 @@ import spock.lang.Specification /** * Test selector filter serialization. */ -class SelectorFilterSpec extends Specification{ +class SelectorFilterSpec extends Specification { ObjectMapper objectMapper From 0e0dabe67c7cf17023990acb1dfb414e841cf224 Mon Sep 17 00:00:00 2001 From: Gary Luo Date: Tue, 30 Jan 2018 20:12:55 -0600 Subject: [PATCH 3/4] modified according to mike's comments --- .../bard/webservice/druid/model/filter/ExtractionFilter.java | 2 +- .../bard/webservice/druid/model/filter/SelectorFilter.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java index ac2a1b989c..89ee836c2c 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/ExtractionFilter.java @@ -10,7 +10,7 @@ /** * Filter for matching a dimension using some specific Extraction function. * - * @deprecated Use other dimensional filters with extractionFn specified instead + * @deprecated Use {@link SelectorFilter} dimensional filters with extractionFn specified instead */ @Deprecated public class ExtractionFilter extends DimensionalFilter { diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java index 4a09ade729..0b9c05a646 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/filter/SelectorFilter.java @@ -43,7 +43,7 @@ public String getValue() { @Override public SelectorFilter withDimension(Dimension dimension) { - return new SelectorFilter(dimension, value); + return new SelectorFilter(dimension, value, getExtractionFunction()); } /** @@ -54,7 +54,7 @@ public SelectorFilter withDimension(Dimension dimension) { * @return a new instance of this filter with the given value */ public SelectorFilter withValue(String value) { - return new SelectorFilter(getDimension(), value); + return new SelectorFilter(getDimension(), value, getExtractionFunction()); } /** From fd437eaef8f0a49a72c9615ad526940bb7cccb96 Mon Sep 17 00:00:00 2001 From: Michael McLawhorn Date: Wed, 31 Jan 2018 15:31:08 -0600 Subject: [PATCH 4/4] Testing both constructor patterns and not using the 'null' dimension option. --- .../model/filter/SelectorFilterSpec.groovy | 64 +++++++++++++++++-- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy index 734857b804..bc84cb0dfe 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/filter/SelectorFilterSpec.groovy @@ -2,23 +2,32 @@ // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. package com.yahoo.bard.webservice.druid.model.filter +import com.yahoo.bard.webservice.data.dimension.impl.LookupDimension +import com.yahoo.bard.webservice.druid.model.datasource.TableDataSource +import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.ExtractionFunction import com.yahoo.bard.webservice.druid.model.dimension.extractionfunction.RegisteredLookupExtractionFunction +import com.yahoo.bard.webservice.druid.model.query.DruidAggregationQuery +import com.yahoo.bard.webservice.table.ConstrainedTable import com.fasterxml.jackson.databind.ObjectMapper import spock.lang.Specification - /** * Test selector filter serialization. */ class SelectorFilterSpec extends Specification { ObjectMapper objectMapper + LookupDimension dimension + DruidAggregationQuery druidQuery + + ExtractionFunction extractionFunction // Dimension missing due to lack of proper way to inject mock dimension value and therefore null is given String expectedSerialization = """ { + "dimension": "foo", "type":"selector", "extractionFn":{ "type":"registeredLookup", @@ -32,16 +41,61 @@ class SelectorFilterSpec extends Specification { } """ + // Dimension missing due to lack of proper way to inject mock dimension value and therefore null is given + String expectedSerialization2 = + """ + { + "dimension": "foo", + "type":"selector", + "extractionFn":{ + "type":"registeredLookup", + "lookup":"lookup2", + "retainMissingValue":false, + "replaceMissingValueWith":"none", + "injective":false, + "optimize":false + }, + "value":"value" + } + """ + def setup() { objectMapper = new ObjectMapper() + dimension = Mock(LookupDimension) + extractionFunction = new RegisteredLookupExtractionFunction("lookup", false, "none", false, false) + TableDataSource dataSource = Mock(TableDataSource) + ConstrainedTable physicalTable = Mock(ConstrainedTable) + dataSource.getPhysicalTable() >> physicalTable + dataSource.getQuery() >> Optional.empty() + physicalTable.getPhysicalColumnName(_) >> "foo" + druidQuery = Mock(DruidAggregationQuery) + dimension.getExtractionFunction() >> Optional.of(extractionFunction) + + druidQuery.getDataSource() >> dataSource + + } + + def "Serialization of dimension with lookups defaulted is correct"() { + given: + SelectorFilter filter = new SelectorFilter(dimension,"value") + druidQuery.getFilter() >> filter + String serializedFilter = objectMapper.writeValueAsString(druidQuery) + + expect: + objectMapper.readTree(serializedFilter).get("filter") == objectMapper.readTree(expectedSerialization) } - def "Test dimensional filter serialize as expected by druid"() { + def "Serialization of dimension with lookups overriden is correct"() { given: - SelectorFilter filter = new SelectorFilter(null, "value", new RegisteredLookupExtractionFunction("lookup", false, "none", false, false)) - String serializedFilter = objectMapper.writeValueAsString(filter) + SelectorFilter filter = new SelectorFilter( + dimension, + "value", + new RegisteredLookupExtractionFunction("lookup2", false, "none", false, false) + ) + druidQuery.getFilter() >> filter + String serializedFilter = objectMapper.writeValueAsString(druidQuery) expect: - objectMapper.readTree(serializedFilter) == objectMapper.readTree(expectedSerialization) + objectMapper.readTree(serializedFilter).get("filter") == objectMapper.readTree(expectedSerialization2) } }