From 4dcf266929f68187b65d1551f3aca6532447536c Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 21 Jul 2016 00:27:53 -0700 Subject: [PATCH 01/18] Add numeric StringComparator --- .../druid/benchmark/BoundFilterBenchmark.java | 19 +- .../benchmark/FilterPartitionBenchmark.java | 16 +- .../FilteredAggregatorBenchmark.java | 3 +- .../IncrementalIndexReadBenchmark.java | 3 +- docs/content/querying/filters.md | 68 ++--- docs/content/querying/limitspec.md | 4 +- docs/content/querying/searchquery.md | 2 +- docs/content/querying/sorting-orders.md | 31 +++ docs/content/querying/topnmetricspec.md | 18 ++ docs/content/toc.md | 1 + .../io/druid/query/filter/BoundDimFilter.java | 159 ++++++++++-- .../query/ordering/StringComparators.java | 101 +++++++- .../search/search/NumericSearchSortSpec.java | 79 ++++++ .../topn/NumericDimensionTopNMetricSpec.java | 77 ++++++ .../io/druid/query/topn/TopNMetricSpec.java | 3 +- .../io/druid/segment/filter/BoundFilter.java | 61 +++-- .../aggregation/FilteredAggregatorTest.java | 9 +- .../query/filter/BoundDimFilterTest.java | 42 +-- .../filter/GetDimensionRangeSetTest.java | 17 +- .../query/groupby/GroupByQueryRunnerTest.java | 47 +++- .../query/ordering/StringComparatorsTest.java | 37 +++ .../search/NumericSearchSortSpecTest.java | 51 ++++ .../query/search/SearchQueryRunnerTest.java | 26 ++ .../timeseries/TimeseriesQueryRunnerTest.java | 10 +- .../druid/query/topn/TopNQueryRunnerTest.java | 33 +++ .../druid/segment/filter/BoundFilterTest.java | 239 ++++++++++++++---- .../segment/filter/LongFilteringTest.java | 9 +- .../segment/filter/TimeFilteringTest.java | 9 +- .../client/CachingClusteredClientTest.java | 25 +- 29 files changed, 992 insertions(+), 207 deletions(-) create mode 100644 docs/content/querying/sorting-orders.md create mode 100644 processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java create mode 100644 processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java create mode 100644 processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java diff --git a/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java index e62d507133f9..1d250693c5c4 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java @@ -32,6 +32,7 @@ import com.metamx.collections.spatial.ImmutableRTree; import io.druid.query.filter.BitmapIndexSelector; import io.druid.query.filter.BoundDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.segment.column.BitmapIndex; import io.druid.segment.data.BitmapSerdeFactory; import io.druid.segment.data.ConciseBitmapSerdeFactory; @@ -75,7 +76,8 @@ public class BoundFilterBenchmark true, false, false, - null + null, + StringComparators.LEXICOGRAPHIC_NAME ) ); @@ -87,7 +89,8 @@ public class BoundFilterBenchmark false, false, false, - null + null, + StringComparators.LEXICOGRAPHIC_NAME ) ); @@ -99,7 +102,8 @@ public class BoundFilterBenchmark false, false, false, - null + null, + StringComparators.LEXICOGRAPHIC_NAME ) ); @@ -111,7 +115,8 @@ public class BoundFilterBenchmark true, false, true, - null + null, + StringComparators.ALPHANUMERIC_NAME ) ); @@ -123,7 +128,8 @@ public class BoundFilterBenchmark false, false, true, - null + null, + StringComparators.ALPHANUMERIC_NAME ) ); @@ -135,7 +141,8 @@ public class BoundFilterBenchmark false, false, true, - null + null, + StringComparators.ALPHANUMERIC_NAME ) ); diff --git a/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java index 7169a12d10f3..a581777bca41 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java @@ -50,6 +50,7 @@ import io.druid.query.filter.Filter; import io.druid.query.filter.OrDimFilter; import io.druid.query.filter.SelectorDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.segment.Cursor; import io.druid.segment.DimensionSelector; import io.druid.segment.IndexIO; @@ -186,8 +187,9 @@ public void setup() throws IOException String.valueOf(Long.MAX_VALUE), true, true, - true, - null + null, + null, + StringComparators.ALPHANUMERIC_NAME )); long halfEnd = (interval.getEndMillis() + interval.getStartMillis()) / 2; @@ -197,8 +199,9 @@ public void setup() throws IOException String.valueOf(halfEnd), true, true, - true, - null + null, + null, + StringComparators.ALPHANUMERIC_NAME )); timeFilterAll = new BoundFilter(new BoundDimFilter( @@ -207,8 +210,9 @@ public void setup() throws IOException String.valueOf(interval.getEndMillis()), true, true, - true, - null + null, + null, + StringComparators.ALPHANUMERIC_NAME )); } diff --git a/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java index 4639da5fc84a..4c1a245d23bb 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java @@ -64,6 +64,7 @@ import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SearchQueryDimFilter; import io.druid.query.filter.SelectorDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.query.spec.QuerySegmentSpec; @@ -185,7 +186,7 @@ public void setup() throws IOException filter = new OrDimFilter( Arrays.asList( - new BoundDimFilter("dimSequential", "-1", "-1", true, true, true, null), + new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC_NAME), new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()), new RegexDimFilter("dimSequential", "X", null), new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null), diff --git a/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java index 1cd13a4027d7..027c616bf03b 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java @@ -41,6 +41,7 @@ import io.druid.query.filter.OrDimFilter; import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SearchQueryDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.segment.Cursor; import io.druid.segment.DimensionSelector; @@ -164,7 +165,7 @@ public void readWithFilters(Blackhole blackhole) throws Exception { DimFilter filter = new OrDimFilter( Arrays.asList( - new BoundDimFilter("dimSequential", "-1", "-1", true, true, true, null), + new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC_NAME), new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()), new RegexDimFilter("dimSequential", "X", null), new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null), diff --git a/docs/content/querying/filters.md b/docs/content/querying/filters.md index 30c9edda714d..68b04277c6d1 100644 --- a/docs/content/querying/filters.md +++ b/docs/content/querying/filters.md @@ -174,26 +174,33 @@ The IN filter supports the use of extraction functions, see [Filtering with Extr ### Bound filter -Bound filter can be used to filter by comparing dimension values to an upper value or/and a lower value. -By default Comparison is string based and **case sensitive**. -To use numeric comparison you can set `alphaNumeric` to `true`. -By default the bound filter is a not a strict inclusion `inputString <= upper && inputSting >= lower`. +The Bound filter can be used to filter by comparing dimension values to an upper value and/or a lower value. +|property|type|description|required?| +|--------|-----------|---------| +|type|String|This should always be "bound".|yes| +|dimension|String|The dimension to filter on|yes| +|lower|String|The lower bound for the filter|no| +|upper|String|The upper bound for the filter|no| +|lowerStrict|Boolean|Perform strict comparison on the lower bound ("<" instead of "<=")|no, default: false| +|upperStrict|Boolean|Perform strict comparison on the upper bound (">" instead of ">=")|no, default: false| +|ordering|String|Specifies the sorting order to use when comparing values against the bound.
Can be one of the following values: "lexicographic", "alphanumeric", "numeric", "strlen".
See [Sorting Orders](./sorting-orders.html) for more details.|no, default: "lexicographic"| +|extractionFn|[Extraction function](#filtering-with-extraction-functions)| Extraction function to apply to the dimension|no| + The bound filter supports the use of extraction functions, see [Filtering with Extraction Functions](#filtering-with-extraction-functions) for details. -The grammar for a bound filter is as follows: - +The following bound filter expresses the condition `21 <= age <= 31`: ```json { "type": "bound", "dimension": "age", "lower": "21", "upper": "31" , - "alphaNumeric": true + "ordering": "numeric" } ``` -Equivalent to retain column if `21 <= age <= 31` +This filter expresses the condition `foo <= name <= hoo`, using the default lexicographic sorting order. ```json { "type": "bound", @@ -203,12 +210,7 @@ Equivalent to retain column if `21 <= age <= 31` } ``` -Equivalent to retain column if `foo <= name <= hoo` - -In order to have a strict inclusion user can set `lowerStrict` or/and `upperStrict` to `true` - -To have strict bounds: - +Using strict bounds, this filter expresses the condition `21 < age < 31` ```json { "type": "bound", @@ -217,59 +219,31 @@ To have strict bounds: "lowerStrict": true, "upper": "31" , "upperStrict": true, - "alphaNumeric": true + "ordering": "numeric" } ``` -Equivalent to retain column if `21 < age < 31` - -To have strict upper bound: +The user can also specify a one-sided bound by omitting "upper" or "lower". This filter expresses `age < 31`. ```json { "type": "bound", "dimension": "age", - "lower": "21", "upper": "31" , "upperStrict": true, - "alphaNumeric": true + "ordering": "numeric" } ``` -Equivalent to retain column if `21 <= age < 31` - -To compare to only an upper bound or lowe bound - -```json -{ - "type": "bound", - "dimension": "age", - "upper": "31" , - "upperStrict": true, - "alphaNumeric": true -} -``` - -Equivalent to retain column if `age < 31` - +Likewise, this filter expresses `age >= 18` ```json { "type": "bound", "dimension": "age", "lower": "18" , - "alphaNumeric": true + "ordering": "numeric" } ``` -Equivalent to retain column if ` 18 <= age` - -For `alphaNumeric` comparator, in case of the dimension value includes none-digits you may expect **fuzzy matching** -If dimension value starts with a none digit, the filter will consider it out of range (`value < lowerBound` and `value > upperBound`) -If dimension value starts with digit and contains a none digits comparing will be done character wise. -For instance suppose lower bound is `100` and value is `10K` the filter will match (`100 < 10K` returns `true`) since `K` is greater than any digit -Now suppose that the lower bound is `110` the filter will not match (`110 < 10K` returns `false`) - - - #### Search Query Spec diff --git a/docs/content/querying/limitspec.md b/docs/content/querying/limitspec.md index ca129172a97a..0d65f95475de 100644 --- a/docs/content/querying/limitspec.md +++ b/docs/content/querying/limitspec.md @@ -24,8 +24,10 @@ OrderByColumnSpecs indicate how to do order by operations. Each order-by conditi { "dimension" : "", "direction" : <"ascending"|"descending">, - "dimensionOrder" : <"lexicographic(default)"|"alphanumeric"|"strlen"> + "dimensionOrder" : <"lexicographic"(default)|"alphanumeric"|"strlen"|"numeric"> } ``` If only the dimension is provided (as a JSON string), the default order-by is ascending. + +See [Sorting Orders](./sorting-orders.html) for more information on the sorting orders specified by "dimensionOrder". diff --git a/docs/content/querying/searchquery.md b/docs/content/querying/searchquery.md index 5be18b238772..7ebca672bfc8 100644 --- a/docs/content/querying/searchquery.md +++ b/docs/content/querying/searchquery.md @@ -38,7 +38,7 @@ There are several main parts to a search query: |intervals|A JSON Object representing ISO-8601 Intervals. This defines the time ranges to run the query over.|yes| |searchDimensions|The dimensions to run the search over. Excluding this means the search is run over all dimensions.|no| |query|See [SearchQuerySpec](../querying/searchqueryspec.html).|yes| -|sort|An object specifying how the results of the search should be sorted. Possible types here are "lexicographic" (the default sort), "alphanumeric" and "strlen".|no| +|sort|An object specifying how the results of the search should be sorted.
Possible types are "lexicographic" (the default sort), "alphanumeric", "strlen", and "numeric".
See [Sorting Orders](./sorting-orders.html) for more details.|no| |context|See [Context](../querying/query-context.html)|no| The format of the result is: diff --git a/docs/content/querying/sorting-orders.md b/docs/content/querying/sorting-orders.md new file mode 100644 index 000000000000..ebff7da7b8f6 --- /dev/null +++ b/docs/content/querying/sorting-orders.md @@ -0,0 +1,31 @@ +--- +layout: doc_page +--- + +# Sorting Orders + +These sorting orders are used by the [TopNMetricSpec](./topnmetricspec.html), [SearchQuery](./searchquery.html), GroupByQuery's [LimitSpec](./limitspec.html), and [BoundFilter](./filters.html#bound-filter). + +## Lexicographic +Sorts values in case-sensitive lexicographic order. + +## Alphanumeric +Suitable for strings with both numeric and non-numeric content, e.g.: + +"file12 sorts after file2" + +See https://github.com/amjjd/java-alphanum for more details on how this ordering sorts values. + +This ordering is not suitable for numbers with decimal points or negative numbers. +* For example, "1.3" precedes "1.15" in this ordering because "15" has more significant digits than "3". +* Negative numbers are sorted after positive numbers (because numeric characters precede the "-" in the negative numbers). + +## Numeric +Sorts values as numbers, supports integers and floating point values. Negative values are supported. + +This sorting order will try to parse all string values as numbers. Unparseable values are treated as nulls, and nulls precede numbers. + +When comparing two unparseable values (e.g., "hello" and "world"), this ordering will sort by comparing the unparsed strings lexicographically. + +## Strlen +Sorts values by the their string lengths. diff --git a/docs/content/querying/topnmetricspec.md b/docs/content/querying/topnmetricspec.md index 94251e049546..e193775f6355 100644 --- a/docs/content/querying/topnmetricspec.md +++ b/docs/content/querying/topnmetricspec.md @@ -6,6 +6,8 @@ TopNMetricSpec The topN metric spec specifies how topN values should be sorted. +See [Sorting Orders](./sorting-orders.html) for more information on the sorting orders used by the Lexicographic, Alphanumeric, and NumericDimension specs. + ## Numeric TopNMetricSpec The simplest metric specification is a String value indicating the metric to sort topN results by. They are included in a topN query with: @@ -76,3 +78,19 @@ Sort dimension values in inverted order, i.e inverts the order of the delegate m |--------|-----------|---------| |type|this indicates an inverted sort|yes| |metric|the delegate metric spec. |yes| + +## NumericDimension TopNMetricSpec + +Sort dimension values in numeric order, i.e treating dimension values as numeric values. Unparseable string values will be sorted before valid numeric values. If two string values are unparseable, they will be compared lexicographically. + +```json +"metric": { + "type": "numericDimension", + "previousStop": "" +} +``` + +|property|description|required?| +|--------|-----------|---------| +|type|this indicates an alpha-numeric sort|yes| +|previousStop|the starting point of the alpha-numeric sort. For example, if a previousStop value is 'b', all values before 'b' are discarded. This field can be used to paginate through all the dimension values.|no| \ No newline at end of file diff --git a/docs/content/toc.md b/docs/content/toc.md index c76fa1ac7c93..0c063be8960b 100644 --- a/docs/content/toc.md +++ b/docs/content/toc.md @@ -47,6 +47,7 @@ layout: toc * [Joins](/docs/VERSION/querying/joins.html) * [Multitenancy](/docs/VERSION/querying/multitenancy.html) * [Caching](/docs/VERSION/querying/caching.html) + * [Sorting Orders](/docs/VERSION/querying/sorting-orders.html) ## Design * [Overview](/docs/VERSION/design/design.html) diff --git a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java index 8296a1f045ed..a02a63e7929d 100644 --- a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java @@ -20,14 +20,19 @@ package io.druid.query.filter; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.common.base.Supplier; import com.google.common.collect.BoundType; import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; +import com.google.common.primitives.Longs; import com.metamx.common.StringUtils; import io.druid.query.extraction.ExtractionFn; +import io.druid.query.ordering.StringComparators; import io.druid.segment.filter.BoundFilter; import java.nio.ByteBuffer; @@ -40,8 +45,10 @@ public class BoundDimFilter implements DimFilter private final String lower; private final boolean lowerStrict; private final boolean upperStrict; - private final boolean alphaNumeric; + private final Boolean alphaNumeric; private final ExtractionFn extractionFn; + private final String ordering; + private final Supplier longPredicateSupplier; @JsonCreator public BoundDimFilter( @@ -50,8 +57,9 @@ public BoundDimFilter( @JsonProperty("upper") String upper, @JsonProperty("lowerStrict") Boolean lowerStrict, @JsonProperty("upperStrict") Boolean upperStrict, - @JsonProperty("alphaNumeric") Boolean alphaNumeric, - @JsonProperty("extractionFn") ExtractionFn extractionFn + @Deprecated @JsonProperty("alphaNumeric") Boolean alphaNumeric, + @JsonProperty("extractionFn") ExtractionFn extractionFn, + @JsonProperty("ordering") String ordering ) { this.dimension = Preconditions.checkNotNull(dimension, "dimension can not be null"); @@ -60,8 +68,31 @@ public BoundDimFilter( this.lower = lower; this.lowerStrict = (lowerStrict == null) ? false : lowerStrict; this.upperStrict = (upperStrict == null) ? false : upperStrict; - this.alphaNumeric = (alphaNumeric == null) ? false : alphaNumeric; + this.alphaNumeric = alphaNumeric; + + if (ordering == null) { + if (alphaNumeric == null || !alphaNumeric) { + this.ordering = StringComparators.LEXICOGRAPHIC_NAME; + } else { + this.ordering = StringComparators.ALPHANUMERIC_NAME; + } + } else { + this.ordering = ordering.toLowerCase(); + Preconditions.checkState( + StringComparators.isOrderingValid(this.ordering), + "ordering must be one of the following: " + StringComparators.ORDERINGS.toString() + ); + + if (alphaNumeric != null) { + boolean orderingIsAlphanumeric = this.ordering.equals(StringComparators.ALPHANUMERIC_NAME); + Preconditions.checkState( + this.alphaNumeric == orderingIsAlphanumeric, + "mismatch between alphanumeric and ordering property" + ); + } + } this.extractionFn = extractionFn; + this.longPredicateSupplier = makeLongPredicateSupplier(); } @JsonProperty @@ -94,8 +125,9 @@ public boolean isUpperStrict() return upperStrict; } - @JsonProperty - public boolean isAlphaNumeric() + @Deprecated + @JsonIgnore + public Boolean isAlphaNumeric() { return alphaNumeric; } @@ -116,6 +148,17 @@ public ExtractionFn getExtractionFn() return extractionFn; } + @JsonProperty + public String getOrdering() + { + return ordering; + } + + public Supplier getLongPredicateSupplier() + { + return longPredicateSupplier; + } + @Override public byte[] getCacheKey() { @@ -135,12 +178,15 @@ public byte[] getCacheKey() byte[] extractionFnBytes = extractionFn == null ? new byte[0] : extractionFn.getCacheKey(); + byte[] orderingBytes = StringUtils.toUtf8(ordering); + ByteBuffer boundCacheBuffer = ByteBuffer.allocate( - 9 + 10 + dimensionBytes.length + upperBytes.length + lowerBytes.length + extractionFnBytes.length + + orderingBytes.length ); boundCacheBuffer.put(DimFilterUtils.BOUND_CACHE_ID) .put(boundType) @@ -154,7 +200,9 @@ public byte[] getCacheKey() .put(DimFilterUtils.STRING_SEPARATOR) .put(lowerBytes) .put(DimFilterUtils.STRING_SEPARATOR) - .put(extractionFnBytes); + .put(extractionFnBytes) + .put(DimFilterUtils.STRING_SEPARATOR) + .put(orderingBytes); return boundCacheBuffer.array(); } @@ -208,9 +256,6 @@ public boolean equals(Object o) if (isUpperStrict() != that.isUpperStrict()) { return false; } - if (isAlphaNumeric() != that.isAlphaNumeric()) { - return false; - } if (!getDimension().equals(that.getDimension())) { return false; } @@ -220,9 +265,12 @@ public boolean equals(Object o) if (getLower() != null ? !getLower().equals(that.getLower()) : that.getLower() != null) { return false; } - return getExtractionFn() != null - ? getExtractionFn().equals(that.getExtractionFn()) - : that.getExtractionFn() == null; + if (getExtractionFn() != null + ? !getExtractionFn().equals(that.getExtractionFn()) + : that.getExtractionFn() != null) { + return false; + } + return getOrdering().equals(that.getOrdering()); } @@ -234,8 +282,89 @@ public int hashCode() result = 31 * result + (getLower() != null ? getLower().hashCode() : 0); result = 31 * result + (isLowerStrict() ? 1 : 0); result = 31 * result + (isUpperStrict() ? 1 : 0); - result = 31 * result + (isAlphaNumeric() ? 1 : 0); result = 31 * result + (getExtractionFn() != null ? getExtractionFn().hashCode() : 0); + result = 31 * result + getOrdering().hashCode(); return result; } + + private Supplier makeLongPredicateSupplier() + { + return new Supplier() + { + private final Object initLock = new Object(); + private volatile boolean longsInitialized = false; + private volatile boolean hasLowerLongBoundVolatile; + private volatile boolean hasUpperLongBoundVolatile; + private volatile long lowerLongBoundVolatile; + private volatile long upperLongBoundVolatile; + + @Override + public DruidLongPredicate get() + { + initLongData(); + + return new DruidLongPredicate() + { + private final boolean hasLowerLongBound = hasLowerLongBoundVolatile; + private final boolean hasUpperLongBound = hasUpperLongBoundVolatile; + private final long lowerLongBound = hasLowerLongBound ? lowerLongBoundVolatile : 0L; + private final long upperLongBound = hasUpperLongBound ? upperLongBoundVolatile : 0L; + + @Override + public boolean applyLong(long input) + { + int lowerComparing = 1; + int upperComparing = 1; + if (hasLowerLongBound) { + lowerComparing = Long.compare(input, lowerLongBound); + } + if (hasUpperLongBound) { + upperComparing = Long.compare(upperLongBound, input); + } + if (lowerStrict && upperStrict) { + return ((lowerComparing > 0)) && (upperComparing > 0); + } else if (lowerStrict) { + return (lowerComparing > 0) && (upperComparing >= 0); + } else if (upperStrict) { + return (lowerComparing >= 0) && (upperComparing > 0); + } + return (lowerComparing >= 0) && (upperComparing >= 0); + } + }; + } + + private void initLongData() + { + if (longsInitialized) { + return; + } + + synchronized (initLock) { + if (longsInitialized) { + return; + } + + Long lowerLong = Longs.tryParse(Strings.nullToEmpty(lower)); + if (hasLowerBound() && lowerLong != null) { + hasLowerLongBoundVolatile = true; + lowerLongBoundVolatile = lowerLong; + } else { + hasLowerLongBoundVolatile = false; + } + + Long upperLong = Longs.tryParse(Strings.nullToEmpty(upper)); + if (hasUpperBound() && upperLong != null) { + hasUpperLongBoundVolatile = true; + upperLongBoundVolatile = upperLong; + } else { + hasUpperLongBoundVolatile = false; + } + + longsInitialized = true; + } + } + }; + } + + } diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparators.java b/processing/src/main/java/io/druid/query/ordering/StringComparators.java index 909eb4856649..1636e72e83ac 100644 --- a/processing/src/main/java/io/druid/query/ordering/StringComparators.java +++ b/processing/src/main/java/io/druid/query/ordering/StringComparators.java @@ -19,12 +19,16 @@ package io.druid.query.ordering; +import java.math.BigDecimal; +import java.util.Arrays; import java.util.Comparator; +import java.util.List; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonTypeInfo.As; import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Ordering; import com.google.common.primitives.Ints; import com.google.common.primitives.UnsignedBytes; @@ -36,22 +40,29 @@ public class StringComparators { public static final String LEXICOGRAPHIC_NAME = "lexicographic"; public static final String ALPHANUMERIC_NAME = "alphanumeric"; + public static final String NUMERIC_NAME = "numeric"; public static final String STRLEN_NAME = "strlen"; + public static final List ORDERINGS = ImmutableList.of( + LEXICOGRAPHIC_NAME, ALPHANUMERIC_NAME, NUMERIC_NAME, STRLEN_NAME + ); + public static final LexicographicComparator LEXICOGRAPHIC = new LexicographicComparator(); public static final AlphanumericComparator ALPHANUMERIC = new AlphanumericComparator(); + public static final NumericComparator NUMERIC = new NumericComparator(); public static final StrlenComparator STRLEN = new StrlenComparator(); @JsonTypeInfo(use=Id.NAME, include=As.PROPERTY, property="type", defaultImpl = LexicographicComparator.class) @JsonSubTypes(value = { @JsonSubTypes.Type(name = StringComparators.LEXICOGRAPHIC_NAME, value = LexicographicComparator.class), @JsonSubTypes.Type(name = StringComparators.ALPHANUMERIC_NAME, value = AlphanumericComparator.class), - @JsonSubTypes.Type(name = StringComparators.STRLEN_NAME, value = StrlenComparator.class) + @JsonSubTypes.Type(name = StringComparators.STRLEN_NAME, value = StrlenComparator.class), + @JsonSubTypes.Type(name = StringComparators.NUMERIC_NAME, value = NumericComparator.class), }) public static interface StringComparator extends Comparator { } - + public static class LexicographicComparator implements StringComparator { private static final Ordering ORDERING = Ordering.from(new Comparator() @@ -342,16 +353,86 @@ public String toString() } } + private static BigDecimal convertStringToBigDecimal(String input) { + // treat unparseable Strings as nulls + BigDecimal bd = null; + try { + bd = new BigDecimal(input); + } catch (NullPointerException | NumberFormatException ex) { + } + return bd; + } + + public static class NumericComparator implements StringComparator + { + @Override + public int compare(String o1, String o2) + { + if (o1 == o2) { + return 0; + } + + BigDecimal bd1 = convertStringToBigDecimal(o1); + BigDecimal bd2 = convertStringToBigDecimal(o2); + + if (bd1 != null && bd2 != null) { + return bd1.compareTo(bd2); + } + + if (bd1 == null && bd2 == null) { + // both Strings are unparseable, just compare lexicographically to have a well-defined ordering + return StringComparators.LEXICOGRAPHIC.compare(o1, o2); + } + + if (bd1 == null) { + return -1; + } else { + return 1; + } + } + + @Override + public String toString() + { + return StringComparators.NUMERIC_NAME; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + return true; + } + } + public static StringComparator makeComparator(String type) { - if (type.equals(StringComparators.LEXICOGRAPHIC_NAME)) { - return LEXICOGRAPHIC; - } else if (type.equals(StringComparators.ALPHANUMERIC_NAME)) { - return ALPHANUMERIC; - } else if (type.equals(StringComparators.STRLEN_NAME)) { - return STRLEN; - } else { - throw new IAE("Unknown string comparator[%s]", type); + switch (type) { + case StringComparators.LEXICOGRAPHIC_NAME: + return LEXICOGRAPHIC; + case StringComparators.ALPHANUMERIC_NAME: + return ALPHANUMERIC; + case StringComparators.STRLEN_NAME: + return STRLEN; + case StringComparators.NUMERIC_NAME: + return NUMERIC; + default: + throw new IAE("Unknown string comparator[%s]", type); } } + + public static boolean isOrderingValid(String ordering) + { + if (ordering == null) { + return false; + } + + return ORDERINGS.contains(ordering); + } } diff --git a/processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java new file mode 100644 index 000000000000..2f19294f8ea1 --- /dev/null +++ b/processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java @@ -0,0 +1,79 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; + +import com.fasterxml.jackson.annotation.JsonCreator; + +import io.druid.query.ordering.StringComparators; + +import java.util.Comparator; + +/** + */ +public class NumericSearchSortSpec implements SearchSortSpec +{ + @JsonCreator + public NumericSearchSortSpec( + ) + { + } + + @Override + public Comparator getComparator() + { + return new Comparator() + { + @Override + public int compare(SearchHit searchHit, SearchHit searchHit1) + { + int retVal = StringComparators.NUMERIC.compare( + searchHit.getValue(), searchHit1.getValue()); + + if (retVal == 0) { + retVal = StringComparators.LEXICOGRAPHIC.compare( + searchHit.getDimension(), searchHit1.getDimension()); + } + return retVal; + } + }; + } + + @Override + public byte[] getCacheKey() + { + return toString().getBytes(); + } + + public String toString() + { + return "numericSort"; + } + + @Override + public boolean equals(Object other) { + return this == other || other instanceof NumericSearchSortSpec; + } + + @Override + public int hashCode() + { + return 0; + } +} diff --git a/processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java new file mode 100644 index 000000000000..8d0250c5356a --- /dev/null +++ b/processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java @@ -0,0 +1,77 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.topn; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.metamx.common.StringUtils; +import io.druid.query.aggregation.AggregatorFactory; +import io.druid.query.aggregation.PostAggregator; +import io.druid.query.ordering.StringComparators; + +import java.nio.ByteBuffer; +import java.util.Comparator; +import java.util.List; + +public class NumericDimensionTopNMetricSpec extends LexicographicTopNMetricSpec +{ + private static final byte CACHE_TYPE_ID = 0x4; + + protected static Comparator comparator = StringComparators.NUMERIC; + + @JsonCreator + public NumericDimensionTopNMetricSpec( + @JsonProperty("previousStop") String previousStop + ) + { + super(previousStop); + } + + @Override + public Comparator getComparator(List aggregatorSpecs, List postAggregatorSpecs) + { + return comparator; + } + + @Override + public byte[] getCacheKey() + { + byte[] previousStopBytes = getPreviousStop() == null ? new byte[]{} : StringUtils.toUtf8(getPreviousStop()); + + return ByteBuffer.allocate(1 + previousStopBytes.length) + .put(CACHE_TYPE_ID) + .put(previousStopBytes) + .array(); + } + + @Override + public TopNMetricSpecBuilder configureOptimizer(TopNMetricSpecBuilder builder) + { + return builder; + } + + @Override + public String toString() + { + return "NumericDimensionTopNMetricSpec{" + + "previousStop='" + getPreviousStop() + '\'' + + '}'; + } +} diff --git a/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java index c0e72b2c92d8..4f428c547b7b 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java +++ b/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java @@ -36,7 +36,8 @@ @JsonSubTypes.Type(name = "numeric", value = NumericTopNMetricSpec.class), @JsonSubTypes.Type(name = "lexicographic", value = LexicographicTopNMetricSpec.class), @JsonSubTypes.Type(name = "alphaNumeric", value = AlphaNumericTopNMetricSpec.class), - @JsonSubTypes.Type(name = "inverted", value = InvertedTopNMetricSpec.class) + @JsonSubTypes.Type(name = "inverted", value = InvertedTopNMetricSpec.class), + @JsonSubTypes.Type(name = "numericDimension", value = NumericDimensionTopNMetricSpec.class), }) public interface TopNMetricSpec { diff --git a/processing/src/main/java/io/druid/segment/filter/BoundFilter.java b/processing/src/main/java/io/druid/segment/filter/BoundFilter.java index 107419cf1bf8..e078c635f8bf 100644 --- a/processing/src/main/java/io/druid/segment/filter/BoundFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/BoundFilter.java @@ -20,6 +20,9 @@ package io.druid.segment.filter; import com.google.common.base.Predicate; +import com.google.common.base.Strings; +import com.google.common.base.Supplier; +import com.google.common.primitives.Longs; import com.metamx.collections.bitmap.ImmutableBitmap; import io.druid.query.extraction.ExtractionFn; import io.druid.query.filter.BitmapIndexSelector; @@ -41,19 +44,20 @@ public class BoundFilter implements Filter private final Comparator comparator; private final ExtractionFn extractionFn; + private final Supplier longPredicateSupplier; + public BoundFilter(final BoundDimFilter boundDimFilter) { this.boundDimFilter = boundDimFilter; - this.comparator = boundDimFilter.isAlphaNumeric() - ? StringComparators.ALPHANUMERIC - : StringComparators.LEXICOGRAPHIC; + this.comparator = StringComparators.makeComparator(boundDimFilter.getOrdering()); this.extractionFn = boundDimFilter.getExtractionFn(); + this.longPredicateSupplier = boundDimFilter.getLongPredicateSupplier(); } @Override public ImmutableBitmap getBitmapIndex(final BitmapIndexSelector selector) { - if (boundDimFilter.isAlphaNumeric() || extractionFn != null) { + if (!boundDimFilter.getOrdering().equals(StringComparators.LEXICOGRAPHIC_NAME) || extractionFn != null) { return Filters.matchPredicate( boundDimFilter.getDimension(), selector, @@ -151,39 +155,48 @@ private DruidPredicateFactory getPredicateFactory() @Override public Predicate makeStringPredicate() { - return new Predicate() - { - @Override - public boolean apply(String input) + if (extractionFn != null) { + return new Predicate() { - return doesMatch(input); - } - }; + @Override + public boolean apply(String input) + { + return doesMatch(extractionFn.apply(input)); + } + }; + } else { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return doesMatch(input); + } + }; + } } @Override public DruidLongPredicate makeLongPredicate() { - return new DruidLongPredicate() - { - @Override - public boolean applyLong(long input) + if (extractionFn != null) { + return new DruidLongPredicate() { - // When BoundFilter has a 'numeric' comparator (see https://github.com/druid-io/druid/issues/2989) - // this should be optimized to compare on longs instead of using string conversion. - return doesMatch(String.valueOf(input)); - } - }; + @Override + public boolean applyLong(long input) + { + return doesMatch(extractionFn.apply(input)); + } + }; + } else { + return longPredicateSupplier.get(); + } } }; } private boolean doesMatch(String input) { - if (extractionFn != null) { - input = extractionFn.apply(input); - } - if (input == null) { return (!boundDimFilter.hasLowerBound() || (boundDimFilter.getLower().isEmpty() && !boundDimFilter.isLowerStrict())) // lower bound allows null diff --git a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java index 11c0a6fb28fd..45d001deb334 100644 --- a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java @@ -34,6 +34,7 @@ import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SearchQueryDimFilter; import io.druid.query.filter.SelectorDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; @@ -257,7 +258,7 @@ public void testAggregateWithPredicateFilters() factory = new FilteredAggregatorFactory( new DoubleSumAggregatorFactory("billy", "value"), - new BoundDimFilter("dim", "a", "a", false, false, true, null) + new BoundDimFilter("dim", "a", "a", false, false, true, null, StringComparators.ALPHANUMERIC_NAME) ); selector = new TestFloatColumnSelector(values); validateFilteredAggs(factory, values, selector); @@ -308,10 +309,12 @@ public void testAggregateWithExtractionFns() ); selector = new TestFloatColumnSelector(values); validateFilteredAggs(factory, values, selector); - + factory = new FilteredAggregatorFactory( new DoubleSumAggregatorFactory("billy", "value"), - new BoundDimFilter("dim", "aAARDVARK", "aAARDVARK", false, false, true, extractionFn) + new BoundDimFilter("dim", "aAARDVARK", "aAARDVARK", false, false, true, extractionFn, + StringComparators.ALPHANUMERIC_NAME + ) ); selector = new TestFloatColumnSelector(values); validateFilteredAggs(factory, values, selector); diff --git a/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java b/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java index 1f99d084fb48..2255aaf09e5e 100644 --- a/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java +++ b/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java @@ -29,6 +29,7 @@ import io.druid.guice.annotations.Json; import io.druid.query.extraction.ExtractionFn; import io.druid.query.extraction.RegexDimExtractionFn; +import io.druid.query.ordering.StringComparators; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -50,15 +51,24 @@ public class BoundDimFilterTest public static Iterable constructorFeeder(){ return ImmutableList.of( - new Object[]{new BoundDimFilter("dimension", "12", "15", null, null, null, null)}, - new Object[]{new BoundDimFilter("dimension", "12", "15", null, true, false, null)}, - new Object[]{new BoundDimFilter("dimension", "12", "15", null, null, true, null)}, - new Object[]{new BoundDimFilter("dimension", null, "15", null, true, true, null)}, - new Object[]{new BoundDimFilter("dimension", "12", "15", true, null, null, null)}, - new Object[]{new BoundDimFilter("dimension", "12", null, true, null, true, null)}, - new Object[]{new BoundDimFilter("dimension", "12", "15", true, true, true, null)}, - new Object[]{new BoundDimFilter("dimension", "12", "15", true, true, false, null)}, - new Object[]{new BoundDimFilter("dimension", null, "15", null, true, true, extractionFn)} + new Object[]{new BoundDimFilter("dimension", "12", "15", null, null, null, null, + StringComparators.LEXICOGRAPHIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", "12", "15", null, true, false, null, + StringComparators.LEXICOGRAPHIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", "12", "15", null, null, true, null, + StringComparators.ALPHANUMERIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", null, "15", null, true, true, null, + StringComparators.ALPHANUMERIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", "12", "15", true, null, null, null, + StringComparators.LEXICOGRAPHIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", "12", null, true, null, true, null, + StringComparators.ALPHANUMERIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", "12", "15", true, true, true, null, + StringComparators.ALPHANUMERIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", "12", "15", true, true, false, null, + StringComparators.LEXICOGRAPHIC_NAME)}, + new Object[]{new BoundDimFilter("dimension", null, "15", null, true, true, extractionFn, + StringComparators.ALPHANUMERIC_NAME)} ); } @@ -75,14 +85,14 @@ public void testSerDesBoundFilter() throws IOException @Test public void testGetCacheKey() { - BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null); - BoundDimFilter boundDimFilterCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, null); + BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null, StringComparators.ALPHANUMERIC_NAME); + BoundDimFilter boundDimFilterCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, null, StringComparators.ALPHANUMERIC_NAME); Assert.assertArrayEquals(boundDimFilter.getCacheKey(), boundDimFilterCopy.getCacheKey()); - BoundDimFilter anotherBoundDimFilter = new BoundDimFilter("dimension", "12", "15", true, null, false, null); + BoundDimFilter anotherBoundDimFilter = new BoundDimFilter("dimension", "12", "15", true, null, false, null, StringComparators.LEXICOGRAPHIC_NAME); Assert.assertFalse(Arrays.equals(anotherBoundDimFilter.getCacheKey(), boundDimFilter.getCacheKey())); - BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn); - BoundDimFilter boundDimFilterWithExtractCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, extractionFn); + BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn, StringComparators.ALPHANUMERIC_NAME); + BoundDimFilter boundDimFilterWithExtractCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, extractionFn, StringComparators.ALPHANUMERIC_NAME); Assert.assertFalse(Arrays.equals(boundDimFilter.getCacheKey(), boundDimFilterWithExtract.getCacheKey())); Assert.assertArrayEquals(boundDimFilterWithExtract.getCacheKey(), boundDimFilterWithExtractCopy.getCacheKey()); } @@ -90,8 +100,8 @@ public void testGetCacheKey() @Test public void testHashCode() { - BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null); - BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn); + BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null, StringComparators.ALPHANUMERIC_NAME); + BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn, StringComparators.ALPHANUMERIC_NAME); Assert.assertNotEquals(boundDimFilter.hashCode(), boundDimFilterWithExtract.hashCode()); } diff --git a/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java b/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java index c1a1545fc9d8..be82c76881bb 100644 --- a/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java +++ b/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.RangeSet; import io.druid.js.JavaScriptConfig; import io.druid.query.extraction.IdentityExtractionFn; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.ContainsSearchQuerySpec; import org.junit.Assert; import org.junit.Test; @@ -43,10 +44,18 @@ public class GetDimensionRangeSetTest private final DimFilter in1 = new InDimFilter("dim1", ImmutableList.of("testing", "this", "filter", "tillend"), null); private final DimFilter in2 = new InDimFilter("dim2", ImmutableList.of("again"), null); private final DimFilter in3 = new InDimFilter("dim1", Arrays.asList("null", null), null); - private final DimFilter bound1 = new BoundDimFilter("dim1", "from", "to", false, false, false, null); - private final DimFilter bound2 = new BoundDimFilter("dim1", null, "tillend", false, false, false, null); - private final DimFilter bound3 = new BoundDimFilter("dim1", "notincluded", null, true, false, false, null); - private final DimFilter bound4 = new BoundDimFilter("dim2", "again", "exclusive", true, true, false, null); + private final DimFilter bound1 = new BoundDimFilter("dim1", "from", "to", false, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ); + private final DimFilter bound2 = new BoundDimFilter("dim1", null, "tillend", false, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ); + private final DimFilter bound3 = new BoundDimFilter("dim1", "notincluded", null, true, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ); + private final DimFilter bound4 = new BoundDimFilter("dim2", "again", "exclusive", true, true, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ); private final DimFilter other1 = new RegexDimFilter("someDim", "pattern", null); private final DimFilter other2 = new JavaScriptDimFilter("someOtherDim", "function(x) { return x }", null, JavaScriptConfig.getDefault()); diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java index 8322bbbe42cd..6fad02de1cd8 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java @@ -2218,6 +2218,46 @@ public void testGroupByWithOrderLimit3() throws Exception ); } + @Test + public void testGroupByOrderLimitNumeric() throws Exception + { + GroupByQuery.Builder builder = GroupByQuery + .builder() + .setDataSource(QueryRunnerTestHelper.dataSource) + .setInterval("2011-04-02/2011-04-04") + .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) + .setAggregatorSpecs( + Arrays.asList( + QueryRunnerTestHelper.rowsCount, + new LongSumAggregatorFactory("idx", "index") + ) + ) + .addOrderByColumn(new OrderByColumnSpec("rows", OrderByColumnSpec.Direction.DESCENDING, StringComparators.NUMERIC)) + .addOrderByColumn(new OrderByColumnSpec("alias", OrderByColumnSpec.Direction.ASCENDING, StringComparators.NUMERIC)) + .setGranularity(new PeriodGranularity(new Period("P1M"), null, null)); + + final GroupByQuery query = builder.build(); + + List expectedResults = Arrays.asList( + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "mezzanine", "rows", 6L, "idx", 4420L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "premium", "rows", 6L, "idx", 4416L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "automotive", "rows", 2L, "idx", 269L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "business", "rows", 2L, "idx", 217L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "entertainment", "rows", 2L, "idx", 319L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "health", "rows", 2L, "idx", 216L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "news", "rows", 2L, "idx", 221L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "technology", "rows", 2L, "idx", 177L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "travel", "rows", 2L, "idx", 243L) + ); + + Map context = Maps.newHashMap(); + QueryRunner mergeRunner = factory.getToolchest().mergeResults(runner); + TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit"); + TestHelper.assertExpectedObjects( + Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited" + ); + } + @Test public void testGroupByWithSameCaseOrdering() { @@ -5476,7 +5516,8 @@ public void testBySegmentResultsWithAllFiltersWithExtractionFns() false, false, true, - extractionFn + extractionFn, + StringComparators.ALPHANUMERIC_NAME )); superFilterList.add(new RegexDimFilter("quality", "super-mezzanine", extractionFn)); superFilterList.add(new SearchQueryDimFilter( @@ -5534,7 +5575,9 @@ public void testGroupByWithAllFiltersOnNullDimsWithExtractionFns() List superFilterList = new ArrayList<>(); superFilterList.add(new SelectorDimFilter("null_column", "EMPTY", extractionFn)); superFilterList.add(new InDimFilter("null_column", Arrays.asList("NOT-EMPTY", "FOOBAR", "EMPTY"), extractionFn)); - superFilterList.add(new BoundDimFilter("null_column", "EMPTY", "EMPTY", false, false, true, extractionFn)); + superFilterList.add(new BoundDimFilter("null_column", "EMPTY", "EMPTY", false, false, true, extractionFn, + StringComparators.ALPHANUMERIC_NAME + )); superFilterList.add(new RegexDimFilter("null_column", "EMPTY", extractionFn)); superFilterList.add(new SearchQueryDimFilter( "null_column", diff --git a/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java b/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java index 857a73182049..e282f013b4f0 100644 --- a/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java +++ b/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java @@ -20,6 +20,7 @@ package io.druid.query.ordering; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -119,6 +120,30 @@ public void testStrlenComparator() Assert.assertTrue(StringComparators.STRLEN.compare("apple", "elppa") < 0); } + @Test + public void testNumericComparator() + { + commonTest(StringComparators.NUMERIC); + + Assert.assertTrue(StringComparators.NUMERIC.compare("-1230.452487532", "6893") < 0); + + List values = Arrays.asList("-1", "-1.10", "-1.2", "-100", "-2", "0", "1", "1.10", "1.2", "2", "100"); + Collections.sort(values, StringComparators.NUMERIC); + + Assert.assertEquals( + Arrays.asList("-100", "-2", "-1.2", "-1.10", "-1", "0", "1", "1.10", "1.2", "2", "100"), + values + ); + + Assert.assertTrue(StringComparators.NUMERIC.compare(null, "1001") < 0); + Assert.assertTrue(StringComparators.NUMERIC.compare("1001", null) > 0); + + Assert.assertTrue(StringComparators.NUMERIC.compare("-500000000.14124", "CAN'T TOUCH THIS") > 0); + Assert.assertTrue(StringComparators.NUMERIC.compare("CAN'T PARSE THIS", "-500000000.14124") < 0); + + Assert.assertTrue(StringComparators.NUMERIC.compare("CAN'T PARSE THIS", "CAN'T TOUCH THIS") < 0); + } + @Test public void testLexicographicComparatorSerdeTest() throws IOException { @@ -154,4 +179,16 @@ public void testStrlenComparatorSerdeTest() throws IOException Assert.assertEquals(StringComparators.STRLEN , jsonMapper.readValue(expectJsonSpec, StringComparators.StrlenComparator.class)); } + + @Test + public void testNumericComparatorSerdeTest() throws IOException + { + ObjectMapper jsonMapper = new DefaultObjectMapper(); + String expectJsonSpec = "{\"type\":\"numeric\"}"; + + String jsonSpec = jsonMapper.writeValueAsString(StringComparators.NUMERIC); + Assert.assertEquals(expectJsonSpec, jsonSpec); + Assert.assertEquals(StringComparators.NUMERIC + , jsonMapper.readValue(expectJsonSpec, StringComparators.NumericComparator.class)); + } } diff --git a/processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java new file mode 100644 index 000000000000..b2ed96263e5e --- /dev/null +++ b/processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java @@ -0,0 +1,51 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.search; + +import io.druid.query.search.search.AlphanumericSearchSortSpec; +import io.druid.query.search.search.NumericSearchSortSpec; +import io.druid.query.search.search.SearchHit; +import io.druid.query.search.search.SearchSortSpec; +import org.junit.Assert; +import org.junit.Test; + +/** + */ +public class NumericSearchSortSpecTest +{ + @Test + public void testComparator() + { + SearchSortSpec spec = new NumericSearchSortSpec(); + + SearchHit hit1 = new SearchHit("test", "1001001.12412"); + SearchHit hit2 = new SearchHit("test", "-1421"); + SearchHit hit3 = new SearchHit("test", "not-numeric-at-all"); + + SearchHit hit4 = new SearchHit("best", "1001001.12412"); + + + Assert.assertTrue(spec.getComparator().compare(hit1, hit2) > 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit1) < 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit2) < 0); + + Assert.assertTrue(spec.getComparator().compare(hit1, hit4) > 0); + } +} diff --git a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java index 9938c5f42332..13d09d70ef84 100644 --- a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java @@ -39,6 +39,7 @@ import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SelectorDimFilter; import io.druid.query.search.search.FragmentSearchQuerySpec; +import io.druid.query.search.search.NumericSearchSortSpec; import io.druid.query.search.search.SearchHit; import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQueryConfig; @@ -579,6 +580,31 @@ public void testSearchAll() ); } + @Test + public void testSearchWithNumericSort() + { + SearchQuery searchQuery = Druids.newSearchQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .query("a") + .sortSpec(new NumericSearchSortSpec()) + .build(); + + List expectedHits = Lists.newLinkedList(); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.placementishDimension, "a", 93)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.qualityDimension, "automotive", 93)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.qualityDimension, "entertainment", 93)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.qualityDimension, "health", 93)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.qualityDimension, "mezzanine", 279)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.marketDimension, "total_market", 186)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.qualityDimension, "travel", 93)); + expectedHits.add(new SearchHit(QueryRunnerTestHelper.partialNullDimension, "value", 186)); + + checkSearchQuery(searchQuery, expectedHits); + } + + private void checkSearchQuery(Query searchQuery, List expectedResults) { checkSearchQuery(searchQuery, runner, expectedResults); diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java index 86dc97e2c3b0..b27c16d55bc6 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -47,6 +47,7 @@ import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SelectorDimFilter; import io.druid.query.lookup.LookupExtractionFn; +import io.druid.query.ordering.StringComparators; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.segment.TestHelper; import org.joda.time.DateTime; @@ -2238,7 +2239,8 @@ public void testTimeseriesWithBoundFilter1() true, null, null, - null + null, + StringComparators.LEXICOGRAPHIC_NAME ), new BoundDimFilter( QueryRunnerTestHelper.marketDimension, @@ -2247,7 +2249,8 @@ public void testTimeseriesWithBoundFilter1() null, true, null, - null + null, + StringComparators.LEXICOGRAPHIC_NAME ), (DimFilter) new BoundDimFilter( QueryRunnerTestHelper.marketDimension, @@ -2256,7 +2259,8 @@ public void testTimeseriesWithBoundFilter1() null, null, null, - null + null, + StringComparators.LEXICOGRAPHIC_NAME ) ) ) diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java index e63467cc9ed4..631fe443050f 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java @@ -3250,6 +3250,39 @@ public void testAlphaNumericTopNWithNullPreviousStop() TestHelper.assertExpectedResults(expectedResults, runner.run(query, new HashMap())); } + @Test + public void testNumericDimensionTopNWithNullPreviousStop() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryGranularities.ALL) + .dimension(QueryRunnerTestHelper.marketDimension) + .metric(new NumericDimensionTopNMetricSpec(null)) + .threshold(2) + .intervals(QueryRunnerTestHelper.secondOnly) + .aggregators(Lists.newArrayList(QueryRunnerTestHelper.rowsCount)) + .build(); + List> expectedResults = Arrays.asList( + new Result<>( + new DateTime("2011-04-02T00:00:00.000Z"), + new TopNResultValue( + Arrays.asList( + ImmutableMap.of( + "market", "spot", + "rows", 9L + ), + ImmutableMap.of( + "market", "total_market", + "rows", 2L + ) + ) + ) + ) + ); + TestHelper.assertExpectedResults(expectedResults, runner.run(query, new HashMap())); + } + + @Test public void testTopNWithExtractionFilter() { diff --git a/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java b/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java index 00b5a9a95d6e..b78ac7a1f879 100644 --- a/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java @@ -34,6 +34,7 @@ import io.druid.query.extraction.JavaScriptExtractionFn; import io.druid.query.filter.BoundDimFilter; import io.druid.query.filter.DimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.segment.IndexBuilder; import io.druid.segment.StorageAdapter; import org.joda.time.DateTime; @@ -65,7 +66,9 @@ public class BoundFilterTest extends BaseFilterTest PARSER.parse(ImmutableMap.of("dim0", "2", "dim1", "2", "dim2", ImmutableList.of(""))), PARSER.parse(ImmutableMap.of("dim0", "3", "dim1", "1", "dim2", ImmutableList.of("a"))), PARSER.parse(ImmutableMap.of("dim0", "4", "dim1", "def", "dim2", ImmutableList.of("c"))), - PARSER.parse(ImmutableMap.of("dim0", "5", "dim1", "abc")) + PARSER.parse(ImmutableMap.of("dim0", "5", "dim1", "abc")), + PARSER.parse(ImmutableMap.of("dim0", "6", "dim1", "-1000", "dim2", ImmutableList.of("a"))), + PARSER.parse(ImmutableMap.of("dim0", "7", "dim1", "-10.012", "dim2", ImmutableList.of("d"))) ); public BoundFilterTest( @@ -88,14 +91,14 @@ public static void tearDown() throws Exception public void testLexicographicMatchEverything() { final List filters = ImmutableList.of( - new BoundDimFilter("dim0", "", "z", false, false, false, null), - new BoundDimFilter("dim1", "", "z", false, false, false, null), - new BoundDimFilter("dim2", "", "z", false, false, false, null), - new BoundDimFilter("dim3", "", "z", false, false, false, null) + new BoundDimFilter("dim0", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim2", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME) ); for (BoundDimFilter filter : filters) { - assertFilterMatches(filter, ImmutableList.of("0", "1", "2", "3", "4", "5")); + assertFilterMatches(filter, ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7")); } } @@ -103,15 +106,15 @@ public void testLexicographicMatchEverything() public void testLexicographicMatchNull() { assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, false, null), + new BoundDimFilter("dim0", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "", "", false, false, false, null), + new BoundDimFilter("dim1", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("0") ); assertFilterMatches( - new BoundDimFilter("dim2", "", "", false, false, false, null), + new BoundDimFilter("dim2", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("1", "2", "5") ); } @@ -120,44 +123,45 @@ public void testLexicographicMatchNull() public void testLexicographicMatchMissingColumn() { assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, false, false, null), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim3", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", true, false, false, null), + new BoundDimFilter("dim3", "", "", true, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, true, false, null), + new BoundDimFilter("dim3", "", "", false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim3", "", null, false, true, false, null), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim3", "", null, false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim3", null, "", false, false, false, null), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim3", null, "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim3", null, "", false, true, false, null), + new BoundDimFilter("dim3", null, "", false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); } + @Test public void testLexicographicMatchTooStrict() { assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", true, false, false, null), + new BoundDimFilter("dim1", "abc", "abc", true, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", true, true, false, null), + new BoundDimFilter("dim1", "abc", "abc", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", false, true, false, null), + new BoundDimFilter("dim1", "abc", "abc", false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of() ); } @@ -166,7 +170,7 @@ public void testLexicographicMatchTooStrict() public void testLexicographicMatchExactlySingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", false, false, false, null), + new BoundDimFilter("dim1", "abc", "abc", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("5") ); } @@ -175,7 +179,7 @@ public void testLexicographicMatchExactlySingleValue() public void testLexicographicMatchSurroundingSingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "ab", "abd", true, true, false, null), + new BoundDimFilter("dim1", "ab", "abd", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("5") ); } @@ -184,7 +188,7 @@ public void testLexicographicMatchSurroundingSingleValue() public void testLexicographicMatchNoUpperLimit() { assertFilterMatches( - new BoundDimFilter("dim1", "ab", null, true, true, false, null), + new BoundDimFilter("dim1", "ab", null, true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("4", "5") ); } @@ -193,8 +197,8 @@ public void testLexicographicMatchNoUpperLimit() public void testLexicographicMatchNoLowerLimit() { assertFilterMatches( - new BoundDimFilter("dim1", null, "abd", true, true, false, null), - ImmutableList.of("0", "1", "2", "3", "5") + new BoundDimFilter("dim1", null, "abd", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "5", "6", "7") ); } @@ -202,33 +206,37 @@ public void testLexicographicMatchNoLowerLimit() public void testLexicographicMatchNumbers() { assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", false, false, false, null), + new BoundDimFilter("dim1", "1", "3", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("1", "2", "3") ); assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", true, true, false, null), + new BoundDimFilter("dim1", "1", "3", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("1", "2") ); + assertFilterMatches( + new BoundDimFilter("dim1", "-1", "3", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("1", "2", "3", "6", "7") + ); } @Test public void testAlphaNumericMatchNull() { assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, true, null), + new BoundDimFilter("dim0", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "", "", false, false, true, null), + new BoundDimFilter("dim1", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("0") ); assertFilterMatches( - new BoundDimFilter("dim2", "", "", false, false, true, null), + new BoundDimFilter("dim2", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, false, true, null), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim3", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); } @@ -236,15 +244,15 @@ public void testAlphaNumericMatchNull() public void testAlphaNumericMatchTooStrict() { assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", true, false, true, null), + new BoundDimFilter("dim1", "2", "2", true, false, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", true, true, true, null), + new BoundDimFilter("dim1", "2", "2", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", false, true, true, null), + new BoundDimFilter("dim1", "2", "2", false, true, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of() ); } @@ -253,7 +261,7 @@ public void testAlphaNumericMatchTooStrict() public void testAlphaNumericMatchExactlySingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", false, false, true, null), + new BoundDimFilter("dim1", "2", "2", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("2") ); } @@ -262,7 +270,7 @@ public void testAlphaNumericMatchExactlySingleValue() public void testAlphaNumericMatchSurroundingSingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", true, true, true, null), + new BoundDimFilter("dim1", "1", "3", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("2") ); } @@ -271,8 +279,13 @@ public void testAlphaNumericMatchSurroundingSingleValue() public void testAlphaNumericMatchNoUpperLimit() { assertFilterMatches( - new BoundDimFilter("dim1", "1", null, true, true, true, null), - ImmutableList.of("1", "2", "4", "5") + new BoundDimFilter("dim1", "1", null, true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + ImmutableList.of("1", "2", "4", "5", "6", "7") + ); + + assertFilterMatches( + new BoundDimFilter("dim1", "-1", null, true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + ImmutableList.of("4", "5", "6", "7") ); } @@ -280,9 +293,121 @@ public void testAlphaNumericMatchNoUpperLimit() public void testAlphaNumericMatchNoLowerLimit() { assertFilterMatches( - new BoundDimFilter("dim1", null, "2", true, true, true, null), + new BoundDimFilter("dim1", null, "2", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("0", "3") ); + + assertFilterMatches( + new BoundDimFilter("dim1", null, "ZZZZZ", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") + ); + } + + @Test + public void testAlphaNumericMatchWithNegatives() + { + assertFilterMatches( + new BoundDimFilter("dim1", "-2000", "3", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + ImmutableList.of() + ); + + assertFilterMatches( + new BoundDimFilter("dim1", "3", "-2000", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + ImmutableList.of("1", "6", "7") + ); + } + + @Test + public void testNumericMatchNull() + { + assertFilterMatches( + new BoundDimFilter("dim0", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of() + ); + assertFilterMatches( + new BoundDimFilter("dim1", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("0") + ); + assertFilterMatches( + new BoundDimFilter("dim2", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("1", "2", "5") + ); + assertFilterMatches( + new BoundDimFilter("dim3", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") + ); + } + + @Test + public void testNumericMatchTooStrict() + { + assertFilterMatches( + new BoundDimFilter("dim1", "2", "2", true, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of() + ); + assertFilterMatches( + new BoundDimFilter("dim1", "2", "2", true, true, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of() + ); + assertFilterMatches( + new BoundDimFilter("dim1", "2", "2", false, true, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of() + ); + } + + @Test + public void testNumericMatchExactlySingleValue() + { + assertFilterMatches( + new BoundDimFilter("dim1", "2", "2", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("2") + ); + + assertFilterMatches( + new BoundDimFilter("dim1", "-10.012", "-10.012", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("7") + ); + } + + @Test + public void testNumericMatchSurroundingSingleValue() + { + assertFilterMatches( + new BoundDimFilter("dim1", "1", "3", true, true, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("2") + ); + + assertFilterMatches( + new BoundDimFilter("dim1", "-11", "-10", false, false, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("7") + ); + } + + @Test + public void testNumericMatchNoUpperLimit() + { + assertFilterMatches( + new BoundDimFilter("dim1", "1", null, true, true, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("1", "2") + ); + } + + @Test + public void testNumericMatchNoLowerLimit() + { + assertFilterMatches( + new BoundDimFilter("dim1", null, "2", true, true, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("0", "3", "4", "5", "6", "7") + ); + } + + @Test + public void testNumericMatchWithNegatives() + { + assertFilterMatches( + new BoundDimFilter("dim1", "-2000", "3", true, true, false, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("2", "3", "6", "7") + ); } @Test @@ -295,38 +420,48 @@ public void testMatchWithExtractionFn() ExtractionFn makeNullFn = new JavaScriptExtractionFn(nullJsFn, false, JavaScriptConfig.getDefault()); assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, false, makeNullFn), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim0", "", "", false, false, false, makeNullFn, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim1", "super-ab", "super-abd", true, true, false, superFn), + new BoundDimFilter("dim1", "super-ab", "super-abd", true, true, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("5") ); assertFilterMatches( - new BoundDimFilter("dim1", "super-0", "super-10", false, false, true, superFn), + new BoundDimFilter("dim1", "super-0", "super-10", false, false, true, superFn, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("1", "2", "3") ); assertFilterMatches( - new BoundDimFilter("dim2", "super-", "super-zzzzzz", false, false, false, superFn), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim2", "super-", "super-zzzzzz", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn), + new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim3", "super-null", "super-null", false, false, false, superFn), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim3", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") + ); + + assertFilterMatches( + new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") + ); + + assertFilterMatches( + new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn, StringComparators.NUMERIC_NAME), + ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn), - ImmutableList.of("0", "1", "2", "3", "4", "5") + new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn, StringComparators.NUMERIC_NAME), + ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); } diff --git a/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java b/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java index 54a96f64a365..601c6c1843ed 100644 --- a/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java +++ b/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java @@ -44,6 +44,7 @@ import io.druid.query.filter.SelectorDimFilter; import io.druid.query.lookup.LookupExtractionFn; import io.druid.query.lookup.LookupExtractor; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.segment.IndexBuilder; import io.druid.segment.StorageAdapter; @@ -131,12 +132,12 @@ public void testTimeFilterAsLong() ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, true, null), + new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, null, null, StringComparators.NUMERIC_NAME), ImmutableList.of("2", "3", "4", "5") ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "1", "4", true, true, true, null), + new BoundDimFilter(COUNT_COLUMN, "1", "4", true, true, null, null, StringComparators.NUMERIC_NAME), ImmutableList.of("2", "3") ); @@ -195,11 +196,11 @@ public void testLongFilterWithExtractionFn() ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "Fridax", "Fridaz", false, false, true, exfn), + new BoundDimFilter(COUNT_COLUMN, "Fridax", "Fridaz", false, false, null, exfn, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("5") ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "Friday", "Friday", true, true, true, exfn), + new BoundDimFilter(COUNT_COLUMN, "Friday", "Friday", true, true, null, exfn, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of() ); diff --git a/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java b/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java index de6da1fbece6..422639ce1a9c 100644 --- a/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java +++ b/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java @@ -42,6 +42,7 @@ import io.druid.query.filter.SelectorDimFilter; import io.druid.query.lookup.LookupExtractionFn; import io.druid.query.lookup.LookupExtractor; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.segment.IndexBuilder; import io.druid.segment.StorageAdapter; @@ -115,11 +116,11 @@ public void testTimeFilterAsLong() ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", false, false, true, null), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", false, false, null, null, StringComparators.NUMERIC_NAME), ImmutableList.of("0", "1", "2", "3", "4") ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", true, true, true, null), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", true, true, null, null, StringComparators.NUMERIC_NAME), ImmutableList.of("1", "2", "3") ); @@ -178,11 +179,11 @@ public void testTimeFilterWithExtractionFn() ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "Fridax", "Fridaz", false, false, true, exfn), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "Fridax", "Fridaz", false, false, null, exfn, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of("4") ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "Friday", "Friday", true, true, true, exfn), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "Friday", "Friday", true, true, null, exfn, StringComparators.ALPHANUMERIC_NAME), ImmutableList.of() ); diff --git a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java index 52dae617c186..d85c6f964f84 100644 --- a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java @@ -97,6 +97,7 @@ import io.druid.query.groupby.GroupByQueryEngine; import io.druid.query.groupby.GroupByQueryQueryToolChest; import io.druid.query.groupby.GroupByQueryRunnerTest; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.SearchQueryQueryToolChest; import io.druid.query.search.SearchResultValue; import io.druid.query.search.search.SearchHit; @@ -1403,14 +1404,20 @@ public void testTimeSeriesWithFilter() throws Exception Druids.newOrDimFilterBuilder().fields( Arrays.asList( new SelectorDimFilter("dim0", "1", null), - new BoundDimFilter("dim0", "222", "333", false, false, false, null) + new BoundDimFilter("dim0", "222", "333", false, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ) ) ).build(), Druids.newAndDimFilterBuilder().fields( Arrays.asList( new InDimFilter("dim1", Arrays.asList("0", "1", "2", "3", "4"), null), - new BoundDimFilter("dim1", "0", "3", false, true, false, null), - new BoundDimFilter("dim1", "1", "9999", true, false, false, null) + new BoundDimFilter("dim1", "0", "3", false, true, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ), + new BoundDimFilter("dim1", "1", "9999", true, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ) ) ).build() ) @@ -1475,14 +1482,20 @@ public void testSingleDimensionPruning() throws Exception Druids.newOrDimFilterBuilder().fields( Arrays.asList( new SelectorDimFilter("dim1", "a", null), - new BoundDimFilter("dim1", "from", "to", false, false, false, null) + new BoundDimFilter("dim1", "from", "to", false, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ) ) ).build(), Druids.newAndDimFilterBuilder().fields( Arrays.asList( new InDimFilter("dim2", Arrays.asList("a", "c", "e", "g"), null), - new BoundDimFilter("dim2", "aaa", "hi", false, false, false, null), - new BoundDimFilter("dim2", "e", "zzz", true, true, false, null) + new BoundDimFilter("dim2", "aaa", "hi", false, false, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ), + new BoundDimFilter("dim2", "e", "zzz", true, true, false, null, + StringComparators.LEXICOGRAPHIC_NAME + ) ) ).build() ) From 2405cc8a69a43e69501da63e098573b24f518876 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 21 Jul 2016 17:03:48 -0700 Subject: [PATCH 02/18] Only use direct long comparison for numeric ordering in BoundFilter, add time filtering benchmark query --- .../benchmark/query/TimeseriesBenchmark.java | 47 ++++++++++++++++++- .../io/druid/segment/filter/BoundFilter.java | 11 ++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java index 051a2370c776..3f9fd841fdcf 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java @@ -45,12 +45,15 @@ import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.DoubleMinAggregatorFactory; import io.druid.query.aggregation.DoubleSumAggregatorFactory; +import io.druid.query.aggregation.FilteredAggregatorFactory; import io.druid.query.aggregation.LongMaxAggregatorFactory; import io.druid.query.aggregation.LongSumAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesSerde; +import io.druid.query.filter.BoundDimFilter; import io.druid.query.filter.DimFilter; import io.druid.query.filter.SelectorDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.query.spec.QuerySegmentSpec; import io.druid.query.timeseries.TimeseriesQuery; @@ -64,6 +67,7 @@ import io.druid.segment.IndexSpec; import io.druid.segment.QueryableIndex; import io.druid.segment.QueryableIndexSegment; +import io.druid.segment.column.Column; import io.druid.segment.column.ColumnConfig; import io.druid.segment.incremental.IncrementalIndex; import io.druid.segment.incremental.IncrementalIndexSchema; @@ -104,7 +108,7 @@ public class TimeseriesBenchmark @Param({"750000"}) private int rowsPerSegment; - @Param({"basic.A"}) + @Param({"basic.A", "basic.timeFilter", "basic.timeFilterAlphanumeric"}) private String schemaAndQuery; private static final Logger log = new Logger(TimeseriesBenchmark.class); @@ -167,6 +171,47 @@ private void setupQueries() basicQueries.put("A", queryA); } + { + QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(Arrays.asList(basicSchema.getDataInterval())); + + List queryAggs = new ArrayList<>(); + LongSumAggregatorFactory lsaf = new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential"); + BoundDimFilter timeFilter = new BoundDimFilter(Column.TIME_COLUMN_NAME, "200000", "300000", false, false, null, null, + StringComparators.NUMERIC_NAME); + queryAggs.add(new FilteredAggregatorFactory(lsaf, timeFilter)); + + TimeseriesQuery timeFilterQuery = + Druids.newTimeseriesQueryBuilder() + .dataSource("blah") + .granularity(QueryGranularities.ALL) + .intervals(intervalSpec) + .aggregators(queryAggs) + .descending(false) + .build(); + + basicQueries.put("timeFilter", timeFilterQuery); + } + { + QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(Arrays.asList(basicSchema.getDataInterval())); + + List queryAggs = new ArrayList<>(); + LongSumAggregatorFactory lsaf = new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential"); + BoundDimFilter timeFilter = new BoundDimFilter(Column.TIME_COLUMN_NAME, "200000", "300000", false, false, null, null, + StringComparators.ALPHANUMERIC_NAME); + queryAggs.add(new FilteredAggregatorFactory(lsaf, timeFilter)); + + TimeseriesQuery timeFilterQuery = + Druids.newTimeseriesQueryBuilder() + .dataSource("blah") + .granularity(QueryGranularities.ALL) + .intervals(intervalSpec) + .aggregators(queryAggs) + .descending(false) + .build(); + + basicQueries.put("timeFilterAlphanumeric", timeFilterQuery); + } + SCHEMA_QUERY_MAP.put("basic", basicQueries); } diff --git a/processing/src/main/java/io/druid/segment/filter/BoundFilter.java b/processing/src/main/java/io/druid/segment/filter/BoundFilter.java index e078c635f8bf..896d9e1a49a5 100644 --- a/processing/src/main/java/io/druid/segment/filter/BoundFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/BoundFilter.java @@ -188,8 +188,17 @@ public boolean applyLong(long input) return doesMatch(extractionFn.apply(input)); } }; - } else { + } else if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC_NAME)){ return longPredicateSupplier.get(); + } else { + return new DruidLongPredicate() + { + @Override + public boolean applyLong(long input) + { + return doesMatch(String.valueOf(input)); + } + }; } } }; From 50400b18f9e1d089bd81724f5271f894dbce32b5 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 22 Jul 2016 13:57:03 -0700 Subject: [PATCH 03/18] Address PR comments, add multithreaded BoundDimFilter test --- .../benchmark/query/TimeseriesBenchmark.java | 18 ++++++++++++++++++ .../io/druid/query/filter/BoundDimFilter.java | 7 +++---- .../segment/filter/LongFilteringTest.java | 7 ++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java index 3f9fd841fdcf..f67a8dad111e 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java @@ -73,6 +73,7 @@ import io.druid.segment.incremental.IncrementalIndexSchema; import io.druid.segment.incremental.OnheapIncrementalIndex; import io.druid.segment.serde.ComplexMetrics; +import org.joda.time.Interval; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -211,6 +212,23 @@ private void setupQueries() basicQueries.put("timeFilterAlphanumeric", timeFilterQuery); } + { + QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(Arrays.asList(new Interval(200000, 300000))); + List queryAggs = new ArrayList<>(); + LongSumAggregatorFactory lsaf = new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential"); + queryAggs.add(lsaf); + + TimeseriesQuery timeFilterQuery = + Druids.newTimeseriesQueryBuilder() + .dataSource("blah") + .granularity(QueryGranularities.ALL) + .intervals(intervalSpec) + .aggregators(queryAggs) + .descending(false) + .build(); + + basicQueries.put("timeFilterByInterval", timeFilterQuery); + } SCHEMA_QUERY_MAP.put("basic", basicQueries); diff --git a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java index a02a63e7929d..f312335b5c29 100644 --- a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java @@ -174,14 +174,13 @@ public byte[] getCacheKey() byte lowerStrictByte = (this.isLowerStrict() == false) ? 0x0 : (byte) 1; byte upperStrictByte = (this.isUpperStrict() == false) ? 0x0 : (byte) 1; - byte AlphaNumericByte = (this.isAlphaNumeric() == false) ? 0x0 : (byte) 1; byte[] extractionFnBytes = extractionFn == null ? new byte[0] : extractionFn.getCacheKey(); byte[] orderingBytes = StringUtils.toUtf8(ordering); ByteBuffer boundCacheBuffer = ByteBuffer.allocate( - 10 + 9 + dimensionBytes.length + upperBytes.length + lowerBytes.length @@ -192,7 +191,6 @@ public byte[] getCacheKey() .put(boundType) .put(upperStrictByte) .put(lowerStrictByte) - .put(AlphaNumericByte) .put(DimFilterUtils.STRING_SEPARATOR) .put(dimensionBytes) .put(DimFilterUtils.STRING_SEPARATOR) @@ -221,7 +219,7 @@ public Filter toFilter() @Override public RangeSet getDimensionRangeSet(String dimension) { - if (!Objects.equals(getDimension(), dimension) || getExtractionFn() != null || alphaNumeric) { + if (!Objects.equals(getDimension(), dimension) || getExtractionFn() != null || !ordering.equals(StringComparators.LEXICOGRAPHIC_NAME)) { return null; } RangeSet retSet = TreeRangeSet.create(); @@ -329,6 +327,7 @@ public boolean applyLong(long input) return (lowerComparing >= 0) && (upperComparing > 0); } return (lowerComparing >= 0) && (upperComparing >= 0); + } }; } diff --git a/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java b/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java index 601c6c1843ed..871105db5747 100644 --- a/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java +++ b/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java @@ -238,7 +238,7 @@ public void testLongFilterWithExtractionFn() } @Test - public void testSelectorAndInFilterMultithreaded() + public void testMultithreaded() { assertFilterMatchesMultithreaded( new SelectorDimFilter(COUNT_COLUMN, "3", null), @@ -259,6 +259,11 @@ public void testSelectorAndInFilterMultithreaded() new InDimFilter(COUNT_COLUMN, infilterValues, null), ImmutableList.of("2", "4", "6") ); + + assertFilterMatches( + new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, null, null, StringComparators.NUMERIC_NAME), + ImmutableList.of("2", "3", "4", "5") + ); } private void assertFilterMatches( From bbde6503b643c5b85e4d851b2f3d61dc7585fddc Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 22 Jul 2016 14:32:41 -0700 Subject: [PATCH 04/18] Add comment on strlen tie handling --- docs/content/querying/sorting-orders.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/querying/sorting-orders.md b/docs/content/querying/sorting-orders.md index ebff7da7b8f6..f2072eeefa10 100644 --- a/docs/content/querying/sorting-orders.md +++ b/docs/content/querying/sorting-orders.md @@ -28,4 +28,4 @@ This sorting order will try to parse all string values as numbers. Unparseable v When comparing two unparseable values (e.g., "hello" and "world"), this ordering will sort by comparing the unparsed strings lexicographically. ## Strlen -Sorts values by the their string lengths. +Sorts values by the their string lengths. When there is a tie, this comparator falls back to using the String compareTo method. From 2e2ae997ae878ddd36b70578396ee4a21f7a3652 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 22 Jul 2016 14:38:25 -0700 Subject: [PATCH 05/18] Add timeseries interval filter benchmark --- .../main/java/io/druid/benchmark/query/TimeseriesBenchmark.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java index f67a8dad111e..d993e2273256 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java @@ -109,7 +109,7 @@ public class TimeseriesBenchmark @Param({"750000"}) private int rowsPerSegment; - @Param({"basic.A", "basic.timeFilter", "basic.timeFilterAlphanumeric"}) + @Param({"basic.A", "basic.timeFilter", "basic.timeFilterAlphanumeric", "basic.timeFilterByInterval"}) private String schemaAndQuery; private static final Logger log = new Logger(TimeseriesBenchmark.class); From bb5945d7303d29bf51d11b0c8c81309c145c379c Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 15:25:44 -0700 Subject: [PATCH 06/18] Adjust docs --- docs/content/querying/filters.md | 6 +++--- docs/content/querying/sorting-orders.md | 8 ++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/docs/content/querying/filters.md b/docs/content/querying/filters.md index 68b04277c6d1..a2a1735c5711 100644 --- a/docs/content/querying/filters.md +++ b/docs/content/querying/filters.md @@ -1,7 +1,7 @@ --- layout: doc_page --- -#Query Filters +# Query Filters A filter is a JSON object indicating which rows of data should be included in the computation for a query. It’s essentially the equivalent of the WHERE clause in SQL. Druid supports the following types of filters. ### Selector filter @@ -177,14 +177,14 @@ The IN filter supports the use of extraction functions, see [Filtering with Extr The Bound filter can be used to filter by comparing dimension values to an upper value and/or a lower value. |property|type|description|required?| -|--------|-----------|---------| +|--------|-----------|---------|---------| |type|String|This should always be "bound".|yes| |dimension|String|The dimension to filter on|yes| |lower|String|The lower bound for the filter|no| |upper|String|The upper bound for the filter|no| |lowerStrict|Boolean|Perform strict comparison on the lower bound ("<" instead of "<=")|no, default: false| |upperStrict|Boolean|Perform strict comparison on the upper bound (">" instead of ">=")|no, default: false| -|ordering|String|Specifies the sorting order to use when comparing values against the bound.
Can be one of the following values: "lexicographic", "alphanumeric", "numeric", "strlen".
See [Sorting Orders](./sorting-orders.html) for more details.|no, default: "lexicographic"| +|ordering|String|Specifies the sorting order to use when comparing values against the bound. Can be one of the following values: "lexicographic", "alphanumeric", "numeric", "strlen". See [Sorting Orders](./sorting-orders.html) for more details.|no, default: "lexicographic"| |extractionFn|[Extraction function](#filtering-with-extraction-functions)| Extraction function to apply to the dimension|no| The bound filter supports the use of extraction functions, see [Filtering with Extraction Functions](#filtering-with-extraction-functions) for details. diff --git a/docs/content/querying/sorting-orders.md b/docs/content/querying/sorting-orders.md index f2072eeefa10..b12a5d6416ab 100644 --- a/docs/content/querying/sorting-orders.md +++ b/docs/content/querying/sorting-orders.md @@ -1,18 +1,14 @@ --- layout: doc_page --- - # Sorting Orders - These sorting orders are used by the [TopNMetricSpec](./topnmetricspec.html), [SearchQuery](./searchquery.html), GroupByQuery's [LimitSpec](./limitspec.html), and [BoundFilter](./filters.html#bound-filter). ## Lexicographic -Sorts values in case-sensitive lexicographic order. +Sorts values by converting Strings to their UTF-8 byte array representations and comparing lexicgraphically, byte-by-byte. ## Alphanumeric -Suitable for strings with both numeric and non-numeric content, e.g.: - -"file12 sorts after file2" +Suitable for strings with both numeric and non-numeric content, e.g.: "file12 sorts after file2" See https://github.com/amjjd/java-alphanum for more details on how this ordering sorts values. From 127ea899d39795248ba659d6e4f94cbb24ff2797 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 16:24:39 -0700 Subject: [PATCH 07/18] Use jackson for StringComparator, address PR comments --- .../druid/benchmark/BoundFilterBenchmark.java | 12 +- .../benchmark/FilterPartitionBenchmark.java | 6 +- .../FilteredAggregatorBenchmark.java | 2 +- .../IncrementalIndexReadBenchmark.java | 2 +- .../benchmark/query/TimeseriesBenchmark.java | 4 +- .../io/druid/jackson/DefaultObjectMapper.java | 1 + .../druid/jackson/StringComparatorModule.java | 45 +++++++ .../io/druid/query/filter/BoundDimFilter.java | 40 +++--- .../groupby/orderby/DefaultLimitSpec.java | 2 +- .../groupby/orderby/OrderByColumnSpec.java | 4 +- .../query/ordering/StringComparator.java | 45 +++++++ .../query/ordering/StringComparators.java | 69 ++-------- .../io/druid/segment/filter/BoundFilter.java | 2 +- .../aggregation/FilteredAggregatorTest.java | 4 +- .../query/filter/BoundDimFilterTest.java | 32 ++--- .../filter/GetDimensionRangeSetTest.java | 8 +- .../query/groupby/GroupByQueryRunnerTest.java | 4 +- .../query/ordering/StringComparatorsTest.java | 31 +++-- .../timeseries/TimeseriesQueryRunnerTest.java | 6 +- .../druid/segment/filter/BoundFilterTest.java | 122 +++++++++--------- .../segment/filter/LongFilteringTest.java | 10 +- .../segment/filter/TimeFilteringTest.java | 8 +- .../client/CachingClusteredClientTest.java | 12 +- 23 files changed, 260 insertions(+), 211 deletions(-) create mode 100644 processing/src/main/java/io/druid/jackson/StringComparatorModule.java create mode 100644 processing/src/main/java/io/druid/query/ordering/StringComparator.java diff --git a/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java index 1d250693c5c4..d4225d3d75fc 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/BoundFilterBenchmark.java @@ -77,7 +77,7 @@ public class BoundFilterBenchmark false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ); @@ -90,7 +90,7 @@ public class BoundFilterBenchmark false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ); @@ -103,7 +103,7 @@ public class BoundFilterBenchmark false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ); @@ -116,7 +116,7 @@ public class BoundFilterBenchmark false, true, null, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC ) ); @@ -129,7 +129,7 @@ public class BoundFilterBenchmark false, true, null, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC ) ); @@ -142,7 +142,7 @@ public class BoundFilterBenchmark false, true, null, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC ) ); diff --git a/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java index a581777bca41..1544e8bccc6e 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java @@ -189,7 +189,7 @@ public void setup() throws IOException true, null, null, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC )); long halfEnd = (interval.getEndMillis() + interval.getStartMillis()) / 2; @@ -201,7 +201,7 @@ public void setup() throws IOException true, null, null, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC )); timeFilterAll = new BoundFilter(new BoundDimFilter( @@ -212,7 +212,7 @@ public void setup() throws IOException true, null, null, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC )); } diff --git a/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java index 4c1a245d23bb..fb079c8c8d56 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java @@ -186,7 +186,7 @@ public void setup() throws IOException filter = new OrDimFilter( Arrays.asList( - new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC), new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()), new RegexDimFilter("dimSequential", "X", null), new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null), diff --git a/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java index 027c616bf03b..c074038a3884 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java @@ -165,7 +165,7 @@ public void readWithFilters(Blackhole blackhole) throws Exception { DimFilter filter = new OrDimFilter( Arrays.asList( - new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC), new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()), new RegexDimFilter("dimSequential", "X", null), new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null), diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java index d993e2273256..27cc33a9eba3 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/TimeseriesBenchmark.java @@ -178,7 +178,7 @@ private void setupQueries() List queryAggs = new ArrayList<>(); LongSumAggregatorFactory lsaf = new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential"); BoundDimFilter timeFilter = new BoundDimFilter(Column.TIME_COLUMN_NAME, "200000", "300000", false, false, null, null, - StringComparators.NUMERIC_NAME); + StringComparators.NUMERIC); queryAggs.add(new FilteredAggregatorFactory(lsaf, timeFilter)); TimeseriesQuery timeFilterQuery = @@ -198,7 +198,7 @@ private void setupQueries() List queryAggs = new ArrayList<>(); LongSumAggregatorFactory lsaf = new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential"); BoundDimFilter timeFilter = new BoundDimFilter(Column.TIME_COLUMN_NAME, "200000", "300000", false, false, null, null, - StringComparators.ALPHANUMERIC_NAME); + StringComparators.ALPHANUMERIC); queryAggs.add(new FilteredAggregatorFactory(lsaf, timeFilter)); TimeseriesQuery timeFilterQuery = diff --git a/processing/src/main/java/io/druid/jackson/DefaultObjectMapper.java b/processing/src/main/java/io/druid/jackson/DefaultObjectMapper.java index 590432a3cef7..4027fcf7ccce 100644 --- a/processing/src/main/java/io/druid/jackson/DefaultObjectMapper.java +++ b/processing/src/main/java/io/druid/jackson/DefaultObjectMapper.java @@ -48,6 +48,7 @@ public DefaultObjectMapper(JsonFactory factory) registerModule(new QueryGranularityModule()); registerModule(new AggregatorsModule()); registerModule(new SegmentsModule()); + registerModule(new StringComparatorModule()); configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); configure(MapperFeature.AUTO_DETECT_GETTERS, false); diff --git a/processing/src/main/java/io/druid/jackson/StringComparatorModule.java b/processing/src/main/java/io/druid/jackson/StringComparatorModule.java new file mode 100644 index 000000000000..ae66db2d7f3e --- /dev/null +++ b/processing/src/main/java/io/druid/jackson/StringComparatorModule.java @@ -0,0 +1,45 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.jackson; + +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.databind.jsontype.NamedType; +import com.fasterxml.jackson.databind.module.SimpleModule; +import io.druid.query.ordering.StringComparator; +import io.druid.query.ordering.StringComparators; + +public class StringComparatorModule extends SimpleModule +{ + public StringComparatorModule() + { + super("StringComparatorModule"); + + setMixInAnnotation(StringComparator.class, StringComparatorMixin.class); + registerSubtypes( + new NamedType(StringComparators.LexicographicComparator.class, StringComparators.LEXICOGRAPHIC_NAME), + new NamedType(StringComparators.AlphanumericComparator.class, StringComparators.ALPHANUMERIC_NAME), + new NamedType(StringComparators.StrlenComparator.class, StringComparators.STRLEN_NAME), + new NamedType(StringComparators.NumericComparator.class, StringComparators.NUMERIC_NAME) + ); + } + + @JsonTypeInfo(use= JsonTypeInfo.Id.NAME, property = "type", defaultImpl = StringComparator.class) + public static interface StringComparatorMixin {} +} diff --git a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java index f312335b5c29..d94e7d4f73a5 100644 --- a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java @@ -32,6 +32,7 @@ import com.google.common.primitives.Longs; import com.metamx.common.StringUtils; import io.druid.query.extraction.ExtractionFn; +import io.druid.query.ordering.StringComparator; import io.druid.query.ordering.StringComparators; import io.druid.segment.filter.BoundFilter; @@ -45,9 +46,8 @@ public class BoundDimFilter implements DimFilter private final String lower; private final boolean lowerStrict; private final boolean upperStrict; - private final Boolean alphaNumeric; private final ExtractionFn extractionFn; - private final String ordering; + private final StringComparator ordering; private final Supplier longPredicateSupplier; @JsonCreator @@ -59,7 +59,7 @@ public BoundDimFilter( @JsonProperty("upperStrict") Boolean upperStrict, @Deprecated @JsonProperty("alphaNumeric") Boolean alphaNumeric, @JsonProperty("extractionFn") ExtractionFn extractionFn, - @JsonProperty("ordering") String ordering + @JsonProperty("ordering") StringComparator ordering ) { this.dimension = Preconditions.checkNotNull(dimension, "dimension can not be null"); @@ -68,25 +68,21 @@ public BoundDimFilter( this.lower = lower; this.lowerStrict = (lowerStrict == null) ? false : lowerStrict; this.upperStrict = (upperStrict == null) ? false : upperStrict; - this.alphaNumeric = alphaNumeric; + // For backwards compatibility, we retain the 'alphaNumeric' property. It will be used if the new 'ordering' + // property is missing. If both 'ordering' and 'alphaNumeric' are present, make sure they are consistent. if (ordering == null) { if (alphaNumeric == null || !alphaNumeric) { - this.ordering = StringComparators.LEXICOGRAPHIC_NAME; + this.ordering = StringComparators.LEXICOGRAPHIC; } else { - this.ordering = StringComparators.ALPHANUMERIC_NAME; + this.ordering = StringComparators.ALPHANUMERIC; } } else { - this.ordering = ordering.toLowerCase(); - Preconditions.checkState( - StringComparators.isOrderingValid(this.ordering), - "ordering must be one of the following: " + StringComparators.ORDERINGS.toString() - ); - + this.ordering = ordering; if (alphaNumeric != null) { - boolean orderingIsAlphanumeric = this.ordering.equals(StringComparators.ALPHANUMERIC_NAME); + boolean orderingIsAlphanumeric = this.ordering.equals(StringComparators.ALPHANUMERIC); Preconditions.checkState( - this.alphaNumeric == orderingIsAlphanumeric, + alphaNumeric == orderingIsAlphanumeric, "mismatch between alphanumeric and ordering property" ); } @@ -125,13 +121,6 @@ public boolean isUpperStrict() return upperStrict; } - @Deprecated - @JsonIgnore - public Boolean isAlphaNumeric() - { - return alphaNumeric; - } - public boolean hasLowerBound() { return lower != null; @@ -149,7 +138,7 @@ public ExtractionFn getExtractionFn() } @JsonProperty - public String getOrdering() + public StringComparator getOrdering() { return ordering; } @@ -177,7 +166,7 @@ public byte[] getCacheKey() byte[] extractionFnBytes = extractionFn == null ? new byte[0] : extractionFn.getCacheKey(); - byte[] orderingBytes = StringUtils.toUtf8(ordering); + byte[] orderingBytes = StringUtils.toUtf8(ordering.toString()); ByteBuffer boundCacheBuffer = ByteBuffer.allocate( 9 @@ -219,9 +208,12 @@ public Filter toFilter() @Override public RangeSet getDimensionRangeSet(String dimension) { - if (!Objects.equals(getDimension(), dimension) || getExtractionFn() != null || !ordering.equals(StringComparators.LEXICOGRAPHIC_NAME)) { + if (!(Objects.equals(getDimension(), dimension) + && getExtractionFn() == null + && ordering.equals(StringComparators.LEXICOGRAPHIC))) { return null; } + RangeSet retSet = TreeRangeSet.create(); Range range; if (getLower() == null) { diff --git a/processing/src/main/java/io/druid/query/groupby/orderby/DefaultLimitSpec.java b/processing/src/main/java/io/druid/query/groupby/orderby/DefaultLimitSpec.java index 18b59f3cc67a..b8ddcfa74e02 100644 --- a/processing/src/main/java/io/druid/query/groupby/orderby/DefaultLimitSpec.java +++ b/processing/src/main/java/io/druid/query/groupby/orderby/DefaultLimitSpec.java @@ -40,7 +40,7 @@ import io.druid.query.aggregation.PostAggregator; import io.druid.query.dimension.DimensionSpec; import io.druid.query.ordering.StringComparators; -import io.druid.query.ordering.StringComparators.StringComparator; +import io.druid.query.ordering.StringComparator; import javax.annotation.Nullable; import java.nio.ByteBuffer; diff --git a/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java b/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java index 388b814d1e35..d200ef5d39bf 100644 --- a/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java +++ b/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java @@ -30,7 +30,7 @@ import com.metamx.common.StringUtils; import io.druid.query.ordering.StringComparators; -import io.druid.query.ordering.StringComparators.StringComparator; +import io.druid.query.ordering.StringComparator; import javax.annotation.Nullable; import java.nio.ByteBuffer; @@ -202,7 +202,7 @@ private static StringComparator determinDimensionComparator(Object dimensionOrde } String dimensionOrderString = dimensionOrderObj.toString().toLowerCase(); - return StringComparators.makeComparator(dimensionOrderString); + return StringComparator.fromString(dimensionOrderString); } @Override diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparator.java b/processing/src/main/java/io/druid/query/ordering/StringComparator.java new file mode 100644 index 000000000000..095ecf028dac --- /dev/null +++ b/processing/src/main/java/io/druid/query/ordering/StringComparator.java @@ -0,0 +1,45 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.ordering; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.metamx.common.IAE; + +import java.util.Comparator; + +public abstract class StringComparator implements Comparator +{ + @JsonCreator + public static StringComparator fromString(String type) + { + switch (type) { + case StringComparators.LEXICOGRAPHIC_NAME: + return StringComparators.LEXICOGRAPHIC; + case StringComparators.ALPHANUMERIC_NAME: + return StringComparators.ALPHANUMERIC; + case StringComparators.STRLEN_NAME: + return StringComparators.STRLEN; + case StringComparators.NUMERIC_NAME: + return StringComparators.NUMERIC; + default: + throw new IAE("Unknown string comparator[%s]", type); + } + } +} diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparators.java b/processing/src/main/java/io/druid/query/ordering/StringComparators.java index 1636e72e83ac..f79c6f1efff8 100644 --- a/processing/src/main/java/io/druid/query/ordering/StringComparators.java +++ b/processing/src/main/java/io/druid/query/ordering/StringComparators.java @@ -20,22 +20,13 @@ package io.druid.query.ordering; import java.math.BigDecimal; -import java.util.Arrays; import java.util.Comparator; -import java.util.List; -import com.fasterxml.jackson.annotation.JsonSubTypes; -import com.fasterxml.jackson.annotation.JsonTypeInfo; -import com.fasterxml.jackson.annotation.JsonTypeInfo.As; -import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Ordering; import com.google.common.primitives.Ints; import com.google.common.primitives.UnsignedBytes; -import com.metamx.common.IAE; import com.metamx.common.StringUtils; - public class StringComparators { public static final String LEXICOGRAPHIC_NAME = "lexicographic"; @@ -43,27 +34,12 @@ public class StringComparators public static final String NUMERIC_NAME = "numeric"; public static final String STRLEN_NAME = "strlen"; - public static final List ORDERINGS = ImmutableList.of( - LEXICOGRAPHIC_NAME, ALPHANUMERIC_NAME, NUMERIC_NAME, STRLEN_NAME - ); - - public static final LexicographicComparator LEXICOGRAPHIC = new LexicographicComparator(); - public static final AlphanumericComparator ALPHANUMERIC = new AlphanumericComparator(); - public static final NumericComparator NUMERIC = new NumericComparator(); - public static final StrlenComparator STRLEN = new StrlenComparator(); - - @JsonTypeInfo(use=Id.NAME, include=As.PROPERTY, property="type", defaultImpl = LexicographicComparator.class) - @JsonSubTypes(value = { - @JsonSubTypes.Type(name = StringComparators.LEXICOGRAPHIC_NAME, value = LexicographicComparator.class), - @JsonSubTypes.Type(name = StringComparators.ALPHANUMERIC_NAME, value = AlphanumericComparator.class), - @JsonSubTypes.Type(name = StringComparators.STRLEN_NAME, value = StrlenComparator.class), - @JsonSubTypes.Type(name = StringComparators.NUMERIC_NAME, value = NumericComparator.class), - }) - public static interface StringComparator extends Comparator - { - } + public static final StringComparator LEXICOGRAPHIC = new LexicographicComparator(); + public static final StringComparator ALPHANUMERIC = new AlphanumericComparator(); + public static final StringComparator NUMERIC = new NumericComparator(); + public static final StringComparator STRLEN = new StrlenComparator(); - public static class LexicographicComparator implements StringComparator + public static class LexicographicComparator extends StringComparator { private static final Ordering ORDERING = Ordering.from(new Comparator() { @@ -85,7 +61,7 @@ public int compare(String s, String s2) return ORDERING.compare(s, s2); } - + @Override public boolean equals(Object o) { @@ -98,7 +74,7 @@ public boolean equals(Object o) return true; } - + @Override public String toString() { @@ -106,7 +82,7 @@ public String toString() } } - public static class AlphanumericComparator implements StringComparator + public static class AlphanumericComparator extends StringComparator { // This code is based on https://github.com/amjjd/java-alphanum, see // NOTICE file for more information @@ -312,7 +288,7 @@ public String toString() } } - public static class StrlenComparator implements StringComparator + public static class StrlenComparator extends StringComparator { private static final Ordering ORDERING = Ordering.from(new Comparator() { @@ -363,7 +339,7 @@ private static BigDecimal convertStringToBigDecimal(String input) { return bd; } - public static class NumericComparator implements StringComparator + public static class NumericComparator extends StringComparator { @Override public int compare(String o1, String o2) @@ -410,29 +386,4 @@ public boolean equals(Object o) return true; } } - - public static StringComparator makeComparator(String type) - { - switch (type) { - case StringComparators.LEXICOGRAPHIC_NAME: - return LEXICOGRAPHIC; - case StringComparators.ALPHANUMERIC_NAME: - return ALPHANUMERIC; - case StringComparators.STRLEN_NAME: - return STRLEN; - case StringComparators.NUMERIC_NAME: - return NUMERIC; - default: - throw new IAE("Unknown string comparator[%s]", type); - } - } - - public static boolean isOrderingValid(String ordering) - { - if (ordering == null) { - return false; - } - - return ORDERINGS.contains(ordering); - } } diff --git a/processing/src/main/java/io/druid/segment/filter/BoundFilter.java b/processing/src/main/java/io/druid/segment/filter/BoundFilter.java index 896d9e1a49a5..dbf336094ca9 100644 --- a/processing/src/main/java/io/druid/segment/filter/BoundFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/BoundFilter.java @@ -49,7 +49,7 @@ public class BoundFilter implements Filter public BoundFilter(final BoundDimFilter boundDimFilter) { this.boundDimFilter = boundDimFilter; - this.comparator = StringComparators.makeComparator(boundDimFilter.getOrdering()); + this.comparator = boundDimFilter.getOrdering(); this.extractionFn = boundDimFilter.getExtractionFn(); this.longPredicateSupplier = boundDimFilter.getLongPredicateSupplier(); } diff --git a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java index 45d001deb334..73523879b22e 100644 --- a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java @@ -258,7 +258,7 @@ public void testAggregateWithPredicateFilters() factory = new FilteredAggregatorFactory( new DoubleSumAggregatorFactory("billy", "value"), - new BoundDimFilter("dim", "a", "a", false, false, true, null, StringComparators.ALPHANUMERIC_NAME) + new BoundDimFilter("dim", "a", "a", false, false, true, null, StringComparators.ALPHANUMERIC) ); selector = new TestFloatColumnSelector(values); validateFilteredAggs(factory, values, selector); @@ -313,7 +313,7 @@ public void testAggregateWithExtractionFns() factory = new FilteredAggregatorFactory( new DoubleSumAggregatorFactory("billy", "value"), new BoundDimFilter("dim", "aAARDVARK", "aAARDVARK", false, false, true, extractionFn, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC ) ); selector = new TestFloatColumnSelector(values); diff --git a/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java b/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java index 2255aaf09e5e..28db13acd453 100644 --- a/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java +++ b/processing/src/test/java/io/druid/query/filter/BoundDimFilterTest.java @@ -52,23 +52,23 @@ public static Iterable constructorFeeder(){ return ImmutableList.of( new Object[]{new BoundDimFilter("dimension", "12", "15", null, null, null, null, - StringComparators.LEXICOGRAPHIC_NAME)}, + StringComparators.LEXICOGRAPHIC)}, new Object[]{new BoundDimFilter("dimension", "12", "15", null, true, false, null, - StringComparators.LEXICOGRAPHIC_NAME)}, + StringComparators.LEXICOGRAPHIC)}, new Object[]{new BoundDimFilter("dimension", "12", "15", null, null, true, null, - StringComparators.ALPHANUMERIC_NAME)}, + StringComparators.ALPHANUMERIC)}, new Object[]{new BoundDimFilter("dimension", null, "15", null, true, true, null, - StringComparators.ALPHANUMERIC_NAME)}, + StringComparators.ALPHANUMERIC)}, new Object[]{new BoundDimFilter("dimension", "12", "15", true, null, null, null, - StringComparators.LEXICOGRAPHIC_NAME)}, + StringComparators.LEXICOGRAPHIC)}, new Object[]{new BoundDimFilter("dimension", "12", null, true, null, true, null, - StringComparators.ALPHANUMERIC_NAME)}, + StringComparators.ALPHANUMERIC)}, new Object[]{new BoundDimFilter("dimension", "12", "15", true, true, true, null, - StringComparators.ALPHANUMERIC_NAME)}, + StringComparators.ALPHANUMERIC)}, new Object[]{new BoundDimFilter("dimension", "12", "15", true, true, false, null, - StringComparators.LEXICOGRAPHIC_NAME)}, + StringComparators.LEXICOGRAPHIC)}, new Object[]{new BoundDimFilter("dimension", null, "15", null, true, true, extractionFn, - StringComparators.ALPHANUMERIC_NAME)} + StringComparators.ALPHANUMERIC)} ); } @@ -85,14 +85,14 @@ public void testSerDesBoundFilter() throws IOException @Test public void testGetCacheKey() { - BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null, StringComparators.ALPHANUMERIC_NAME); - BoundDimFilter boundDimFilterCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, null, StringComparators.ALPHANUMERIC_NAME); + BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null, StringComparators.ALPHANUMERIC); + BoundDimFilter boundDimFilterCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, null, StringComparators.ALPHANUMERIC); Assert.assertArrayEquals(boundDimFilter.getCacheKey(), boundDimFilterCopy.getCacheKey()); - BoundDimFilter anotherBoundDimFilter = new BoundDimFilter("dimension", "12", "15", true, null, false, null, StringComparators.LEXICOGRAPHIC_NAME); + BoundDimFilter anotherBoundDimFilter = new BoundDimFilter("dimension", "12", "15", true, null, false, null, StringComparators.LEXICOGRAPHIC); Assert.assertFalse(Arrays.equals(anotherBoundDimFilter.getCacheKey(), boundDimFilter.getCacheKey())); - BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn, StringComparators.ALPHANUMERIC_NAME); - BoundDimFilter boundDimFilterWithExtractCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, extractionFn, StringComparators.ALPHANUMERIC_NAME); + BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn, StringComparators.ALPHANUMERIC); + BoundDimFilter boundDimFilterWithExtractCopy = new BoundDimFilter("dimension", "12", "15", false, false, true, extractionFn, StringComparators.ALPHANUMERIC); Assert.assertFalse(Arrays.equals(boundDimFilter.getCacheKey(), boundDimFilterWithExtract.getCacheKey())); Assert.assertArrayEquals(boundDimFilterWithExtract.getCacheKey(), boundDimFilterWithExtractCopy.getCacheKey()); } @@ -100,8 +100,8 @@ public void testGetCacheKey() @Test public void testHashCode() { - BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null, StringComparators.ALPHANUMERIC_NAME); - BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn, StringComparators.ALPHANUMERIC_NAME); + BoundDimFilter boundDimFilter = new BoundDimFilter("dimension", "12", "15", null, null, true, null, StringComparators.ALPHANUMERIC); + BoundDimFilter boundDimFilterWithExtract = new BoundDimFilter("dimension", "12", "15", null, null, true, extractionFn, StringComparators.ALPHANUMERIC); Assert.assertNotEquals(boundDimFilter.hashCode(), boundDimFilterWithExtract.hashCode()); } diff --git a/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java b/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java index be82c76881bb..eb3e4e7c265a 100644 --- a/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java +++ b/processing/src/test/java/io/druid/query/filter/GetDimensionRangeSetTest.java @@ -45,16 +45,16 @@ public class GetDimensionRangeSetTest private final DimFilter in2 = new InDimFilter("dim2", ImmutableList.of("again"), null); private final DimFilter in3 = new InDimFilter("dim1", Arrays.asList("null", null), null); private final DimFilter bound1 = new BoundDimFilter("dim1", "from", "to", false, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ); private final DimFilter bound2 = new BoundDimFilter("dim1", null, "tillend", false, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ); private final DimFilter bound3 = new BoundDimFilter("dim1", "notincluded", null, true, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ); private final DimFilter bound4 = new BoundDimFilter("dim2", "again", "exclusive", true, true, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ); private final DimFilter other1 = new RegexDimFilter("someDim", "pattern", null); private final DimFilter other2 = new JavaScriptDimFilter("someOtherDim", "function(x) { return x }", null, diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java index 6fad02de1cd8..107fcdb9bf76 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java @@ -5517,7 +5517,7 @@ public void testBySegmentResultsWithAllFiltersWithExtractionFns() false, true, extractionFn, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC )); superFilterList.add(new RegexDimFilter("quality", "super-mezzanine", extractionFn)); superFilterList.add(new SearchQueryDimFilter( @@ -5576,7 +5576,7 @@ public void testGroupByWithAllFiltersOnNullDimsWithExtractionFns() superFilterList.add(new SelectorDimFilter("null_column", "EMPTY", extractionFn)); superFilterList.add(new InDimFilter("null_column", Arrays.asList("NOT-EMPTY", "FOOBAR", "EMPTY"), extractionFn)); superFilterList.add(new BoundDimFilter("null_column", "EMPTY", "EMPTY", false, false, true, extractionFn, - StringComparators.ALPHANUMERIC_NAME + StringComparators.ALPHANUMERIC )); superFilterList.add(new RegexDimFilter("null_column", "EMPTY", extractionFn)); superFilterList.add(new SearchQueryDimFilter( diff --git a/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java b/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java index e282f013b4f0..e0e82d538be9 100644 --- a/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java +++ b/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java @@ -32,11 +32,10 @@ import com.google.common.collect.Lists; import io.druid.jackson.DefaultObjectMapper; -import io.druid.query.ordering.StringComparators.StringComparator; +import io.druid.query.ordering.StringComparator; public class StringComparatorsTest { - private void commonTest(StringComparator comparator) { // equality test @@ -149,11 +148,15 @@ public void testLexicographicComparatorSerdeTest() throws IOException { ObjectMapper jsonMapper = new DefaultObjectMapper(); String expectJsonSpec = "{\"type\":\"lexicographic\"}"; - + String jsonSpec = jsonMapper.writeValueAsString(StringComparators.LEXICOGRAPHIC); Assert.assertEquals(expectJsonSpec, jsonSpec); Assert.assertEquals(StringComparators.LEXICOGRAPHIC - , jsonMapper.readValue(expectJsonSpec, StringComparators.LexicographicComparator.class)); + , jsonMapper.readValue(expectJsonSpec, StringComparator.class)); + + String makeFromJsonSpec = "\"lexicographic\""; + Assert.assertEquals(StringComparators.LEXICOGRAPHIC + , jsonMapper.readValue(makeFromJsonSpec, StringComparator.class)); } @Test @@ -161,11 +164,15 @@ public void testAlphanumericComparatorSerdeTest() throws IOException { ObjectMapper jsonMapper = new DefaultObjectMapper(); String expectJsonSpec = "{\"type\":\"alphanumeric\"}"; - + String jsonSpec = jsonMapper.writeValueAsString(StringComparators.ALPHANUMERIC); Assert.assertEquals(expectJsonSpec, jsonSpec); Assert.assertEquals(StringComparators.ALPHANUMERIC - , jsonMapper.readValue(expectJsonSpec, StringComparators.AlphanumericComparator.class)); + , jsonMapper.readValue(expectJsonSpec, StringComparator.class)); + + String makeFromJsonSpec = "\"alphanumeric\""; + Assert.assertEquals(StringComparators.ALPHANUMERIC + , jsonMapper.readValue(makeFromJsonSpec, StringComparator.class)); } @Test @@ -177,7 +184,11 @@ public void testStrlenComparatorSerdeTest() throws IOException String jsonSpec = jsonMapper.writeValueAsString(StringComparators.STRLEN); Assert.assertEquals(expectJsonSpec, jsonSpec); Assert.assertEquals(StringComparators.STRLEN - , jsonMapper.readValue(expectJsonSpec, StringComparators.StrlenComparator.class)); + , jsonMapper.readValue(expectJsonSpec, StringComparator.class)); + + String makeFromJsonSpec = "\"strlen\""; + Assert.assertEquals(StringComparators.STRLEN + , jsonMapper.readValue(makeFromJsonSpec, StringComparator.class)); } @Test @@ -189,6 +200,10 @@ public void testNumericComparatorSerdeTest() throws IOException String jsonSpec = jsonMapper.writeValueAsString(StringComparators.NUMERIC); Assert.assertEquals(expectJsonSpec, jsonSpec); Assert.assertEquals(StringComparators.NUMERIC - , jsonMapper.readValue(expectJsonSpec, StringComparators.NumericComparator.class)); + , jsonMapper.readValue(expectJsonSpec, StringComparator.class)); + + String makeFromJsonSpec = "\"numeric\""; + Assert.assertEquals(StringComparators.NUMERIC + , jsonMapper.readValue(makeFromJsonSpec, StringComparator.class)); } } diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java index b27c16d55bc6..9b7df13d20b5 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -2240,7 +2240,7 @@ public void testTimeseriesWithBoundFilter1() null, null, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ), new BoundDimFilter( QueryRunnerTestHelper.marketDimension, @@ -2250,7 +2250,7 @@ public void testTimeseriesWithBoundFilter1() true, null, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ), (DimFilter) new BoundDimFilter( QueryRunnerTestHelper.marketDimension, @@ -2260,7 +2260,7 @@ public void testTimeseriesWithBoundFilter1() null, null, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ) ) diff --git a/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java b/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java index b78ac7a1f879..4d0ec976d136 100644 --- a/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/BoundFilterTest.java @@ -91,10 +91,10 @@ public static void tearDown() throws Exception public void testLexicographicMatchEverything() { final List filters = ImmutableList.of( - new BoundDimFilter("dim0", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), - new BoundDimFilter("dim1", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), - new BoundDimFilter("dim2", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), - new BoundDimFilter("dim3", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME) + new BoundDimFilter("dim0", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC), + new BoundDimFilter("dim1", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC), + new BoundDimFilter("dim2", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC), + new BoundDimFilter("dim3", "", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC) ); for (BoundDimFilter filter : filters) { @@ -106,15 +106,15 @@ public void testLexicographicMatchEverything() public void testLexicographicMatchNull() { assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim0", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0") ); assertFilterMatches( - new BoundDimFilter("dim2", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim2", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("1", "2", "5") ); } @@ -123,27 +123,27 @@ public void testLexicographicMatchNull() public void testLexicographicMatchMissingColumn() { assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", "", "", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", true, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", "", "", true, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", "", "", false, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim3", "", null, false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", "", null, false, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim3", null, "", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", null, "", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim3", null, "", false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", null, "", false, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); } @@ -153,15 +153,15 @@ public void testLexicographicMatchMissingColumn() public void testLexicographicMatchTooStrict() { assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", true, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "abc", "abc", true, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "abc", "abc", true, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", false, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "abc", "abc", false, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of() ); } @@ -170,7 +170,7 @@ public void testLexicographicMatchTooStrict() public void testLexicographicMatchExactlySingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "abc", "abc", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "abc", "abc", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("5") ); } @@ -179,7 +179,7 @@ public void testLexicographicMatchExactlySingleValue() public void testLexicographicMatchSurroundingSingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "ab", "abd", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "ab", "abd", true, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("5") ); } @@ -188,7 +188,7 @@ public void testLexicographicMatchSurroundingSingleValue() public void testLexicographicMatchNoUpperLimit() { assertFilterMatches( - new BoundDimFilter("dim1", "ab", null, true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "ab", null, true, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("4", "5") ); } @@ -197,7 +197,7 @@ public void testLexicographicMatchNoUpperLimit() public void testLexicographicMatchNoLowerLimit() { assertFilterMatches( - new BoundDimFilter("dim1", null, "abd", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", null, "abd", true, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "5", "6", "7") ); } @@ -206,15 +206,15 @@ public void testLexicographicMatchNoLowerLimit() public void testLexicographicMatchNumbers() { assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", false, false, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "1", "3", false, false, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("1", "2", "3") ); assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "1", "3", true, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("1", "2") ); assertFilterMatches( - new BoundDimFilter("dim1", "-1", "3", true, true, false, null, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "-1", "3", true, true, false, null, StringComparators.LEXICOGRAPHIC), ImmutableList.of("1", "2", "3", "6", "7") ); } @@ -223,19 +223,19 @@ public void testLexicographicMatchNumbers() public void testAlphaNumericMatchNull() { assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim0", "", "", false, false, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "", "", false, false, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("0") ); assertFilterMatches( - new BoundDimFilter("dim2", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim2", "", "", false, false, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim3", "", "", false, false, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); } @@ -244,15 +244,15 @@ public void testAlphaNumericMatchNull() public void testAlphaNumericMatchTooStrict() { assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", true, false, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", true, false, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", false, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", false, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of() ); } @@ -261,7 +261,7 @@ public void testAlphaNumericMatchTooStrict() public void testAlphaNumericMatchExactlySingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", false, false, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", false, false, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("2") ); } @@ -270,7 +270,7 @@ public void testAlphaNumericMatchExactlySingleValue() public void testAlphaNumericMatchSurroundingSingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "1", "3", true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("2") ); } @@ -279,12 +279,12 @@ public void testAlphaNumericMatchSurroundingSingleValue() public void testAlphaNumericMatchNoUpperLimit() { assertFilterMatches( - new BoundDimFilter("dim1", "1", null, true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "1", null, true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("1", "2", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim1", "-1", null, true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "-1", null, true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("4", "5", "6", "7") ); } @@ -293,12 +293,12 @@ public void testAlphaNumericMatchNoUpperLimit() public void testAlphaNumericMatchNoLowerLimit() { assertFilterMatches( - new BoundDimFilter("dim1", null, "2", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", null, "2", true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("0", "3") ); assertFilterMatches( - new BoundDimFilter("dim1", null, "ZZZZZ", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", null, "ZZZZZ", true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); } @@ -307,12 +307,12 @@ public void testAlphaNumericMatchNoLowerLimit() public void testAlphaNumericMatchWithNegatives() { assertFilterMatches( - new BoundDimFilter("dim1", "-2000", "3", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "-2000", "3", true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "3", "-2000", true, true, true, null, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "3", "-2000", true, true, true, null, StringComparators.ALPHANUMERIC), ImmutableList.of("1", "6", "7") ); } @@ -321,19 +321,19 @@ public void testAlphaNumericMatchWithNegatives() public void testNumericMatchNull() { assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim0", "", "", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "", "", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of("0") ); assertFilterMatches( - new BoundDimFilter("dim2", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim2", "", "", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim3", "", "", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim3", "", "", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); } @@ -342,15 +342,15 @@ public void testNumericMatchNull() public void testNumericMatchTooStrict() { assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", true, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", true, false, false, null, StringComparators.NUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", true, true, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", true, true, false, null, StringComparators.NUMERIC), ImmutableList.of() ); assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", false, true, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", false, true, false, null, StringComparators.NUMERIC), ImmutableList.of() ); } @@ -359,12 +359,12 @@ public void testNumericMatchTooStrict() public void testNumericMatchExactlySingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "2", "2", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "2", "2", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of("2") ); assertFilterMatches( - new BoundDimFilter("dim1", "-10.012", "-10.012", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "-10.012", "-10.012", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of("7") ); } @@ -373,12 +373,12 @@ public void testNumericMatchExactlySingleValue() public void testNumericMatchSurroundingSingleValue() { assertFilterMatches( - new BoundDimFilter("dim1", "1", "3", true, true, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "1", "3", true, true, false, null, StringComparators.NUMERIC), ImmutableList.of("2") ); assertFilterMatches( - new BoundDimFilter("dim1", "-11", "-10", false, false, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "-11", "-10", false, false, false, null, StringComparators.NUMERIC), ImmutableList.of("7") ); } @@ -387,7 +387,7 @@ public void testNumericMatchSurroundingSingleValue() public void testNumericMatchNoUpperLimit() { assertFilterMatches( - new BoundDimFilter("dim1", "1", null, true, true, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "1", null, true, true, false, null, StringComparators.NUMERIC), ImmutableList.of("1", "2") ); } @@ -396,7 +396,7 @@ public void testNumericMatchNoUpperLimit() public void testNumericMatchNoLowerLimit() { assertFilterMatches( - new BoundDimFilter("dim1", null, "2", true, true, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", null, "2", true, true, false, null, StringComparators.NUMERIC), ImmutableList.of("0", "3", "4", "5", "6", "7") ); } @@ -405,7 +405,7 @@ public void testNumericMatchNoLowerLimit() public void testNumericMatchWithNegatives() { assertFilterMatches( - new BoundDimFilter("dim1", "-2000", "3", true, true, false, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim1", "-2000", "3", true, true, false, null, StringComparators.NUMERIC), ImmutableList.of("2", "3", "6", "7") ); } @@ -420,47 +420,47 @@ public void testMatchWithExtractionFn() ExtractionFn makeNullFn = new JavaScriptExtractionFn(nullJsFn, false, JavaScriptConfig.getDefault()); assertFilterMatches( - new BoundDimFilter("dim0", "", "", false, false, false, makeNullFn, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim0", "", "", false, false, false, makeNullFn, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim1", "super-ab", "super-abd", true, true, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim1", "super-ab", "super-abd", true, true, false, superFn, StringComparators.LEXICOGRAPHIC), ImmutableList.of("5") ); assertFilterMatches( - new BoundDimFilter("dim1", "super-0", "super-10", false, false, true, superFn, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter("dim1", "super-0", "super-10", false, false, true, superFn, StringComparators.ALPHANUMERIC), ImmutableList.of("1", "2", "3") ); assertFilterMatches( - new BoundDimFilter("dim2", "super-", "super-zzzzzz", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim2", "super-", "super-zzzzzz", false, false, false, superFn, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC), ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim3", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim3", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC_NAME), + new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn, StringComparators.LEXICOGRAPHIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); assertFilterMatches( - new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim2", "super-null", "super-null", false, false, false, superFn, StringComparators.NUMERIC), ImmutableList.of("1", "2", "5") ); assertFilterMatches( - new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn, StringComparators.NUMERIC_NAME), + new BoundDimFilter("dim4", "super-null", "super-null", false, false, false, superFn, StringComparators.NUMERIC), ImmutableList.of("0", "1", "2", "3", "4", "5", "6", "7") ); } diff --git a/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java b/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java index 871105db5747..dbc41b87909a 100644 --- a/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java +++ b/processing/src/test/java/io/druid/segment/filter/LongFilteringTest.java @@ -132,12 +132,12 @@ public void testTimeFilterAsLong() ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, null, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, null, null, StringComparators.NUMERIC), ImmutableList.of("2", "3", "4", "5") ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "1", "4", true, true, null, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter(COUNT_COLUMN, "1", "4", true, true, null, null, StringComparators.NUMERIC), ImmutableList.of("2", "3") ); @@ -196,11 +196,11 @@ public void testLongFilterWithExtractionFn() ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "Fridax", "Fridaz", false, false, null, exfn, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter(COUNT_COLUMN, "Fridax", "Fridaz", false, false, null, exfn, StringComparators.ALPHANUMERIC), ImmutableList.of("5") ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "Friday", "Friday", true, true, null, exfn, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter(COUNT_COLUMN, "Friday", "Friday", true, true, null, exfn, StringComparators.ALPHANUMERIC), ImmutableList.of() ); @@ -261,7 +261,7 @@ public void testMultithreaded() ); assertFilterMatches( - new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, null, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter(COUNT_COLUMN, "2", "5", false, false, null, null, StringComparators.NUMERIC), ImmutableList.of("2", "3", "4", "5") ); } diff --git a/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java b/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java index 422639ce1a9c..0461e4f8e42e 100644 --- a/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java +++ b/processing/src/test/java/io/druid/segment/filter/TimeFilteringTest.java @@ -116,11 +116,11 @@ public void testTimeFilterAsLong() ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", false, false, null, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", false, false, null, null, StringComparators.NUMERIC), ImmutableList.of("0", "1", "2", "3", "4") ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", true, true, null, null, StringComparators.NUMERIC_NAME), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "0", "4", true, true, null, null, StringComparators.NUMERIC), ImmutableList.of("1", "2", "3") ); @@ -179,11 +179,11 @@ public void testTimeFilterWithExtractionFn() ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "Fridax", "Fridaz", false, false, null, exfn, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "Fridax", "Fridaz", false, false, null, exfn, StringComparators.ALPHANUMERIC), ImmutableList.of("4") ); assertFilterMatches( - new BoundDimFilter(Column.TIME_COLUMN_NAME, "Friday", "Friday", true, true, null, exfn, StringComparators.ALPHANUMERIC_NAME), + new BoundDimFilter(Column.TIME_COLUMN_NAME, "Friday", "Friday", true, true, null, exfn, StringComparators.ALPHANUMERIC), ImmutableList.of() ); diff --git a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java index d85c6f964f84..c30fc07bbf3a 100644 --- a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java @@ -1405,7 +1405,7 @@ public void testTimeSeriesWithFilter() throws Exception Arrays.asList( new SelectorDimFilter("dim0", "1", null), new BoundDimFilter("dim0", "222", "333", false, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ) ).build(), @@ -1413,10 +1413,10 @@ public void testTimeSeriesWithFilter() throws Exception Arrays.asList( new InDimFilter("dim1", Arrays.asList("0", "1", "2", "3", "4"), null), new BoundDimFilter("dim1", "0", "3", false, true, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ), new BoundDimFilter("dim1", "1", "9999", true, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ) ).build() @@ -1483,7 +1483,7 @@ public void testSingleDimensionPruning() throws Exception Arrays.asList( new SelectorDimFilter("dim1", "a", null), new BoundDimFilter("dim1", "from", "to", false, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ) ).build(), @@ -1491,10 +1491,10 @@ public void testSingleDimensionPruning() throws Exception Arrays.asList( new InDimFilter("dim2", Arrays.asList("a", "c", "e", "g"), null), new BoundDimFilter("dim2", "aaa", "hi", false, false, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ), new BoundDimFilter("dim2", "e", "zzz", true, true, false, null, - StringComparators.LEXICOGRAPHIC_NAME + StringComparators.LEXICOGRAPHIC ) ) ).build() From fd463ad274f812857e8a14e95cc4d4671ccbbf96 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 19:08:08 -0700 Subject: [PATCH 08/18] Add new TopNMetricSpec and SearchSortSpec with tests (WIP) --- .../search/search/NewSearchSortSpec.java | 92 +++++++++ .../query/topn/DimensionTopNMetricSpec.java | 182 ++++++++++++++++++ .../io/druid/query/topn/TopNMetricSpec.java | 2 +- .../query/search/NewSearchSortSpecTest.java | 99 ++++++++++ .../topn/DimensionTopNMetricSpecTest.java | 116 +++++++++++ 5 files changed, 490 insertions(+), 1 deletion(-) create mode 100644 processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java create mode 100644 processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java create mode 100644 processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java create mode 100644 processing/src/test/java/io/druid/query/topn/DimensionTopNMetricSpecTest.java diff --git a/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java new file mode 100644 index 000000000000..8710f00b9210 --- /dev/null +++ b/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java @@ -0,0 +1,92 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.metamx.common.StringUtils; +import io.druid.query.ordering.StringComparator; +import io.druid.query.ordering.StringComparators; + +import java.util.Comparator; + +public class NewSearchSortSpec +{ + public static final StringComparator DEFAULT_ORDERING = StringComparators.LEXICOGRAPHIC; + private final StringComparator ordering; + + public NewSearchSortSpec( + @JsonProperty StringComparator ordering + ) + { + this.ordering = ordering == null ? DEFAULT_ORDERING : ordering; + } + + public Comparator getComparator() + { + return new Comparator() + { + @Override + public int compare(SearchHit searchHit, SearchHit searchHit1) + { + int retVal = ordering.compare( + searchHit.getValue(), searchHit1.getValue()); + + if (retVal == 0) { + retVal = StringComparators.LEXICOGRAPHIC.compare( + searchHit.getDimension(), searchHit1.getDimension()); + } + return retVal; + } + }; + } + + public byte[] getCacheKey() + { + byte[] orderingBytes = StringUtils.toUtf8(ordering.toString()); + return orderingBytes; + } + + public String toString() + { + return String.format("%sSort", ordering.toString()); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + NewSearchSortSpec that = (NewSearchSortSpec) o; + + return ordering.equals(that.ordering); + + } + + @Override + public int hashCode() + { + return ordering.hashCode(); + } +} \ No newline at end of file diff --git a/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java new file mode 100644 index 000000000000..3a33b00ff02b --- /dev/null +++ b/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java @@ -0,0 +1,182 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.topn; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.metamx.common.StringUtils; +import io.druid.query.aggregation.AggregatorFactory; +import io.druid.query.aggregation.PostAggregator; +import io.druid.query.dimension.DimensionSpec; +import io.druid.query.ordering.StringComparator; +import io.druid.query.ordering.StringComparators; + +import org.joda.time.DateTime; + +import java.nio.ByteBuffer; +import java.util.Comparator; +import java.util.List; + +/** + */ +public class DimensionTopNMetricSpec implements TopNMetricSpec +{ + private static final StringComparator DEFAULT_ORDERING = StringComparators.LEXICOGRAPHIC; + public static final byte STRING_SEPARATOR = (byte) 0xFF; + + private static final byte CACHE_TYPE_ID = 0x4; + private final String previousStop; + private final StringComparator ordering; + + @JsonCreator + public DimensionTopNMetricSpec( + @JsonProperty("previousStop") String previousStop, + @JsonProperty("ordering") StringComparator ordering + ) + { + this.previousStop = previousStop; + this.ordering = ordering == null ? DEFAULT_ORDERING : ordering; + } + + @Override + public void verifyPreconditions(List aggregatorSpecs, List postAggregatorSpecs) + { + } + + @JsonProperty + public String getPreviousStop() + { + return previousStop; + } + + @JsonProperty + public StringComparator getOrdering() + { + return ordering; + } + + @Override + public Comparator getComparator(List aggregatorSpecs, List postAggregatorSpecs) + { + return ordering; + } + + @Override + public TopNResultBuilder getResultBuilder( + DateTime timestamp, + DimensionSpec dimSpec, + int threshold, + Comparator comparator, + List aggFactories, + List postAggs + ) + { + return new TopNLexicographicResultBuilder( + timestamp, + dimSpec, + threshold, + previousStop, + comparator, + aggFactories + ); + } + + @Override + public byte[] getCacheKey() + { + byte[] previousStopBytes = previousStop == null ? new byte[]{} : StringUtils.toUtf8(previousStop); + byte[] orderingBytes = StringUtils.toUtf8(ordering.toString()); + + int totalLen = 2 + previousStopBytes.length + orderingBytes.length; + + return ByteBuffer.allocate(totalLen) + .put(CACHE_TYPE_ID) + .put(previousStopBytes) + .put(STRING_SEPARATOR) + .put(orderingBytes) + .array(); + } + + @Override + public TopNMetricSpecBuilder configureOptimizer(TopNMetricSpecBuilder builder) + { + if (ordering.equals(StringComparators.LEXICOGRAPHIC)) { + builder.skipTo(previousStop); + builder.ignoreAfterThreshold(); + } + return builder; + } + + @Override + public void initTopNAlgorithmSelector(TopNAlgorithmSelector selector) + { + selector.setAggregateAllMetrics(true); + } + + @Override + public String getMetricName(DimensionSpec dimSpec) + { + return dimSpec.getOutputName(); + } + + @Override + public boolean canBeOptimizedUnordered() + { + return false; + } + + @Override + public String toString() + { + return "DimensionTopNMetricSpec{" + + "previousStop='" + previousStop + '\'' + + "ordering='" + ordering + '\'' + + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + DimensionTopNMetricSpec that = (DimensionTopNMetricSpec) o; + + if (getPreviousStop() != null + ? !getPreviousStop().equals(that.getPreviousStop()) + : that.getPreviousStop() != null) { + return false; + } + return getOrdering().equals(that.getOrdering()); + + } + + @Override + public int hashCode() + { + int result = getPreviousStop() != null ? getPreviousStop().hashCode() : 0; + result = 31 * result + getOrdering().hashCode(); + return result; + } +} \ No newline at end of file diff --git a/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java index 4f428c547b7b..2c2894125974 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java +++ b/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java @@ -37,7 +37,7 @@ @JsonSubTypes.Type(name = "lexicographic", value = LexicographicTopNMetricSpec.class), @JsonSubTypes.Type(name = "alphaNumeric", value = AlphaNumericTopNMetricSpec.class), @JsonSubTypes.Type(name = "inverted", value = InvertedTopNMetricSpec.class), - @JsonSubTypes.Type(name = "numericDimension", value = NumericDimensionTopNMetricSpec.class), + @JsonSubTypes.Type(name = "dimension", value = DimensionTopNMetricSpec.class), }) public interface TopNMetricSpec { diff --git a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java new file mode 100644 index 000000000000..9903a1922837 --- /dev/null +++ b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java @@ -0,0 +1,99 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.search; + +import io.druid.query.ordering.StringComparators; +import io.druid.query.search.search.AlphanumericSearchSortSpec; +import io.druid.query.search.search.LexicographicSearchSortSpec; +import io.druid.query.search.search.NewSearchSortSpec; +import io.druid.query.search.search.NumericSearchSortSpec; +import io.druid.query.search.search.SearchHit; +import io.druid.query.search.search.SearchSortSpec; +import org.junit.Assert; +import org.junit.Test; + +/** + */ +public class NewSearchSortSpecTest +{ + @Test + public void testLexicographicComparator() + { + SearchHit hit1 = new SearchHit("test", "apple"); + SearchHit hit2 = new SearchHit("test", "banana"); + SearchHit hit3 = new SearchHit("test", "banana"); + + NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + + Assert.assertTrue(spec.getComparator().compare(hit2, hit3) == 0); + Assert.assertTrue(spec.getComparator().compare(hit2, hit1) > 0); + Assert.assertTrue(spec.getComparator().compare(hit1, hit3) < 0); + } + + @Test + public void testAlphanumericComparator() + { + NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.ALPHANUMERIC); + + SearchHit hit1 = new SearchHit("test", "a100"); + SearchHit hit2 = new SearchHit("test", "a9"); + SearchHit hit3 = new SearchHit("test", "b0"); + + Assert.assertTrue(spec.getComparator().compare(hit1, hit2) > 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit1) > 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit2) > 0); + } + + @Test + public void testNumericComparator() + { + NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.NUMERIC); + + SearchHit hit1 = new SearchHit("test", "1001001.12412"); + SearchHit hit2 = new SearchHit("test", "-1421"); + SearchHit hit3 = new SearchHit("test", "not-numeric-at-all"); + + SearchHit hit4 = new SearchHit("best", "1001001.12412"); + + + Assert.assertTrue(spec.getComparator().compare(hit1, hit2) > 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit1) < 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit2) < 0); + + Assert.assertTrue(spec.getComparator().compare(hit1, hit4) > 0); + } + + @Test + public void testStrlenComparator() + { + NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + + SearchHit hit1 = new SearchHit("test", "apple"); + SearchHit hit2 = new SearchHit("test", "banana"); + SearchHit hit3 = new SearchHit("test", "orange"); + + Assert.assertTrue(spec.getComparator().compare(hit1, hit2) < 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit1) > 0); + Assert.assertTrue(spec.getComparator().compare(hit3, hit2) > 0); + + Assert.assertTrue(spec.getComparator().compare(hit1, hit1) == 0); + } + +} diff --git a/processing/src/test/java/io/druid/query/topn/DimensionTopNMetricSpecTest.java b/processing/src/test/java/io/druid/query/topn/DimensionTopNMetricSpecTest.java new file mode 100644 index 000000000000..f692dde7ffc0 --- /dev/null +++ b/processing/src/test/java/io/druid/query/topn/DimensionTopNMetricSpecTest.java @@ -0,0 +1,116 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.topn; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; +import io.druid.jackson.DefaultObjectMapper; +import io.druid.query.ordering.StringComparators; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +public class DimensionTopNMetricSpecTest +{ + @Test + public void testSerdeAlphaNumericDimensionTopNMetricSpec() throws IOException{ + DimensionTopNMetricSpec expectedMetricSpec = new DimensionTopNMetricSpec(null, StringComparators.ALPHANUMERIC); + DimensionTopNMetricSpec expectedMetricSpec1 = new DimensionTopNMetricSpec("test", StringComparators.ALPHANUMERIC); + String jsonSpec = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"alphanumeric\"\n" + + "}"; + String jsonSpec1 = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"alphanumeric\",\n" + + " \"previousStop\": \"test\"\n" + + "}"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + TopNMetricSpec actualMetricSpec = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + TopNMetricSpec actualMetricSpec1 = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec1, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + Assert.assertEquals(expectedMetricSpec, actualMetricSpec); + Assert.assertEquals(expectedMetricSpec1, actualMetricSpec1); + } + + @Test + public void testSerdeLexicographicDimensionTopNMetricSpec() throws IOException{ + DimensionTopNMetricSpec expectedMetricSpec = new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC); + DimensionTopNMetricSpec expectedMetricSpec1 = new DimensionTopNMetricSpec("test", StringComparators.LEXICOGRAPHIC); + String jsonSpec = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"lexicographic\"\n" + + "}"; + String jsonSpec1 = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"lexicographic\",\n" + + " \"previousStop\": \"test\"\n" + + "}"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + TopNMetricSpec actualMetricSpec = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + TopNMetricSpec actualMetricSpec1 = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec1, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + Assert.assertEquals(expectedMetricSpec, actualMetricSpec); + Assert.assertEquals(expectedMetricSpec1, actualMetricSpec1); + } + + @Test + public void testSerdeStrlenDimensionTopNMetricSpec() throws IOException{ + DimensionTopNMetricSpec expectedMetricSpec = new DimensionTopNMetricSpec(null, StringComparators.STRLEN); + DimensionTopNMetricSpec expectedMetricSpec1 = new DimensionTopNMetricSpec("test", StringComparators.STRLEN); + String jsonSpec = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"strlen\"\n" + + "}"; + String jsonSpec1 = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"strlen\",\n" + + " \"previousStop\": \"test\"\n" + + "}"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + TopNMetricSpec actualMetricSpec = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + TopNMetricSpec actualMetricSpec1 = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec1, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + Assert.assertEquals(expectedMetricSpec, actualMetricSpec); + Assert.assertEquals(expectedMetricSpec1, actualMetricSpec1); + } + + @Test + public void testSerdeNumericDimensionTopNMetricSpec() throws IOException{ + DimensionTopNMetricSpec expectedMetricSpec = new DimensionTopNMetricSpec(null, StringComparators.NUMERIC); + DimensionTopNMetricSpec expectedMetricSpec1 = new DimensionTopNMetricSpec("test", StringComparators.NUMERIC); + String jsonSpec = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"numeric\"\n" + + "}"; + String jsonSpec1 = "{\n" + + " \"type\": \"dimension\"," + + " \"ordering\": \"numeric\",\n" + + " \"previousStop\": \"test\"\n" + + "}"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + TopNMetricSpec actualMetricSpec = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + TopNMetricSpec actualMetricSpec1 = jsonMapper.readValue(jsonMapper.writeValueAsString(jsonMapper.readValue(jsonSpec1, TopNMetricSpec.class)), DimensionTopNMetricSpec.class); + Assert.assertEquals(expectedMetricSpec, actualMetricSpec); + Assert.assertEquals(expectedMetricSpec1, actualMetricSpec1); + } +} From 79a0e4b05f467042e1d1a6b028f1d0ad3c69caca Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 19:41:35 -0700 Subject: [PATCH 09/18] More TopNMetricSpec and SearchSortSpec tests --- .../src/main/java/io/druid/query/Druids.java | 5 +- .../io/druid/query/search/SearchBinaryFn.java | 6 +- .../search/search/NumericSearchSortSpec.java | 79 ------------------- .../query/search/search/SearchQuery.java | 11 ++- .../query/search/NewSearchSortSpecTest.java | 4 - .../search/NumericSearchSortSpecTest.java | 51 ------------ .../query/search/SearchBinaryFnTest.java | 25 +++--- .../query/search/SearchQueryRunnerTest.java | 6 +- .../io/druid/query/topn/TopNBinaryFnTest.java | 3 +- .../druid/query/topn/TopNQueryRunnerTest.java | 33 ++++---- .../io/druid/query/topn/TopNQueryTest.java | 6 +- 11 files changed, 52 insertions(+), 177 deletions(-) delete mode 100644 processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java delete mode 100644 processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java diff --git a/processing/src/main/java/io/druid/query/Druids.java b/processing/src/main/java/io/druid/query/Druids.java index 1963592e80e3..e9db3c2389e6 100644 --- a/processing/src/main/java/io/druid/query/Druids.java +++ b/processing/src/main/java/io/druid/query/Druids.java @@ -44,6 +44,7 @@ import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.query.search.search.FragmentSearchQuerySpec; import io.druid.query.search.search.InsensitiveContainsSearchQuerySpec; +import io.druid.query.search.search.NewSearchSortSpec; import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQuerySpec; import io.druid.query.search.search.SearchSortSpec; @@ -546,7 +547,7 @@ public static class SearchQueryBuilder private QuerySegmentSpec querySegmentSpec; private List dimensions; private SearchQuerySpec querySpec; - private SearchSortSpec sortSpec; + private NewSearchSortSpec sortSpec; private Map context; public SearchQueryBuilder() @@ -731,7 +732,7 @@ public SearchQueryBuilder fragments(List q) return fragments(q, false); } - public SearchQueryBuilder sortSpec(SearchSortSpec sortSpec) + public SearchQueryBuilder sortSpec(NewSearchSortSpec sortSpec) { this.sortSpec = sortSpec; return this; diff --git a/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java b/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java index 734bdbd22609..5cdbf038d82b 100644 --- a/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java +++ b/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java @@ -25,8 +25,8 @@ import io.druid.granularity.AllGranularity; import io.druid.granularity.QueryGranularity; import io.druid.query.Result; +import io.druid.query.search.search.NewSearchSortSpec; import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.SearchSortSpec; import org.joda.time.DateTime; import java.util.Arrays; @@ -37,12 +37,12 @@ public class SearchBinaryFn implements BinaryFn, Result, Result> { - private final SearchSortSpec searchSortSpec; + private final NewSearchSortSpec searchSortSpec; private final QueryGranularity gran; private final int limit; public SearchBinaryFn( - SearchSortSpec searchSortSpec, + NewSearchSortSpec searchSortSpec, QueryGranularity granularity, int limit ) diff --git a/processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java deleted file mode 100644 index 2f19294f8ea1..000000000000 --- a/processing/src/main/java/io/druid/query/search/search/NumericSearchSortSpec.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; - -import com.fasterxml.jackson.annotation.JsonCreator; - -import io.druid.query.ordering.StringComparators; - -import java.util.Comparator; - -/** - */ -public class NumericSearchSortSpec implements SearchSortSpec -{ - @JsonCreator - public NumericSearchSortSpec( - ) - { - } - - @Override - public Comparator getComparator() - { - return new Comparator() - { - @Override - public int compare(SearchHit searchHit, SearchHit searchHit1) - { - int retVal = StringComparators.NUMERIC.compare( - searchHit.getValue(), searchHit1.getValue()); - - if (retVal == 0) { - retVal = StringComparators.LEXICOGRAPHIC.compare( - searchHit.getDimension(), searchHit1.getDimension()); - } - return retVal; - } - }; - } - - @Override - public byte[] getCacheKey() - { - return toString().getBytes(); - } - - public String toString() - { - return "numericSort"; - } - - @Override - public boolean equals(Object other) { - return this == other || other instanceof NumericSearchSortSpec; - } - - @Override - public int hashCode() - { - return 0; - } -} diff --git a/processing/src/main/java/io/druid/query/search/search/SearchQuery.java b/processing/src/main/java/io/druid/query/search/search/SearchQuery.java index 00c8106483d1..b58eff773f5a 100644 --- a/processing/src/main/java/io/druid/query/search/search/SearchQuery.java +++ b/processing/src/main/java/io/druid/query/search/search/SearchQuery.java @@ -30,6 +30,7 @@ import io.druid.query.Result; import io.druid.query.dimension.DimensionSpec; import io.druid.query.filter.DimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.SearchResultValue; import io.druid.query.spec.QuerySegmentSpec; @@ -40,8 +41,10 @@ */ public class SearchQuery extends BaseQuery> { + private static final NewSearchSortSpec DEFAULT_SORT_SPEC = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + private final DimFilter dimFilter; - private final SearchSortSpec sortSpec; + private final NewSearchSortSpec sortSpec; private final QueryGranularity granularity; private final List dimensions; private final SearchQuerySpec querySpec; @@ -56,13 +59,13 @@ public SearchQuery( @JsonProperty("intervals") QuerySegmentSpec querySegmentSpec, @JsonProperty("searchDimensions") List dimensions, @JsonProperty("query") SearchQuerySpec querySpec, - @JsonProperty("sort") SearchSortSpec sortSpec, + @JsonProperty("sort") NewSearchSortSpec sortSpec, @JsonProperty("context") Map context ) { super(dataSource, querySegmentSpec, false, context); this.dimFilter = dimFilter; - this.sortSpec = sortSpec == null ? new LexicographicSearchSortSpec() : sortSpec; + this.sortSpec = sortSpec == null ? DEFAULT_SORT_SPEC : sortSpec; this.granularity = granularity == null ? QueryGranularities.ALL : granularity; this.limit = (limit == 0) ? 1000 : limit; this.dimensions = dimensions; @@ -183,7 +186,7 @@ public SearchQuerySpec getQuery() } @JsonProperty("sort") - public SearchSortSpec getSort() + public NewSearchSortSpec getSort() { return sortSpec; } diff --git a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java index 9903a1922837..bf2e6c0cd77d 100644 --- a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java +++ b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java @@ -20,12 +20,8 @@ package io.druid.query.search; import io.druid.query.ordering.StringComparators; -import io.druid.query.search.search.AlphanumericSearchSortSpec; -import io.druid.query.search.search.LexicographicSearchSortSpec; import io.druid.query.search.search.NewSearchSortSpec; -import io.druid.query.search.search.NumericSearchSortSpec; import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.SearchSortSpec; import org.junit.Assert; import org.junit.Test; diff --git a/processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java deleted file mode 100644 index b2ed96263e5e..000000000000 --- a/processing/src/test/java/io/druid/query/search/NumericSearchSortSpecTest.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search; - -import io.druid.query.search.search.AlphanumericSearchSortSpec; -import io.druid.query.search.search.NumericSearchSortSpec; -import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.SearchSortSpec; -import org.junit.Assert; -import org.junit.Test; - -/** - */ -public class NumericSearchSortSpecTest -{ - @Test - public void testComparator() - { - SearchSortSpec spec = new NumericSearchSortSpec(); - - SearchHit hit1 = new SearchHit("test", "1001001.12412"); - SearchHit hit2 = new SearchHit("test", "-1421"); - SearchHit hit3 = new SearchHit("test", "not-numeric-at-all"); - - SearchHit hit4 = new SearchHit("best", "1001001.12412"); - - - Assert.assertTrue(spec.getComparator().compare(hit1, hit2) > 0); - Assert.assertTrue(spec.getComparator().compare(hit3, hit1) < 0); - Assert.assertTrue(spec.getComparator().compare(hit3, hit2) < 0); - - Assert.assertTrue(spec.getComparator().compare(hit1, hit4) > 0); - } -} diff --git a/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java b/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java index 1ab58720bc06..289eb8cd76d2 100644 --- a/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java @@ -22,10 +22,9 @@ import com.google.common.collect.ImmutableList; import io.druid.granularity.QueryGranularities; import io.druid.query.Result; -import io.druid.query.search.search.AlphanumericSearchSortSpec; -import io.druid.query.search.search.LexicographicSearchSortSpec; +import io.druid.query.ordering.StringComparators; +import io.druid.query.search.search.NewSearchSortSpec; import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.StrlenSearchSortSpec; import org.joda.time.DateTime; import org.junit.Assert; import org.junit.Test; @@ -98,7 +97,7 @@ public void testMerge() ) ); - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -146,7 +145,7 @@ public void testMergeDay() ) ); - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.DAY, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.DAY, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -170,7 +169,7 @@ public void testMergeOneResultNull() Result expected = r1; - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -218,7 +217,7 @@ public void testMergeShiftedTimestamp() ) ); - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -226,7 +225,7 @@ public void testMergeShiftedTimestamp() @Test public void testStrlenMerge() { - StrlenSearchSortSpec searchSortSpec = new StrlenSearchSortSpec(); + NewSearchSortSpec searchSortSpec = new NewSearchSortSpec(StringComparators.STRLEN); Comparator c = searchSortSpec.getComparator(); Result r1 = new Result( @@ -252,7 +251,7 @@ public void testStrlenMerge() @Test public void testStrlenMerge2() { - StrlenSearchSortSpec searchSortSpec = new StrlenSearchSortSpec(); + NewSearchSortSpec searchSortSpec = new NewSearchSortSpec(StringComparators.STRLEN); Comparator c = searchSortSpec.getComparator(); Result r1 = new Result( @@ -278,7 +277,7 @@ public void testStrlenMerge2() @Test public void testAlphanumericMerge() { - AlphanumericSearchSortSpec searchSortSpec = new AlphanumericSearchSortSpec(); + NewSearchSortSpec searchSortSpec = new NewSearchSortSpec(StringComparators.ALPHANUMERIC); Comparator c = searchSortSpec.getComparator(); Result r1 = new Result( @@ -332,7 +331,7 @@ public void testMergeUniqueResults() Result expected = r1; - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -363,7 +362,7 @@ public void testMergeLimit(){ ) ); Result expected = r1; - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.ALL, 1).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, 1).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -397,7 +396,7 @@ public void testMergeCountWithNull() { Result expected = r1; - Result actual = new SearchBinaryFn(new LexicographicSearchSortSpec(), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } diff --git a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java index 13d09d70ef84..84c47c4e84ae 100644 --- a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java @@ -38,7 +38,9 @@ import io.druid.query.filter.ExtractionDimFilter; import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SelectorDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.FragmentSearchQuerySpec; +import io.druid.query.search.search.NewSearchSortSpec; import io.druid.query.search.search.NumericSearchSortSpec; import io.druid.query.search.search.SearchHit; import io.druid.query.search.search.SearchQuery; @@ -214,7 +216,7 @@ public void testSearchSameValueInMultiDims2() QueryRunnerTestHelper.placementishDimension ) ) - .sortSpec(new StrlenSearchSortSpec()) + .sortSpec(new NewSearchSortSpec(StringComparators.STRLEN)) .query("e") .build(); @@ -588,7 +590,7 @@ public void testSearchWithNumericSort() .granularity(QueryRunnerTestHelper.allGran) .intervals(QueryRunnerTestHelper.fullOnInterval) .query("a") - .sortSpec(new NumericSearchSortSpec()) + .sortSpec(new NewSearchSortSpec(StringComparators.NUMERIC)) .build(); List expectedHits = Lists.newLinkedList(); diff --git a/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java b/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java index fa662460bd39..4d136590544d 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java @@ -32,6 +32,7 @@ import io.druid.query.aggregation.post.ConstantPostAggregator; import io.druid.query.aggregation.post.FieldAccessPostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; +import io.druid.query.ordering.StringComparators; import org.joda.time.DateTime; import org.junit.Assert; import org.junit.Test; @@ -509,7 +510,7 @@ public void testMergeLexicographicWithInvalidDimName() TopNResultMerger.identity, QueryGranularities.ALL, new DefaultDimensionSpec("INVALID_DIM_NAME", null), - new LexicographicTopNMetricSpec(null), + new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC), 2, aggregatorFactories, postAggregators diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java index 631fe443050f..8c23a4457cbc 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java @@ -62,6 +62,7 @@ import io.druid.query.filter.DimFilter; import io.druid.query.filter.ExtractionDimFilter; import io.druid.query.filter.SelectorDimFilter; +import io.druid.query.ordering.StringComparators; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.query.timeseries.TimeseriesQuery; import io.druid.segment.TestHelper; @@ -1299,7 +1300,7 @@ public void testTopNLexicographic() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryRunnerTestHelper.allGran) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new LexicographicTopNMetricSpec("")) + .metric(new DimensionTopNMetricSpec("", StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -1346,7 +1347,7 @@ public void testTopNLexicographicNoAggregators() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryRunnerTestHelper.allGran) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new LexicographicTopNMetricSpec("")) + .metric(new DimensionTopNMetricSpec("", StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .build(); @@ -1379,7 +1380,7 @@ public void testTopNLexicographicWithPreviousStop() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryRunnerTestHelper.allGran) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new LexicographicTopNMetricSpec("spot")) + .metric(new DimensionTopNMetricSpec("spot", StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -1419,7 +1420,7 @@ public void testTopNLexicographicWithNonExistingPreviousStop() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryRunnerTestHelper.allGran) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new LexicographicTopNMetricSpec("t")) + .metric(new DimensionTopNMetricSpec("t", StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -1459,7 +1460,7 @@ public void testTopNInvertedLexicographicWithPreviousStop() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryRunnerTestHelper.allGran) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new InvertedTopNMetricSpec(new LexicographicTopNMetricSpec("upfront"))) + .metric(new InvertedTopNMetricSpec(new DimensionTopNMetricSpec("upfront", StringComparators.LEXICOGRAPHIC))) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -1499,7 +1500,7 @@ public void testTopNInvertedLexicographicWithNonExistingPreviousStop() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryRunnerTestHelper.allGran) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new InvertedTopNMetricSpec(new LexicographicTopNMetricSpec("u"))) + .metric(new InvertedTopNMetricSpec(new DimensionTopNMetricSpec("u", StringComparators.LEXICOGRAPHIC))) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -1986,7 +1987,7 @@ public void testTopNLexicographicDimExtractionOptimalNamespace() null ) ) - .metric(new LexicographicTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2053,7 +2054,7 @@ public void testTopNLexicographicDimExtractionUnOptimalNamespace() null ) ) - .metric(new LexicographicTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2121,7 +2122,7 @@ public void testTopNLexicographicDimExtractionOptimalNamespaceWithRunner() null ) ) - .metric(new LexicographicTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2175,7 +2176,7 @@ public void testTopNLexicographicDimExtraction() null ) ) - .metric(new LexicographicTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2229,7 +2230,7 @@ public void testInvertedTopNLexicographicDimExtraction2() null ) ) - .metric(new InvertedTopNMetricSpec(new LexicographicTopNMetricSpec(null))) + .metric(new InvertedTopNMetricSpec(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC))) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2283,7 +2284,7 @@ public void testTopNLexicographicDimExtractionWithPreviousStop() null ) ) - .metric(new LexicographicTopNMetricSpec("s")) + .metric(new DimensionTopNMetricSpec("s", StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2353,7 +2354,7 @@ public ExtractionType getExtractionType() }, null ) ) - .metric(new LexicographicTopNMetricSpec("s")) + .metric(new DimensionTopNMetricSpec("s", StringComparators.LEXICOGRAPHIC)) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2401,7 +2402,7 @@ public void testInvertedTopNLexicographicDimExtractionWithPreviousStop() null ) ) - .metric(new InvertedTopNMetricSpec(new LexicographicTopNMetricSpec("u"))) + .metric(new InvertedTopNMetricSpec(new DimensionTopNMetricSpec("u", StringComparators.LEXICOGRAPHIC))) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -2448,7 +2449,7 @@ public void testInvertedTopNLexicographicDimExtractionWithPreviousStop2() null ) ) - .metric(new InvertedTopNMetricSpec(new LexicographicTopNMetricSpec("p"))) + .metric(new InvertedTopNMetricSpec(new DimensionTopNMetricSpec("p", StringComparators.LEXICOGRAPHIC))) .threshold(4) .intervals(QueryRunnerTestHelper.firstToThird) .aggregators(QueryRunnerTestHelper.commonAggregators) @@ -3225,7 +3226,7 @@ public void testAlphaNumericTopNWithNullPreviousStop() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryGranularities.ALL) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new AlphaNumericTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.ALPHANUMERIC)) .threshold(2) .intervals(QueryRunnerTestHelper.secondOnly) .aggregators(Lists.newArrayList(QueryRunnerTestHelper.rowsCount)) diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryTest.java index de034d5d9cb1..7aac6855484a 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryTest.java @@ -33,6 +33,7 @@ import io.druid.query.dimension.LegacyDimensionSpec; import io.druid.query.lookup.LookupExtractionFn; import io.druid.query.extraction.MapLookupExtractor; +import io.druid.query.ordering.StringComparators; import org.junit.Assert; import org.junit.Test; @@ -123,7 +124,7 @@ public void testQuerySerdeWithAlphaNumericTopNMetricSpec() throws IOException .dataSource(dataSource) .granularity(allGran) .dimension(new LegacyDimensionSpec(marketDimension)) - .metric(new AlphaNumericTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.ALPHANUMERIC)) .threshold(2) .intervals(fullOnInterval.getIntervals()) .aggregators(Lists.newArrayList(rowsCount)) @@ -134,7 +135,8 @@ public void testQuerySerdeWithAlphaNumericTopNMetricSpec() throws IOException + " \"dimension\": \"market\",\n" + " \"threshold\": 2,\n" + " \"metric\": {\n" - + " \"type\": \"alphaNumeric\"\n" + + " \"type\": \"dimension\",\n" + + " \"ordering\": \"alphanumeric\"\n" + " },\n" + " \"granularity\": \"all\",\n" + " \"aggregations\": [\n" From fd7ad38913c53fd9e4f9463e34cb1bd1644fdfe9 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 20:39:37 -0700 Subject: [PATCH 10/18] Fix NewSearchSortSpec serde --- .../src/main/java/io/druid/query/Druids.java | 1 - .../query/search/search/NewSearchSortSpec.java | 11 ++++++++++- .../query/search/NewSearchSortSpecTest.java | 16 ++++++++++++++++ .../query/search/SearchQueryRunnerTest.java | 4 ---- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/processing/src/main/java/io/druid/query/Druids.java b/processing/src/main/java/io/druid/query/Druids.java index e9db3c2389e6..4c722b0c613b 100644 --- a/processing/src/main/java/io/druid/query/Druids.java +++ b/processing/src/main/java/io/druid/query/Druids.java @@ -47,7 +47,6 @@ import io.druid.query.search.search.NewSearchSortSpec; import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQuerySpec; -import io.druid.query.search.search.SearchSortSpec; import io.druid.query.select.PagingSpec; import io.druid.query.select.SelectQuery; import io.druid.query.spec.LegacySegmentSpec; diff --git a/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java index 8710f00b9210..edc0998b28aa 100644 --- a/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java +++ b/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java @@ -19,6 +19,7 @@ package io.druid.query.search.search; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.metamx.common.StringUtils; import io.druid.query.ordering.StringComparator; @@ -29,15 +30,23 @@ public class NewSearchSortSpec { public static final StringComparator DEFAULT_ORDERING = StringComparators.LEXICOGRAPHIC; + private final StringComparator ordering; + @JsonCreator public NewSearchSortSpec( - @JsonProperty StringComparator ordering + @JsonProperty("ordering") StringComparator ordering ) { this.ordering = ordering == null ? DEFAULT_ORDERING : ordering; } + @JsonProperty("ordering") + public StringComparator getOrdering() + { + return ordering; + } + public Comparator getComparator() { return new Comparator() diff --git a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java index bf2e6c0cd77d..3f9f22b00b70 100644 --- a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java +++ b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java @@ -19,12 +19,16 @@ package io.druid.query.search; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.druid.jackson.DefaultObjectMapper; import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.NewSearchSortSpec; import io.druid.query.search.search.SearchHit; import org.junit.Assert; import org.junit.Test; +import java.io.IOException; + /** */ public class NewSearchSortSpecTest @@ -92,4 +96,16 @@ public void testStrlenComparator() Assert.assertTrue(spec.getComparator().compare(hit1, hit1) == 0); } + + @Test + public void testSerde() throws IOException + { + ObjectMapper jsonMapper = new DefaultObjectMapper(); + NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + + String expectJsonSpec = "{\"ordering\":{\"type\":\"lexicographic\"}}"; + String jsonSpec = jsonMapper.writeValueAsString(spec); + Assert.assertEquals(expectJsonSpec, jsonSpec); + Assert.assertEquals(spec, jsonMapper.readValue(jsonSpec, NewSearchSortSpec.class)); + } } diff --git a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java index 84c47c4e84ae..6f727e29baba 100644 --- a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java @@ -19,7 +19,6 @@ package io.druid.query.search; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.metamx.common.guava.Sequence; @@ -36,16 +35,13 @@ import io.druid.query.filter.AndDimFilter; import io.druid.query.filter.DimFilter; import io.druid.query.filter.ExtractionDimFilter; -import io.druid.query.filter.RegexDimFilter; import io.druid.query.filter.SelectorDimFilter; import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.FragmentSearchQuerySpec; import io.druid.query.search.search.NewSearchSortSpec; -import io.druid.query.search.search.NumericSearchSortSpec; import io.druid.query.search.search.SearchHit; import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQueryConfig; -import io.druid.query.search.search.StrlenSearchSortSpec; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.segment.TestHelper; import org.joda.time.DateTime; From c423825089a99b35195bcab207445d983fd6eaf9 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 21:03:46 -0700 Subject: [PATCH 11/18] Update docs for new DimensionTopNMetricSpec --- docs/content/querying/topnmetricspec.md | 53 ++++--------------- .../query/search/NewSearchSortSpecTest.java | 5 ++ 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/docs/content/querying/topnmetricspec.md b/docs/content/querying/topnmetricspec.md index e193775f6355..391e779f86b5 100644 --- a/docs/content/querying/topnmetricspec.md +++ b/docs/content/querying/topnmetricspec.md @@ -6,8 +6,6 @@ TopNMetricSpec The topN metric spec specifies how topN values should be sorted. -See [Sorting Orders](./sorting-orders.html) for more information on the sorting orders used by the Lexicographic, Alphanumeric, and NumericDimension specs. - ## Numeric TopNMetricSpec The simplest metric specification is a String value indicating the metric to sort topN results by. They are included in a topN query with: @@ -30,39 +28,26 @@ The metric field can also be given as a JSON object. The grammar for dimension v |type|this indicates a numeric sort|yes| |metric|the actual metric field in which results will be sorted by|yes| -## Lexicographic TopNMetricSpec - -The grammar for dimension values sorted lexicographically is as follows: - -```json -"metric": { - "type": "lexicographic", - "previousStop": "" -} -``` +## Dimension TopNMetricSpec -|property|description|required?| -|--------|-----------|---------| -|type|this indicates a lexicographic sort|yes| -|previousStop|the starting point of the lexicographic sort. For example, if a previousStop value is 'b', all values before 'b' are discarded. This field can be used to paginate through all the dimension values.|no| +This metric specification sorts TopN results by dimension value, using one of the sorting orders described here: [Sorting Orders](./sorting-orders.html) -## AlphaNumeric TopNMetricSpec +|property|type|description|required?| +|--------|----|-----------|---------| +|type|String|this indicates a sort a dimension's values|yes, must be 'dimension'| +|ordering|String|Specifies the sorting order. Can be one of the following values: "lexicographic", "alphanumeric", "numeric", "strlen". See [Sorting Orders](./sorting-orders.html) for more details.|no, default: "lexicographic"| +|previousStop|String|the starting point of the sort. For example, if a previousStop value is 'b', all values before 'b' are discarded. This field can be used to paginate through all the dimension values.|no| -Sort dimension values in alpha-numeric order, i.e treating numbers differently from other characters in sorting the values. -The algorithm is based on [https://github.com/amjjd/java-alphanum](https://github.com/amjjd/java-alphanum). +The following metricSpec uses lexicographic sorting. ```json "metric": { - "type": "alphaNumeric", + "type": "dimension", + "ordering": "lexicographic", "previousStop": "" } ``` -|property|description|required?| -|--------|-----------|---------| -|type|this indicates an alpha-numeric sort|yes| -|previousStop|the starting point of the alpha-numeric sort. For example, if a previousStop value is 'b', all values before 'b' are discarded. This field can be used to paginate through all the dimension values.|no| - ## Inverted TopNMetricSpec Sort dimension values in inverted order, i.e inverts the order of the delegate metric spec. It can be used to sort the values in ascending order. @@ -77,20 +62,4 @@ Sort dimension values in inverted order, i.e inverts the order of the delegate m |property|description|required?| |--------|-----------|---------| |type|this indicates an inverted sort|yes| -|metric|the delegate metric spec. |yes| - -## NumericDimension TopNMetricSpec - -Sort dimension values in numeric order, i.e treating dimension values as numeric values. Unparseable string values will be sorted before valid numeric values. If two string values are unparseable, they will be compared lexicographically. - -```json -"metric": { - "type": "numericDimension", - "previousStop": "" -} -``` - -|property|description|required?| -|--------|-----------|---------| -|type|this indicates an alpha-numeric sort|yes| -|previousStop|the starting point of the alpha-numeric sort. For example, if a previousStop value is 'b', all values before 'b' are discarded. This field can be used to paginate through all the dimension values.|no| \ No newline at end of file +|metric|the delegate metric spec. |yes| \ No newline at end of file diff --git a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java index 3f9f22b00b70..f4325ff8ffdb 100644 --- a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java +++ b/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java @@ -107,5 +107,10 @@ public void testSerde() throws IOException String jsonSpec = jsonMapper.writeValueAsString(spec); Assert.assertEquals(expectJsonSpec, jsonSpec); Assert.assertEquals(spec, jsonMapper.readValue(jsonSpec, NewSearchSortSpec.class)); + + // this works too, without specifying "ordering"... + String expectJsonSpec2 = "{\"type\":\"lexicographic\"}"; + Assert.assertEquals(spec, jsonMapper.readValue(expectJsonSpec2, NewSearchSortSpec.class)); + } } From 5af2d3874462efa106a36296cee379d018e487dc Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 26 Jul 2016 21:12:19 -0700 Subject: [PATCH 12/18] Delete NumericDimensionTopNMetricSpec --- .../topn/NumericDimensionTopNMetricSpec.java | 77 ------------------- .../druid/query/topn/TopNQueryRunnerTest.java | 2 +- 2 files changed, 1 insertion(+), 78 deletions(-) delete mode 100644 processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java diff --git a/processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java deleted file mode 100644 index 8d0250c5356a..000000000000 --- a/processing/src/main/java/io/druid/query/topn/NumericDimensionTopNMetricSpec.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.topn; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.metamx.common.StringUtils; -import io.druid.query.aggregation.AggregatorFactory; -import io.druid.query.aggregation.PostAggregator; -import io.druid.query.ordering.StringComparators; - -import java.nio.ByteBuffer; -import java.util.Comparator; -import java.util.List; - -public class NumericDimensionTopNMetricSpec extends LexicographicTopNMetricSpec -{ - private static final byte CACHE_TYPE_ID = 0x4; - - protected static Comparator comparator = StringComparators.NUMERIC; - - @JsonCreator - public NumericDimensionTopNMetricSpec( - @JsonProperty("previousStop") String previousStop - ) - { - super(previousStop); - } - - @Override - public Comparator getComparator(List aggregatorSpecs, List postAggregatorSpecs) - { - return comparator; - } - - @Override - public byte[] getCacheKey() - { - byte[] previousStopBytes = getPreviousStop() == null ? new byte[]{} : StringUtils.toUtf8(getPreviousStop()); - - return ByteBuffer.allocate(1 + previousStopBytes.length) - .put(CACHE_TYPE_ID) - .put(previousStopBytes) - .array(); - } - - @Override - public TopNMetricSpecBuilder configureOptimizer(TopNMetricSpecBuilder builder) - { - return builder; - } - - @Override - public String toString() - { - return "NumericDimensionTopNMetricSpec{" + - "previousStop='" + getPreviousStop() + '\'' + - '}'; - } -} diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java index 8c23a4457cbc..e6cab9493c63 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java @@ -3258,7 +3258,7 @@ public void testNumericDimensionTopNWithNullPreviousStop() .dataSource(QueryRunnerTestHelper.dataSource) .granularity(QueryGranularities.ALL) .dimension(QueryRunnerTestHelper.marketDimension) - .metric(new NumericDimensionTopNMetricSpec(null)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) .threshold(2) .intervals(QueryRunnerTestHelper.secondOnly) .aggregators(Lists.newArrayList(QueryRunnerTestHelper.rowsCount)) From 2202de20fb5a17259372783ebd5f181f12a81234 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 27 Jul 2016 12:39:19 -0700 Subject: [PATCH 13/18] Delete old SearchSortSpec --- .../search/AlphanumericSearchSortSpec.java | 78 ------------------ .../search/LexicographicSearchSortSpec.java | 79 ------------------- .../query/search/search/SearchSortSpec.java | 40 ---------- .../search/search/StrlenSearchSortSpec.java | 72 ----------------- .../AlphanumericSearchSortSpecTest.java | 45 ----------- .../LexicographicSearchSortSpecTest.java | 45 ----------- .../search/StrlenSearchSortSpecTest.java | 45 ----------- 7 files changed, 404 deletions(-) delete mode 100644 processing/src/main/java/io/druid/query/search/search/AlphanumericSearchSortSpec.java delete mode 100644 processing/src/main/java/io/druid/query/search/search/LexicographicSearchSortSpec.java delete mode 100644 processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java delete mode 100644 processing/src/main/java/io/druid/query/search/search/StrlenSearchSortSpec.java delete mode 100644 processing/src/test/java/io/druid/query/search/AlphanumericSearchSortSpecTest.java delete mode 100644 processing/src/test/java/io/druid/query/search/LexicographicSearchSortSpecTest.java delete mode 100644 processing/src/test/java/io/druid/query/search/StrlenSearchSortSpecTest.java diff --git a/processing/src/main/java/io/druid/query/search/search/AlphanumericSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/AlphanumericSearchSortSpec.java deleted file mode 100644 index 3ad3a5523127..000000000000 --- a/processing/src/main/java/io/druid/query/search/search/AlphanumericSearchSortSpec.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; - -import com.fasterxml.jackson.annotation.JsonCreator; - -import io.druid.query.ordering.StringComparators; - -import java.util.Comparator; - -/** - */ -public class AlphanumericSearchSortSpec implements SearchSortSpec -{ - @JsonCreator - public AlphanumericSearchSortSpec( - ) - { - } - - @Override - public Comparator getComparator() - { - return new Comparator() - { - @Override - public int compare(SearchHit searchHit1, SearchHit searchHit2) - { - int retVal = StringComparators.ALPHANUMERIC.compare( - searchHit1.getValue(), searchHit2.getValue()); - if (retVal == 0) { - retVal = StringComparators.LEXICOGRAPHIC.compare( - searchHit1.getDimension(), searchHit2.getDimension()); - } - return retVal; - } - }; - } - - public String toString() - { - return "alphanumericSort"; - } - - @Override - public boolean equals(Object other) { - return this == other || other instanceof AlphanumericSearchSortSpec; - } - - @Override - public int hashCode() - { - return 0; - } - - @Override - public byte[] getCacheKey() - { - return toString().getBytes(); - } -} diff --git a/processing/src/main/java/io/druid/query/search/search/LexicographicSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/LexicographicSearchSortSpec.java deleted file mode 100644 index 55f00992c32d..000000000000 --- a/processing/src/main/java/io/druid/query/search/search/LexicographicSearchSortSpec.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; - -import com.fasterxml.jackson.annotation.JsonCreator; - -import io.druid.query.ordering.StringComparators; - -import java.util.Comparator; - -/** - */ -public class LexicographicSearchSortSpec implements SearchSortSpec -{ - @JsonCreator - public LexicographicSearchSortSpec( - ) - { - } - - @Override - public Comparator getComparator() - { - return new Comparator() - { - @Override - public int compare(SearchHit searchHit, SearchHit searchHit1) - { - int retVal = StringComparators.LEXICOGRAPHIC.compare( - searchHit.getValue(), searchHit1.getValue()); - - if (retVal == 0) { - retVal = StringComparators.LEXICOGRAPHIC.compare( - searchHit.getDimension(), searchHit1.getDimension()); - } - return retVal; - } - }; - } - - @Override - public byte[] getCacheKey() - { - return toString().getBytes(); - } - - public String toString() - { - return "lexicographicSort"; - } - - @Override - public boolean equals(Object other) { - return this == other || other instanceof LexicographicSearchSortSpec; - } - - @Override - public int hashCode() - { - return 0; - } -} diff --git a/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java deleted file mode 100644 index 4a2caf44dcb9..000000000000 --- a/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; - -import com.fasterxml.jackson.annotation.JsonSubTypes; -import com.fasterxml.jackson.annotation.JsonTypeInfo; - -import java.util.Comparator; - -/** - */ -@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl = LexicographicSearchSortSpec.class) -@JsonSubTypes(value = { - @JsonSubTypes.Type(name = "lexicographic", value = LexicographicSearchSortSpec.class), - @JsonSubTypes.Type(name = "alphanumeric", value = AlphanumericSearchSortSpec.class), - @JsonSubTypes.Type(name = "strlen", value = StrlenSearchSortSpec.class) -}) -public interface SearchSortSpec -{ - Comparator getComparator(); - - byte[] getCacheKey(); -} diff --git a/processing/src/main/java/io/druid/query/search/search/StrlenSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/StrlenSearchSortSpec.java deleted file mode 100644 index ef9fa134ce13..000000000000 --- a/processing/src/main/java/io/druid/query/search/search/StrlenSearchSortSpec.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search.search; - -import io.druid.query.ordering.StringComparators; - -import java.util.Comparator; - -/** - */ -public class StrlenSearchSortSpec implements SearchSortSpec -{ - public StrlenSearchSortSpec() - { - } - - @Override - public Comparator getComparator() - { - return new Comparator() { - @Override - public int compare(SearchHit s, SearchHit s1) - { - int res = StringComparators.STRLEN.compare(s.getValue(), s1.getValue()); - if (res == 0) { - res = StringComparators.LEXICOGRAPHIC.compare( - s.getDimension(), s1.getDimension()); - } - return res; - } - }; - } - - @Override - public byte[] getCacheKey() - { - return toString().getBytes(); - } - - public String toString() - { - return "stringLengthSort"; - } - - @Override - public boolean equals(Object other) { - return this == other || other instanceof StrlenSearchSortSpec; - } - - @Override - public int hashCode() - { - return 0; - } -} diff --git a/processing/src/test/java/io/druid/query/search/AlphanumericSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/AlphanumericSearchSortSpecTest.java deleted file mode 100644 index 3dd42598c3be..000000000000 --- a/processing/src/test/java/io/druid/query/search/AlphanumericSearchSortSpecTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search; - -import io.druid.query.search.search.AlphanumericSearchSortSpec; -import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.SearchSortSpec; -import org.junit.Assert; -import org.junit.Test; - -/** - */ -public class AlphanumericSearchSortSpecTest -{ - @Test - public void testComparator() - { - SearchSortSpec spec = new AlphanumericSearchSortSpec(); - - SearchHit hit1 = new SearchHit("test", "a100"); - SearchHit hit2 = new SearchHit("test", "a9"); - SearchHit hit3 = new SearchHit("test", "b0"); - - Assert.assertTrue(spec.getComparator().compare(hit1, hit2) > 0); - Assert.assertTrue(spec.getComparator().compare(hit3, hit1) > 0); - Assert.assertTrue(spec.getComparator().compare(hit3, hit2) > 0); - } -} diff --git a/processing/src/test/java/io/druid/query/search/LexicographicSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/LexicographicSearchSortSpecTest.java deleted file mode 100644 index 6f2848659720..000000000000 --- a/processing/src/test/java/io/druid/query/search/LexicographicSearchSortSpecTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search; - -import io.druid.query.search.search.LexicographicSearchSortSpec; -import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.SearchSortSpec; -import org.junit.Assert; -import org.junit.Test; - -/** - */ -public class LexicographicSearchSortSpecTest -{ - @Test - public void testComparator() - { - SearchHit hit1 = new SearchHit("test", "apple"); - SearchHit hit2 = new SearchHit("test", "banana"); - SearchHit hit3 = new SearchHit("test", "banana"); - - SearchSortSpec spec = new LexicographicSearchSortSpec(); - - Assert.assertTrue(spec.getComparator().compare(hit2, hit3) == 0); - Assert.assertTrue(spec.getComparator().compare(hit2, hit1) > 0); - Assert.assertTrue(spec.getComparator().compare(hit1, hit3) < 0); - } -} diff --git a/processing/src/test/java/io/druid/query/search/StrlenSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/StrlenSearchSortSpecTest.java deleted file mode 100644 index b567caf38de3..000000000000 --- a/processing/src/test/java/io/druid/query/search/StrlenSearchSortSpecTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets 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 io.druid.query.search; - -import io.druid.query.search.search.SearchHit; -import io.druid.query.search.search.SearchSortSpec; -import io.druid.query.search.search.StrlenSearchSortSpec; -import org.junit.Assert; -import org.junit.Test; - -/** - */ -public class StrlenSearchSortSpecTest -{ - @Test - public void testComparator() - { - SearchSortSpec spec = new StrlenSearchSortSpec(); - - SearchHit hit1 = new SearchHit("test", "a"); - SearchHit hit2 = new SearchHit("test", "apple"); - SearchHit hit3 = new SearchHit("test", "elppa"); - - Assert.assertTrue(spec.getComparator().compare(hit2, hit3) < 0); - Assert.assertTrue(spec.getComparator().compare(hit2, hit1) > 0); - Assert.assertTrue(spec.getComparator().compare(hit1, hit3) < 0); - } -} From 9bdc438ac54cf6f01cecca3b0953187a1a8f3217 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 27 Jul 2016 12:40:11 -0700 Subject: [PATCH 14/18] Rename NewSearchSortSpec to SearchSortSpec --- .../src/main/java/io/druid/query/Druids.java | 6 ++--- .../io/druid/query/search/SearchBinaryFn.java | 6 ++--- .../query/search/search/SearchQuery.java | 8 +++---- ...earchSortSpec.java => SearchSortSpec.java} | 6 ++--- .../query/search/SearchBinaryFnTest.java | 22 +++++++++---------- .../query/search/SearchQueryRunnerTest.java | 6 ++--- ...tSpecTest.java => SearchSortSpecTest.java} | 18 +++++++-------- 7 files changed, 36 insertions(+), 36 deletions(-) rename processing/src/main/java/io/druid/query/search/search/{NewSearchSortSpec.java => SearchSortSpec.java} (95%) rename processing/src/test/java/io/druid/query/search/{NewSearchSortSpecTest.java => SearchSortSpecTest.java} (84%) diff --git a/processing/src/main/java/io/druid/query/Druids.java b/processing/src/main/java/io/druid/query/Druids.java index 4c722b0c613b..4e1deb914cc5 100644 --- a/processing/src/main/java/io/druid/query/Druids.java +++ b/processing/src/main/java/io/druid/query/Druids.java @@ -44,7 +44,7 @@ import io.druid.query.search.search.ContainsSearchQuerySpec; import io.druid.query.search.search.FragmentSearchQuerySpec; import io.druid.query.search.search.InsensitiveContainsSearchQuerySpec; -import io.druid.query.search.search.NewSearchSortSpec; +import io.druid.query.search.search.SearchSortSpec; import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQuerySpec; import io.druid.query.select.PagingSpec; @@ -546,7 +546,7 @@ public static class SearchQueryBuilder private QuerySegmentSpec querySegmentSpec; private List dimensions; private SearchQuerySpec querySpec; - private NewSearchSortSpec sortSpec; + private SearchSortSpec sortSpec; private Map context; public SearchQueryBuilder() @@ -731,7 +731,7 @@ public SearchQueryBuilder fragments(List q) return fragments(q, false); } - public SearchQueryBuilder sortSpec(NewSearchSortSpec sortSpec) + public SearchQueryBuilder sortSpec(SearchSortSpec sortSpec) { this.sortSpec = sortSpec; return this; diff --git a/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java b/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java index 5cdbf038d82b..9c0affe1546c 100644 --- a/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java +++ b/processing/src/main/java/io/druid/query/search/SearchBinaryFn.java @@ -25,7 +25,7 @@ import io.druid.granularity.AllGranularity; import io.druid.granularity.QueryGranularity; import io.druid.query.Result; -import io.druid.query.search.search.NewSearchSortSpec; +import io.druid.query.search.search.SearchSortSpec; import io.druid.query.search.search.SearchHit; import org.joda.time.DateTime; @@ -37,12 +37,12 @@ public class SearchBinaryFn implements BinaryFn, Result, Result> { - private final NewSearchSortSpec searchSortSpec; + private final SearchSortSpec searchSortSpec; private final QueryGranularity gran; private final int limit; public SearchBinaryFn( - NewSearchSortSpec searchSortSpec, + SearchSortSpec searchSortSpec, QueryGranularity granularity, int limit ) diff --git a/processing/src/main/java/io/druid/query/search/search/SearchQuery.java b/processing/src/main/java/io/druid/query/search/search/SearchQuery.java index b58eff773f5a..8f5cd64d7b0f 100644 --- a/processing/src/main/java/io/druid/query/search/search/SearchQuery.java +++ b/processing/src/main/java/io/druid/query/search/search/SearchQuery.java @@ -41,10 +41,10 @@ */ public class SearchQuery extends BaseQuery> { - private static final NewSearchSortSpec DEFAULT_SORT_SPEC = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + private static final SearchSortSpec DEFAULT_SORT_SPEC = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); private final DimFilter dimFilter; - private final NewSearchSortSpec sortSpec; + private final SearchSortSpec sortSpec; private final QueryGranularity granularity; private final List dimensions; private final SearchQuerySpec querySpec; @@ -59,7 +59,7 @@ public SearchQuery( @JsonProperty("intervals") QuerySegmentSpec querySegmentSpec, @JsonProperty("searchDimensions") List dimensions, @JsonProperty("query") SearchQuerySpec querySpec, - @JsonProperty("sort") NewSearchSortSpec sortSpec, + @JsonProperty("sort") SearchSortSpec sortSpec, @JsonProperty("context") Map context ) { @@ -186,7 +186,7 @@ public SearchQuerySpec getQuery() } @JsonProperty("sort") - public NewSearchSortSpec getSort() + public SearchSortSpec getSort() { return sortSpec; } diff --git a/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java similarity index 95% rename from processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java rename to processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java index edc0998b28aa..94bad597ff46 100644 --- a/processing/src/main/java/io/druid/query/search/search/NewSearchSortSpec.java +++ b/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java @@ -27,14 +27,14 @@ import java.util.Comparator; -public class NewSearchSortSpec +public class SearchSortSpec { public static final StringComparator DEFAULT_ORDERING = StringComparators.LEXICOGRAPHIC; private final StringComparator ordering; @JsonCreator - public NewSearchSortSpec( + public SearchSortSpec( @JsonProperty("ordering") StringComparator ordering ) { @@ -87,7 +87,7 @@ public boolean equals(Object o) return false; } - NewSearchSortSpec that = (NewSearchSortSpec) o; + SearchSortSpec that = (SearchSortSpec) o; return ordering.equals(that.ordering); diff --git a/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java b/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java index 289eb8cd76d2..29a70b5f0c40 100644 --- a/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java @@ -23,7 +23,7 @@ import io.druid.granularity.QueryGranularities; import io.druid.query.Result; import io.druid.query.ordering.StringComparators; -import io.druid.query.search.search.NewSearchSortSpec; +import io.druid.query.search.search.SearchSortSpec; import io.druid.query.search.search.SearchHit; import org.joda.time.DateTime; import org.junit.Assert; @@ -97,7 +97,7 @@ public void testMerge() ) ); - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -145,7 +145,7 @@ public void testMergeDay() ) ); - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.DAY, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.DAY, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -169,7 +169,7 @@ public void testMergeOneResultNull() Result expected = r1; - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -217,7 +217,7 @@ public void testMergeShiftedTimestamp() ) ); - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -225,7 +225,7 @@ public void testMergeShiftedTimestamp() @Test public void testStrlenMerge() { - NewSearchSortSpec searchSortSpec = new NewSearchSortSpec(StringComparators.STRLEN); + SearchSortSpec searchSortSpec = new SearchSortSpec(StringComparators.STRLEN); Comparator c = searchSortSpec.getComparator(); Result r1 = new Result( @@ -251,7 +251,7 @@ public void testStrlenMerge() @Test public void testStrlenMerge2() { - NewSearchSortSpec searchSortSpec = new NewSearchSortSpec(StringComparators.STRLEN); + SearchSortSpec searchSortSpec = new SearchSortSpec(StringComparators.STRLEN); Comparator c = searchSortSpec.getComparator(); Result r1 = new Result( @@ -277,7 +277,7 @@ public void testStrlenMerge2() @Test public void testAlphanumericMerge() { - NewSearchSortSpec searchSortSpec = new NewSearchSortSpec(StringComparators.ALPHANUMERIC); + SearchSortSpec searchSortSpec = new SearchSortSpec(StringComparators.ALPHANUMERIC); Comparator c = searchSortSpec.getComparator(); Result r1 = new Result( @@ -331,7 +331,7 @@ public void testMergeUniqueResults() Result expected = r1; - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -362,7 +362,7 @@ public void testMergeLimit(){ ) ); Result expected = r1; - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, 1).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, 1).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } @@ -396,7 +396,7 @@ public void testMergeCountWithNull() { Result expected = r1; - Result actual = new SearchBinaryFn(new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); + Result actual = new SearchBinaryFn(new SearchSortSpec(StringComparators.LEXICOGRAPHIC), QueryGranularities.ALL, Integer.MAX_VALUE).apply(r1, r2); Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp()); assertSearchMergeResult(expected.getValue(), actual.getValue()); } diff --git a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java index 6f727e29baba..8b7620314fba 100644 --- a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java @@ -38,7 +38,7 @@ import io.druid.query.filter.SelectorDimFilter; import io.druid.query.ordering.StringComparators; import io.druid.query.search.search.FragmentSearchQuerySpec; -import io.druid.query.search.search.NewSearchSortSpec; +import io.druid.query.search.search.SearchSortSpec; import io.druid.query.search.search.SearchHit; import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQueryConfig; @@ -212,7 +212,7 @@ public void testSearchSameValueInMultiDims2() QueryRunnerTestHelper.placementishDimension ) ) - .sortSpec(new NewSearchSortSpec(StringComparators.STRLEN)) + .sortSpec(new SearchSortSpec(StringComparators.STRLEN)) .query("e") .build(); @@ -586,7 +586,7 @@ public void testSearchWithNumericSort() .granularity(QueryRunnerTestHelper.allGran) .intervals(QueryRunnerTestHelper.fullOnInterval) .query("a") - .sortSpec(new NewSearchSortSpec(StringComparators.NUMERIC)) + .sortSpec(new SearchSortSpec(StringComparators.NUMERIC)) .build(); List expectedHits = Lists.newLinkedList(); diff --git a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java similarity index 84% rename from processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java rename to processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java index f4325ff8ffdb..a604c85b9287 100644 --- a/processing/src/test/java/io/druid/query/search/NewSearchSortSpecTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java @@ -22,7 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.druid.jackson.DefaultObjectMapper; import io.druid.query.ordering.StringComparators; -import io.druid.query.search.search.NewSearchSortSpec; +import io.druid.query.search.search.SearchSortSpec; import io.druid.query.search.search.SearchHit; import org.junit.Assert; import org.junit.Test; @@ -31,7 +31,7 @@ /** */ -public class NewSearchSortSpecTest +public class SearchSortSpecTest { @Test public void testLexicographicComparator() @@ -40,7 +40,7 @@ public void testLexicographicComparator() SearchHit hit2 = new SearchHit("test", "banana"); SearchHit hit3 = new SearchHit("test", "banana"); - NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + SearchSortSpec spec = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); Assert.assertTrue(spec.getComparator().compare(hit2, hit3) == 0); Assert.assertTrue(spec.getComparator().compare(hit2, hit1) > 0); @@ -50,7 +50,7 @@ public void testLexicographicComparator() @Test public void testAlphanumericComparator() { - NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.ALPHANUMERIC); + SearchSortSpec spec = new SearchSortSpec(StringComparators.ALPHANUMERIC); SearchHit hit1 = new SearchHit("test", "a100"); SearchHit hit2 = new SearchHit("test", "a9"); @@ -64,7 +64,7 @@ public void testAlphanumericComparator() @Test public void testNumericComparator() { - NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.NUMERIC); + SearchSortSpec spec = new SearchSortSpec(StringComparators.NUMERIC); SearchHit hit1 = new SearchHit("test", "1001001.12412"); SearchHit hit2 = new SearchHit("test", "-1421"); @@ -83,7 +83,7 @@ public void testNumericComparator() @Test public void testStrlenComparator() { - NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + SearchSortSpec spec = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); SearchHit hit1 = new SearchHit("test", "apple"); SearchHit hit2 = new SearchHit("test", "banana"); @@ -101,16 +101,16 @@ public void testStrlenComparator() public void testSerde() throws IOException { ObjectMapper jsonMapper = new DefaultObjectMapper(); - NewSearchSortSpec spec = new NewSearchSortSpec(StringComparators.LEXICOGRAPHIC); + SearchSortSpec spec = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); String expectJsonSpec = "{\"ordering\":{\"type\":\"lexicographic\"}}"; String jsonSpec = jsonMapper.writeValueAsString(spec); Assert.assertEquals(expectJsonSpec, jsonSpec); - Assert.assertEquals(spec, jsonMapper.readValue(jsonSpec, NewSearchSortSpec.class)); + Assert.assertEquals(spec, jsonMapper.readValue(jsonSpec, SearchSortSpec.class)); // this works too, without specifying "ordering"... String expectJsonSpec2 = "{\"type\":\"lexicographic\"}"; - Assert.assertEquals(spec, jsonMapper.readValue(expectJsonSpec2, NewSearchSortSpec.class)); + Assert.assertEquals(spec, jsonMapper.readValue(expectJsonSpec2, SearchSortSpec.class)); } } From 8bf5fff4ad958cb3e713be0ebf89e586df919a63 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 27 Jul 2016 13:56:58 -0700 Subject: [PATCH 15/18] Add TopN numeric comparator benchmark, address PR comments --- .../druid/benchmark/query/TopNBenchmark.java | 36 ++++++++++++++++- docs/content/querying/topnmetricspec.md | 2 + .../io/druid/query/filter/BoundDimFilter.java | 2 +- .../query/ordering/StringComparator.java | 2 + .../query/ordering/StringComparators.java | 40 ++++++++++++++++++- .../query/search/search/SearchSortSpec.java | 9 ++--- .../query/topn/DimensionTopNMetricSpec.java | 6 +-- .../query/search/SearchSortSpecTest.java | 8 ++-- 8 files changed, 88 insertions(+), 17 deletions(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/TopNBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/TopNBenchmark.java index 50a7eecb164b..3b7bd849b709 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/TopNBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/TopNBenchmark.java @@ -50,8 +50,10 @@ import io.druid.query.aggregation.LongSumAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesSerde; +import io.druid.query.ordering.StringComparators; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.query.spec.QuerySegmentSpec; +import io.druid.query.topn.DimensionTopNMetricSpec; import io.druid.query.topn.TopNQuery; import io.druid.query.topn.TopNQueryBuilder; import io.druid.query.topn.TopNQueryConfig; @@ -104,7 +106,7 @@ public class TopNBenchmark @Param({"750000"}) private int rowsPerSegment; - @Param({"basic.A"}) + @Param({"basic.A", "basic.numericSort", "basic.alphanumericSort"}) private String schemaAndQuery; @Param({"10"}) @@ -170,6 +172,38 @@ private void setupQueries() basicQueries.put("A", queryBuilderA); } + { // basic.numericSort + QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(Arrays.asList(basicSchema.getDataInterval())); + + List queryAggs = new ArrayList<>(); + queryAggs.add(new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential")); + + TopNQueryBuilder queryBuilderA = new TopNQueryBuilder() + .dataSource("blah") + .granularity(QueryGranularities.ALL) + .dimension("dimUniform") + .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) + .intervals(intervalSpec) + .aggregators(queryAggs); + + basicQueries.put("numericSort", queryBuilderA); + } + { // basic.alphanumericSort + QuerySegmentSpec intervalSpec = new MultipleIntervalSegmentSpec(Arrays.asList(basicSchema.getDataInterval())); + + List queryAggs = new ArrayList<>(); + queryAggs.add(new LongSumAggregatorFactory("sumLongSequential", "sumLongSequential")); + + TopNQueryBuilder queryBuilderA = new TopNQueryBuilder() + .dataSource("blah") + .granularity(QueryGranularities.ALL) + .dimension("dimUniform") + .metric(new DimensionTopNMetricSpec(null, StringComparators.ALPHANUMERIC)) + .intervals(intervalSpec) + .aggregators(queryAggs); + + basicQueries.put("alphanumericSort", queryBuilderA); + } SCHEMA_QUERY_MAP.put("basic", basicQueries); } diff --git a/docs/content/querying/topnmetricspec.md b/docs/content/querying/topnmetricspec.md index 391e779f86b5..1232cf723ec4 100644 --- a/docs/content/querying/topnmetricspec.md +++ b/docs/content/querying/topnmetricspec.md @@ -48,6 +48,8 @@ The following metricSpec uses lexicographic sorting. } ``` +Note that in earlier versions of Druid, the functionality provided by the DimensionTopNMetricSpec was handled by two separate spec types, Lexicographic and Alphanumeric (when only two sorting orders were supported). These spec types have been deprecated but are still usable. + ## Inverted TopNMetricSpec Sort dimension values in inverted order, i.e inverts the order of the delegate metric spec. It can be used to sort the values in ascending order. diff --git a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java index d94e7d4f73a5..5c9642df43be 100644 --- a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java @@ -166,7 +166,7 @@ public byte[] getCacheKey() byte[] extractionFnBytes = extractionFn == null ? new byte[0] : extractionFn.getCacheKey(); - byte[] orderingBytes = StringUtils.toUtf8(ordering.toString()); + byte[] orderingBytes = ordering.getCacheKey(); ByteBuffer boundCacheBuffer = ByteBuffer.allocate( 9 diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparator.java b/processing/src/main/java/io/druid/query/ordering/StringComparator.java index 095ecf028dac..f61cab877a23 100644 --- a/processing/src/main/java/io/druid/query/ordering/StringComparator.java +++ b/processing/src/main/java/io/druid/query/ordering/StringComparator.java @@ -42,4 +42,6 @@ public static StringComparator fromString(String type) throw new IAE("Unknown string comparator[%s]", type); } } + + public abstract byte[] getCacheKey(); } diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparators.java b/processing/src/main/java/io/druid/query/ordering/StringComparators.java index f79c6f1efff8..49972e139890 100644 --- a/processing/src/main/java/io/druid/query/ordering/StringComparators.java +++ b/processing/src/main/java/io/druid/query/ordering/StringComparators.java @@ -24,6 +24,7 @@ import com.google.common.collect.Ordering; import com.google.common.primitives.Ints; +import com.google.common.primitives.Longs; import com.google.common.primitives.UnsignedBytes; import com.metamx.common.StringUtils; @@ -39,6 +40,11 @@ public class StringComparators public static final StringComparator NUMERIC = new NumericComparator(); public static final StringComparator STRLEN = new StrlenComparator(); + public static final int LEXICOGRAPHIC_CACHE_ID = 0x01; + public static final int ALPHANUMERIC_CACHE_ID = 0x02; + public static final int NUMERIC_CACHE_ID = 0x03; + public static final int STRLEN_CACHE_ID = 0x04; + public static class LexicographicComparator extends StringComparator { private static final Ordering ORDERING = Ordering.from(new Comparator() @@ -80,6 +86,12 @@ public String toString() { return StringComparators.LEXICOGRAPHIC_NAME; } + + @Override + public byte[] getCacheKey() + { + return new byte[]{(byte) LEXICOGRAPHIC_CACHE_ID}; + } } public static class AlphanumericComparator extends StringComparator @@ -286,6 +298,12 @@ public String toString() { return StringComparators.ALPHANUMERIC_NAME; } + + @Override + public byte[] getCacheKey() + { + return new byte[]{(byte) ALPHANUMERIC_CACHE_ID}; + } } public static class StrlenComparator extends StringComparator @@ -327,6 +345,12 @@ public String toString() { return StringComparators.STRLEN_NAME; } + + @Override + public byte[] getCacheKey() + { + return new byte[]{(byte) STRLEN_CACHE_ID}; + } } private static BigDecimal convertStringToBigDecimal(String input) { @@ -348,8 +372,14 @@ public int compare(String o1, String o2) return 0; } - BigDecimal bd1 = convertStringToBigDecimal(o1); - BigDecimal bd2 = convertStringToBigDecimal(o2); + // Creating a BigDecimal from a String is expensive (involves copying the String into a char[]) + // Converting the String to a Long first is faster. + // We optimize here with the assumption that integer values are more common than floating point. + Long long1 = o1 == null ? null : Longs.tryParse(o1); + Long long2 = o2 == null ? null : Longs.tryParse(o2); + + final BigDecimal bd1 = long1 == null ? convertStringToBigDecimal(o1) : new BigDecimal(long1); + final BigDecimal bd2 = long2 == null ? convertStringToBigDecimal(o2) : new BigDecimal(long2); if (bd1 != null && bd2 != null) { return bd1.compareTo(bd2); @@ -385,5 +415,11 @@ public boolean equals(Object o) return true; } + + @Override + public byte[] getCacheKey() + { + return new byte[]{(byte) NUMERIC_CACHE_ID}; + } } } diff --git a/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java b/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java index 94bad597ff46..947ac89a534b 100644 --- a/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java +++ b/processing/src/main/java/io/druid/query/search/search/SearchSortSpec.java @@ -35,13 +35,13 @@ public class SearchSortSpec @JsonCreator public SearchSortSpec( - @JsonProperty("ordering") StringComparator ordering + @JsonProperty("type") StringComparator ordering ) { this.ordering = ordering == null ? DEFAULT_ORDERING : ordering; } - @JsonProperty("ordering") + @JsonProperty("type") public StringComparator getOrdering() { return ordering; @@ -68,8 +68,7 @@ public int compare(SearchHit searchHit, SearchHit searchHit1) public byte[] getCacheKey() { - byte[] orderingBytes = StringUtils.toUtf8(ordering.toString()); - return orderingBytes; + return ordering.getCacheKey(); } public String toString() @@ -98,4 +97,4 @@ public int hashCode() { return ordering.hashCode(); } -} \ No newline at end of file +} diff --git a/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java index 3a33b00ff02b..e866b9153144 100644 --- a/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java +++ b/processing/src/main/java/io/druid/query/topn/DimensionTopNMetricSpec.java @@ -39,7 +39,7 @@ public class DimensionTopNMetricSpec implements TopNMetricSpec { private static final StringComparator DEFAULT_ORDERING = StringComparators.LEXICOGRAPHIC; - public static final byte STRING_SEPARATOR = (byte) 0xFF; + private static final byte STRING_SEPARATOR = (byte) 0xFF; private static final byte CACHE_TYPE_ID = 0x4; private final String previousStop; @@ -102,7 +102,7 @@ public TopNResultBuilder getResultBuilder( public byte[] getCacheKey() { byte[] previousStopBytes = previousStop == null ? new byte[]{} : StringUtils.toUtf8(previousStop); - byte[] orderingBytes = StringUtils.toUtf8(ordering.toString()); + byte[] orderingBytes = ordering.getCacheKey(); int totalLen = 2 + previousStopBytes.length + orderingBytes.length; @@ -179,4 +179,4 @@ public int hashCode() result = 31 * result + getOrdering().hashCode(); return result; } -} \ No newline at end of file +} diff --git a/processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java b/processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java index a604c85b9287..ece399887660 100644 --- a/processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchSortSpecTest.java @@ -101,16 +101,14 @@ public void testStrlenComparator() public void testSerde() throws IOException { ObjectMapper jsonMapper = new DefaultObjectMapper(); - SearchSortSpec spec = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); + SearchSortSpec spec = new SearchSortSpec(StringComparators.ALPHANUMERIC); - String expectJsonSpec = "{\"ordering\":{\"type\":\"lexicographic\"}}"; + String expectJsonSpec = "{\"type\":{\"type\":\"alphanumeric\"}}"; String jsonSpec = jsonMapper.writeValueAsString(spec); Assert.assertEquals(expectJsonSpec, jsonSpec); Assert.assertEquals(spec, jsonMapper.readValue(jsonSpec, SearchSortSpec.class)); - // this works too, without specifying "ordering"... - String expectJsonSpec2 = "{\"type\":\"lexicographic\"}"; + String expectJsonSpec2 = "{\"type\":\"alphanumeric\"}"; Assert.assertEquals(spec, jsonMapper.readValue(expectJsonSpec2, SearchSortSpec.class)); - } } From 2a4666b405ce77dd8d2ebc4a5522b9081f73f2ca Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 27 Jul 2016 15:49:28 -0700 Subject: [PATCH 16/18] Refactor OrderByColumnSpec --- docs/content/querying/limitspec.md | 2 +- .../io/druid/query/groupby/GroupByQuery.java | 5 - .../groupby/orderby/OrderByColumnSpec.java | 142 +++++++----------- .../query/groupby/GroupByQueryRunnerTest.java | 8 +- 4 files changed, 62 insertions(+), 95 deletions(-) diff --git a/docs/content/querying/limitspec.md b/docs/content/querying/limitspec.md index 0d65f95475de..8bd46f8a87be 100644 --- a/docs/content/querying/limitspec.md +++ b/docs/content/querying/limitspec.md @@ -28,6 +28,6 @@ OrderByColumnSpecs indicate how to do order by operations. Each order-by conditi } ``` -If only the dimension is provided (as a JSON string), the default order-by is ascending. +If only the dimension is provided (as a JSON string), the default order-by is ascending with lexicographic sorting. See [Sorting Orders](./sorting-orders.html) for more information on the sorting orders specified by "dimensionOrder". diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java index c76274e74992..5933f5c4e87f 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -487,11 +487,6 @@ public Builder addOrderByColumn(String dimension) return addOrderByColumn(dimension, (OrderByColumnSpec.Direction) null); } - public Builder addOrderByColumn(String dimension, String direction) - { - return addOrderByColumn(dimension, OrderByColumnSpec.determineDirection(direction)); - } - public Builder addOrderByColumn(String dimension, OrderByColumnSpec.Direction direction) { return addOrderByColumn(new OrderByColumnSpec(dimension, direction)); diff --git a/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java b/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java index d200ef5d39bf..25a7819281e0 100644 --- a/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java +++ b/processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java @@ -21,11 +21,10 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonValue; import com.google.common.base.Function; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; -import com.metamx.common.IAE; import com.metamx.common.ISE; import com.metamx.common.StringUtils; @@ -42,50 +41,74 @@ */ public class OrderByColumnSpec { - public static enum Direction + public enum Direction { ASCENDING, - DESCENDING - } + DESCENDING; + + /** + * Maintain a map of the enum values so that we can just do a lookup and get a null if it doesn't exist instead + * of an exception thrown. + */ + private static final Map stupidEnumMap; + static { + final ImmutableMap.Builder bob = ImmutableMap.builder(); + for (Direction direction : Direction.values()) { + bob.put(direction.name(), direction); + } + stupidEnumMap = bob.build(); + } - public static final StringComparator DEFAULT_DIMENSION_ORDER = StringComparators.LEXICOGRAPHIC; + @JsonValue + @Override + public String toString() + { + return this.name().toLowerCase(); + } - /** - * Maintain a map of the enum values so that we can just do a lookup and get a null if it doesn't exist instead - * of an exception thrown. - */ - private static final Map stupidEnumMap; + @JsonCreator + public static Direction fromString(String name) + { + final String upperName = name.toUpperCase(); + Direction direction = stupidEnumMap.get(upperName); + + if (direction == null) { + for (Direction dir : Direction.values()) { + if (dir.name().startsWith(upperName)) { + if (direction != null) { + throw new ISE("Ambiguous directions[%s] and [%s]", direction, dir); + } + direction = dir; + } + } + } - static { - final ImmutableMap.Builder bob = ImmutableMap.builder(); - for (Direction direction : Direction.values()) { - bob.put(direction.toString(), direction); + return direction; } - stupidEnumMap = bob.build(); } + public static final StringComparator DEFAULT_DIMENSION_ORDER = StringComparators.LEXICOGRAPHIC; + private final String dimension; private final Direction direction; private final StringComparator dimensionComparator; @JsonCreator - public static OrderByColumnSpec create(Object obj) + public OrderByColumnSpec( + @JsonProperty("dimension") String dimension, + @JsonProperty("direction") Direction direction, + @JsonProperty("dimensionOrder") StringComparator dimensionComparator + ) { - Preconditions.checkNotNull(obj, "Cannot build an OrderByColumnSpec from a null object."); - - if (obj instanceof String) { - return new OrderByColumnSpec(obj.toString(), null, null); - } else if (obj instanceof Map) { - final Map map = (Map) obj; - - final String dimension = map.get("dimension").toString(); - final Direction direction = determineDirection(map.get("direction")); - final StringComparator dimensionComparator = determinDimensionComparator(map.get("dimensionOrder")); + this.dimension = dimension; + this.direction = direction == null ? Direction.ASCENDING : direction; + this.dimensionComparator = dimensionComparator == null ? DEFAULT_DIMENSION_ORDER : dimensionComparator; + } - return new OrderByColumnSpec(dimension, direction, dimensionComparator); - } else { - throw new ISE("Cannot build an OrderByColumnSpec from a %s", obj.getClass()); - } + @JsonCreator + public static OrderByColumnSpec fromString(String dimension) + { + return new OrderByColumnSpec(dimension, null, null); } public static OrderByColumnSpec asc(String dimension) @@ -136,75 +159,24 @@ public OrderByColumnSpec( this(dimension, direction, null); } - public OrderByColumnSpec( - String dimension, - Direction direction, - StringComparator dimensionComparator - ) - { - this.dimension = dimension; - this.direction = direction == null ? Direction.ASCENDING : direction; - this.dimensionComparator = dimensionComparator == null ? DEFAULT_DIMENSION_ORDER : dimensionComparator; - } - - @JsonProperty + @JsonProperty("dimension") public String getDimension() { return dimension; } - @JsonProperty + @JsonProperty("direction") public Direction getDirection() { return direction; } - @JsonProperty + @JsonProperty("dimensionOrder") public StringComparator getDimensionComparator() { return dimensionComparator; } - public static Direction determineDirection(Object directionObj) - { - if (directionObj == null) { - return null; - } - - String directionString = directionObj.toString(); - - Direction direction = stupidEnumMap.get(directionString); - - if (direction == null) { - final String lowerDimension = directionString.toLowerCase(); - - for (Direction dir : Direction.values()) { - if (dir.toString().toLowerCase().startsWith(lowerDimension)) { - if (direction != null) { - throw new ISE("Ambiguous directions[%s] and [%s]", direction, dir); - } - direction = dir; - } - } - } - - if (direction == null) { - throw new IAE("Unknown direction[%s]", directionString); - } - - return direction; - } - - private static StringComparator determinDimensionComparator(Object dimensionOrderObj) - { - if (dimensionOrderObj == null) { - return DEFAULT_DIMENSION_ORDER; - } - - String dimensionOrderString = dimensionOrderObj.toString().toLowerCase(); - return StringComparator.fromString(dimensionOrderString); - } - @Override public boolean equals(Object o) { diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java index 107fcdb9bf76..3c16b23a939c 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java @@ -2080,8 +2080,8 @@ public void testGroupByWithOrderLimit2() throws Exception new LongSumAggregatorFactory("idx", "index") ) ) - .addOrderByColumn("rows", "desc") - .addOrderByColumn("alias", "d") + .addOrderByColumn("rows", OrderByColumnSpec.Direction.DESCENDING) + .addOrderByColumn("alias", OrderByColumnSpec.Direction.DESCENDING) .setGranularity(new PeriodGranularity(new Period("P1M"), null, null)); final GroupByQuery query = builder.build(); @@ -2120,8 +2120,8 @@ public void testGroupByWithOrderLimit3() throws Exception new DoubleSumAggregatorFactory("idx", "index") ) ) - .addOrderByColumn("idx", "desc") - .addOrderByColumn("alias", "d") + .addOrderByColumn("idx", OrderByColumnSpec.Direction.DESCENDING) + .addOrderByColumn("alias", OrderByColumnSpec.Direction.DESCENDING) .setGranularity(new PeriodGranularity(new Period("P1M"), null, null)); final GroupByQuery query = builder.build(); From d25e28d116da6c048a7425c7911db96d38a30acb Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 28 Jul 2016 14:12:31 -0700 Subject: [PATCH 17/18] Add null checks to NumericComparator and String->BigDecimal conversion function --- .../query/ordering/StringComparators.java | 18 +++++++++++++++--- .../query/ordering/StringComparatorsTest.java | 2 ++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparators.java b/processing/src/main/java/io/druid/query/ordering/StringComparators.java index 49972e139890..86e8cd80aef5 100644 --- a/processing/src/main/java/io/druid/query/ordering/StringComparators.java +++ b/processing/src/main/java/io/druid/query/ordering/StringComparators.java @@ -354,11 +354,15 @@ public byte[] getCacheKey() } private static BigDecimal convertStringToBigDecimal(String input) { + if (input == null) { + return null; + } + // treat unparseable Strings as nulls BigDecimal bd = null; try { bd = new BigDecimal(input); - } catch (NullPointerException | NumberFormatException ex) { + } catch (NumberFormatException ex) { } return bd; } @@ -368,15 +372,23 @@ public static class NumericComparator extends StringComparator @Override public int compare(String o1, String o2) { + // return if o1 and o2 are the same object if (o1 == o2) { return 0; } + // we know o1 != o2 + if (o1 == null) { + return -1; + } + if (o2 == null) { + return 1; + } // Creating a BigDecimal from a String is expensive (involves copying the String into a char[]) // Converting the String to a Long first is faster. // We optimize here with the assumption that integer values are more common than floating point. - Long long1 = o1 == null ? null : Longs.tryParse(o1); - Long long2 = o2 == null ? null : Longs.tryParse(o2); + Long long1 = Longs.tryParse(o1); + Long long2 = Longs.tryParse(o2); final BigDecimal bd1 = long1 == null ? convertStringToBigDecimal(o1) : new BigDecimal(long1); final BigDecimal bd2 = long2 == null ? convertStringToBigDecimal(o2) : new BigDecimal(long2); diff --git a/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java b/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java index e0e82d538be9..1273921b29ba 100644 --- a/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java +++ b/processing/src/test/java/io/druid/query/ordering/StringComparatorsTest.java @@ -134,6 +134,8 @@ public void testNumericComparator() values ); + + Assert.assertTrue(StringComparators.NUMERIC.compare(null, null) == 0); Assert.assertTrue(StringComparators.NUMERIC.compare(null, "1001") < 0); Assert.assertTrue(StringComparators.NUMERIC.compare("1001", null) > 0); From 7cb3e7756f3ec6e2be7ca2c3f4b3b1d52b207e6c Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 29 Jul 2016 11:51:55 -0700 Subject: [PATCH 18/18] Add more OrderByColumnSpec serde tests --- .../groupby/orderby/DefaultLimitSpecTest.java | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/processing/src/test/java/io/druid/query/groupby/orderby/DefaultLimitSpecTest.java b/processing/src/test/java/io/druid/query/groupby/orderby/DefaultLimitSpecTest.java index 9bbc3cee1ddb..af5249caa1cb 100644 --- a/processing/src/test/java/io/druid/query/groupby/orderby/DefaultLimitSpecTest.java +++ b/processing/src/test/java/io/druid/query/groupby/orderby/DefaultLimitSpecTest.java @@ -35,6 +35,7 @@ import io.druid.query.aggregation.post.ConstantPostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; +import io.druid.query.ordering.StringComparators; import io.druid.segment.TestHelper; import org.joda.time.DateTime; import org.junit.Assert; @@ -83,19 +84,67 @@ public void testSerde() throws Exception //non-defaults json = "{\n" + " \"type\":\"default\",\n" - + " \"columns\":[{\"dimension\":\"d\",\"direction\":\"ASCENDING\"}],\n" + + " \"columns\":[{\"dimension\":\"d\",\"direction\":\"DESCENDING\", \"dimensionOrder\":\"numeric\"}],\n" + " \"limit\":10\n" + "}"; + spec = mapper.readValue( + mapper.writeValueAsString(mapper.readValue(json, DefaultLimitSpec.class)), + DefaultLimitSpec.class + ); + Assert.assertEquals( + new DefaultLimitSpec(ImmutableList.of(new OrderByColumnSpec("d", OrderByColumnSpec.Direction.DESCENDING, + StringComparators.NUMERIC)), 10), + spec + ); + + json = "{\n" + + " \"type\":\"default\",\n" + + " \"columns\":[{\"dimension\":\"d\",\"direction\":\"DES\", \"dimensionOrder\":\"numeric\"}],\n" + + " \"limit\":10\n" + + "}"; + + spec = mapper.readValue( + mapper.writeValueAsString(mapper.readValue(json, DefaultLimitSpec.class)), + DefaultLimitSpec.class + ); + + Assert.assertEquals( + new DefaultLimitSpec(ImmutableList.of(new OrderByColumnSpec("d", OrderByColumnSpec.Direction.DESCENDING, + StringComparators.NUMERIC)), 10), + spec + ); + json = "{\n" + + " \"type\":\"default\",\n" + + " \"columns\":[{\"dimension\":\"d\"}],\n" + + " \"limit\":10\n" + + "}"; spec = mapper.readValue( mapper.writeValueAsString(mapper.readValue(json, DefaultLimitSpec.class)), DefaultLimitSpec.class ); + Assert.assertEquals( + new DefaultLimitSpec(ImmutableList.of(new OrderByColumnSpec("d", OrderByColumnSpec.Direction.ASCENDING, + StringComparators.LEXICOGRAPHIC)), 10), + spec + ); + json = "{\n" + + " \"type\":\"default\",\n" + + " \"columns\":[\"d\"],\n" + + " \"limit\":10\n" + + "}"; + spec = mapper.readValue( + mapper.writeValueAsString(mapper.readValue(json, DefaultLimitSpec.class)), + DefaultLimitSpec.class + ); Assert.assertEquals( - new DefaultLimitSpec(ImmutableList.of(new OrderByColumnSpec("d", OrderByColumnSpec.Direction.ASCENDING)), 10), + new DefaultLimitSpec(ImmutableList.of(new OrderByColumnSpec("d", OrderByColumnSpec.Direction.ASCENDING, + StringComparators.LEXICOGRAPHIC)), 10), spec ); + + } @Test