From 678b6ba746e80ccd16fd089fa40c9a8c83a8fee7 Mon Sep 17 00:00:00 2001 From: "navis.ryu" Date: Mon, 15 Feb 2016 10:44:55 +0900 Subject: [PATCH 1/2] Consolidate argument type of extracation function --- .../extraction/AbstractExtractionFn.java | 47 ++++++++ .../query/extraction/CascadeExtractionFn.java | 62 ++++------ .../query/extraction/DimExtractionFn.java | 14 +-- .../druid/query/extraction/ExtractionFn.java | 29 +---- .../extraction/FunctionalExtraction.java | 6 - .../extraction/IdentityExtractionFn.java | 16 +-- .../extraction/JavaScriptExtractionFn.java | 14 +-- .../query/extraction/LowerExtractionFn.java | 22 +--- .../extraction/MatchingDimExtractionFn.java | 14 +-- .../extraction/RegexDimExtractionFn.java | 14 +-- .../SearchQuerySpecDimExtractionFn.java | 2 +- .../extraction/StringFormatExtractionFn.java | 14 +-- .../extraction/SubstringDimExtractionFn.java | 10 +- .../query/extraction/TimeDimExtractionFn.java | 14 +-- .../extraction/TimeFormatExtractionFn.java | 30 +---- .../query/extraction/UpperExtractionFn.java | 22 +--- .../groupby/GroupByQueryQueryToolChest.java | 5 +- .../lookup/RegisteredLookupExtractionFn.java | 13 +-- .../druid/query/search/SearchQueryRunner.java | 3 + .../query/topn/TopNQueryQueryToolChest.java | 14 ++- .../segment/QueryableIndexStorageAdapter.java | 5 +- .../segment/SingleScanTimeDimSelector.java | 2 + .../druid/segment/column/ValueAccessor.java | 106 ++++++++++++++++++ .../segment/incremental/IncrementalIndex.java | 4 + .../IncrementalIndexStorageAdapter.java | 5 + .../StringFormatExtractionFnTest.java | 2 + .../druid/query/topn/TopNQueryRunnerTest.java | 12 -- .../filter/ExtractionDimFilterTest.java | 12 -- 28 files changed, 240 insertions(+), 273 deletions(-) create mode 100644 processing/src/main/java/io/druid/query/extraction/AbstractExtractionFn.java create mode 100644 processing/src/main/java/io/druid/segment/column/ValueAccessor.java diff --git a/processing/src/main/java/io/druid/query/extraction/AbstractExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/AbstractExtractionFn.java new file mode 100644 index 000000000000..db9a54ee950c --- /dev/null +++ b/processing/src/main/java/io/druid/query/extraction/AbstractExtractionFn.java @@ -0,0 +1,47 @@ +/* + * 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.extraction; + +import io.druid.segment.column.ValueAccessor; + +/** + */ +public abstract class AbstractExtractionFn implements ExtractionFn +{ + protected transient ValueAccessor accessor = ValueAccessor.STRING; + + @Override + public void init(ValueAccessor accessor) + { + this.accessor = accessor; + } + + @Override + public boolean preservesOrdering() + { + return false; + } + + @Override + public ExtractionType getExtractionType() + { + return ExtractionType.MANY_TO_ONE; + } +} diff --git a/processing/src/main/java/io/druid/query/extraction/CascadeExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/CascadeExtractionFn.java index b3d09805ca11..e284f9d956c9 100644 --- a/processing/src/main/java/io/druid/query/extraction/CascadeExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/CascadeExtractionFn.java @@ -24,46 +24,29 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.primitives.Bytes; +import io.druid.segment.column.ValueAccessor; import java.util.Arrays; -public class CascadeExtractionFn implements ExtractionFn +public class CascadeExtractionFn extends AbstractExtractionFn { private final ExtractionFn extractionFns[]; private final ChainedExtractionFn chainedExtractionFn; private final ChainedExtractionFn DEFAULT_CHAINED_EXTRACTION_FN = new ChainedExtractionFn( - new ExtractionFn() + new AbstractExtractionFn() { + @Override public byte[] getCacheKey() { return new byte[0]; } + @Override public String apply(Object value) { return null; } - public String apply(String value) - { - return null; - } - - public String apply(long value) - { - return null; - } - - public boolean preservesOrdering() - { - return false; - } - - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } - @Override public String toString() { @@ -99,27 +82,21 @@ public ExtractionFn[] getExtractionFns() } @Override - public byte[] getCacheKey() + public void init(ValueAccessor accessor) { - byte[] cacheKey = new byte[]{ExtractionCacheHelper.CACHE_TYPE_ID_CASCADE}; - - return Bytes.concat(cacheKey, chainedExtractionFn.getCacheKey()); + chainedExtractionFn.init(accessor); } @Override - public String apply(Object value) + public byte[] getCacheKey() { - return chainedExtractionFn.apply(value); - } + byte[] cacheKey = new byte[]{ExtractionCacheHelper.CACHE_TYPE_ID_CASCADE}; - @Override - public String apply(String value) - { - return chainedExtractionFn.apply(value); + return Bytes.concat(cacheKey, chainedExtractionFn.getCacheKey()); } @Override - public String apply(long value) + public String apply(Object value) { return chainedExtractionFn.apply(value); } @@ -171,7 +148,7 @@ public String toString() "extractionFns=[" + chainedExtractionFn.toString() + "]}"; } - private class ChainedExtractionFn + private class ChainedExtractionFn implements ExtractionFn { private final ExtractionFn fn; private final ChainedExtractionFn child; @@ -189,17 +166,16 @@ public byte[] getCacheKey() return (child != null) ? Bytes.concat(fnCacheKey, child.getCacheKey()) : fnCacheKey; } - public String apply(Object value) - { - return fn.apply((child != null) ? child.apply(value) : value); - } - - public String apply(String value) + @Override + public void init(ValueAccessor accessor) { - return fn.apply((child != null) ? child.apply(value) : value); + fn.init(accessor); + if (child != null) { + child.init(accessor); + } } - public String apply(long value) + public String apply(Object value) { return fn.apply((child != null) ? child.apply(value) : value); } diff --git a/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java index afb22cd567b7..fa22594d3d20 100644 --- a/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java @@ -19,17 +19,11 @@ package io.druid.query.extraction; -public abstract class DimExtractionFn implements ExtractionFn +public abstract class DimExtractionFn extends AbstractExtractionFn { - @Override - public String apply(Object value) - { - return apply(value == null ? null : value.toString()); + public String apply(Object value) { + return apply(accessor.getString(value, false)); } - @Override - public String apply(long value) - { - return apply(Long.toString(value)); - } + protected abstract String apply(String value); } diff --git a/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java index 7a2a92f54ecc..b3287c184979 100644 --- a/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import io.druid.query.lookup.LookupExtractionFn; import io.druid.query.lookup.RegisteredLookupExtractionFn; +import io.druid.segment.column.ValueAccessor; /** */ @@ -60,6 +61,8 @@ public interface ExtractionFn */ public byte[] getCacheKey(); + public void init(ValueAccessor accessor); + /** * The "extraction" function. This should map an Object into some String value. *

@@ -74,33 +77,9 @@ public interface ExtractionFn */ public String apply(Object value); - /** - * The "extraction" function. This should map a String value into some other String value. - *

- * Like {@link #apply(Object)}, the empty string is considered invalid output for this method and it should - * instead return null. - * - * @param value the original value of the dimension - * - * @return a value that should be used instead of the original - */ - public String apply(String value); - - /** - * The "extraction" function. This should map a long value into some String value. - *

- * Like {@link #apply(Object)}, the empty string is considered invalid output for this method and it should - * instead return null. - * - * @param value the original value of the dimension - * - * @return a value that should be used instead of the original - */ - public String apply(long value); - /** * Offers information on whether the extraction will preserve the original ordering of the values. - *

+ *

* Some optimizations of queries is possible if ordering is preserved. Null values *do* count towards * ordering. * diff --git a/processing/src/main/java/io/druid/query/extraction/FunctionalExtraction.java b/processing/src/main/java/io/druid/query/extraction/FunctionalExtraction.java index a85b3827b980..07a2d14c6c69 100644 --- a/processing/src/main/java/io/druid/query/extraction/FunctionalExtraction.java +++ b/processing/src/main/java/io/druid/query/extraction/FunctionalExtraction.java @@ -110,12 +110,6 @@ public String apply(String value) return extractionFunction.apply(value); } - @Override - public boolean preservesOrdering() - { - return false; - } - @Override public ExtractionType getExtractionType() { diff --git a/processing/src/main/java/io/druid/query/extraction/IdentityExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/IdentityExtractionFn.java index 9d9153efabd1..bb76ee392fe9 100644 --- a/processing/src/main/java/io/druid/query/extraction/IdentityExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/IdentityExtractionFn.java @@ -21,7 +21,7 @@ import com.google.common.base.Strings; -public class IdentityExtractionFn implements ExtractionFn +public class IdentityExtractionFn extends AbstractExtractionFn { private static final IdentityExtractionFn instance = new IdentityExtractionFn(); @@ -42,18 +42,6 @@ public String apply(Object value) return value == null ? null : Strings.emptyToNull(value.toString()); } - @Override - public String apply(String value) - { - return Strings.emptyToNull(value); - } - - @Override - public String apply(long value) - { - return Long.toString(value); - } - @Override public boolean preservesOrdering() { @@ -78,7 +66,7 @@ public boolean equals(Object o) return o != null && o instanceof IdentityExtractionFn; } - public static final IdentityExtractionFn getInstance() + public static IdentityExtractionFn getInstance() { return instance; } diff --git a/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java index a6b3ac08110d..79ced6667776 100644 --- a/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java @@ -31,7 +31,7 @@ import java.nio.ByteBuffer; -public class JavaScriptExtractionFn implements ExtractionFn +public class JavaScriptExtractionFn extends AbstractExtractionFn { private static Function compile(String function) { @@ -106,18 +106,6 @@ public String apply(Object value) return Strings.emptyToNull(fn.apply(value)); } - @Override - public String apply(String value) - { - return this.apply((Object) Strings.emptyToNull(value)); - } - - @Override - public String apply(long value) - { - return this.apply((Long) value); - } - @Override public boolean preservesOrdering() { diff --git a/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java index 64ebfe0f9dbb..19efb6f1dd7e 100644 --- a/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java @@ -31,7 +31,7 @@ import java.util.Locale; @JsonTypeName("lower") -public class LowerExtractionFn extends DimExtractionFn +public class LowerExtractionFn extends AbstractExtractionFn { private final Locale locale; @@ -52,24 +52,10 @@ public LowerExtractionFn(@JsonProperty("locale") String localeString) @Nullable @Override - public String apply(String key) + public String apply(Object key) { - if (Strings.isNullOrEmpty(key)) { - return null; - } - return key.toLowerCase(locale); - } - - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; + final String value = accessor.getString(key, false); + return Strings.isNullOrEmpty(value) ? null : value.toLowerCase(locale); } @Override diff --git a/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java index 9e5d16ec8f70..e781f7038820 100644 --- a/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java @@ -58,7 +58,7 @@ public byte[] getCacheKey() } @Override - public String apply(String dimValue) + protected String apply(String dimValue) { if (Strings.isNullOrEmpty(dimValue)) { // We'd return null whether or not the pattern matched @@ -75,18 +75,6 @@ public String getExpr() return expr; } - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } - @Override public String toString() { diff --git a/processing/src/main/java/io/druid/query/extraction/RegexDimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/RegexDimExtractionFn.java index be2600ec6487..3d476a886c27 100644 --- a/processing/src/main/java/io/druid/query/extraction/RegexDimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/RegexDimExtractionFn.java @@ -84,7 +84,7 @@ public byte[] getCacheKey() } @Override - public String apply(String dimValue) + protected String apply(String dimValue) { final String retVal; final Matcher matcher = pattern.matcher(Strings.nullToEmpty(dimValue)); @@ -114,18 +114,6 @@ public String getReplaceMissingValueWith() return replaceMissingValueWith; } - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } - @Override public String toString() { diff --git a/processing/src/main/java/io/druid/query/extraction/SearchQuerySpecDimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/SearchQuerySpecDimExtractionFn.java index 77ba930680ea..6cdf615d9f6f 100644 --- a/processing/src/main/java/io/druid/query/extraction/SearchQuerySpecDimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/SearchQuerySpecDimExtractionFn.java @@ -60,7 +60,7 @@ public byte[] getCacheKey() } @Override - public String apply(String dimValue) + protected String apply(String dimValue) { return searchQuerySpec.accept(dimValue) ? Strings.emptyToNull(dimValue) : null; } diff --git a/processing/src/main/java/io/druid/query/extraction/StringFormatExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/StringFormatExtractionFn.java index 0343b306472e..b666a450c261 100644 --- a/processing/src/main/java/io/druid/query/extraction/StringFormatExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/StringFormatExtractionFn.java @@ -95,7 +95,7 @@ public byte[] getCacheKey() } @Override - public String apply(String value) + protected String apply(String value) { if (value == null) { if (nullHandling == NullHandling.RETURNNULL) { @@ -108,18 +108,6 @@ public String apply(String value) return Strings.emptyToNull(String.format(format, value)); } - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } - @Override public boolean equals(Object o) { diff --git a/processing/src/main/java/io/druid/query/extraction/SubstringDimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/SubstringDimExtractionFn.java index 4802c1cff8ab..85f9013d4d90 100644 --- a/processing/src/main/java/io/druid/query/extraction/SubstringDimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/SubstringDimExtractionFn.java @@ -61,7 +61,7 @@ public byte[] getCacheKey() } @Override - public String apply(String dimValue) + protected String apply(String dimValue) { if (Strings.isNullOrEmpty(dimValue)) { return null; @@ -95,13 +95,7 @@ public Integer getLength() @Override public boolean preservesOrdering() { - return index == 0 ? true : false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; + return index == 0; } @Override diff --git a/processing/src/main/java/io/druid/query/extraction/TimeDimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/TimeDimExtractionFn.java index acf145177812..cf122b662ea6 100644 --- a/processing/src/main/java/io/druid/query/extraction/TimeDimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/TimeDimExtractionFn.java @@ -66,7 +66,7 @@ public byte[] getCacheKey() } @Override - public String apply(String dimValue) + protected String apply(String dimValue) { Date date; try { @@ -90,18 +90,6 @@ public String getResultFormat() return resultFormat; } - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } - @Override public String toString() { diff --git a/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java index 2ee2011e7010..87b0d0545338 100644 --- a/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import com.metamx.common.StringUtils; +import io.druid.segment.column.ValueType; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.format.DateTimeFormat; @@ -30,7 +31,7 @@ import java.nio.ByteBuffer; import java.util.Locale; -public class TimeFormatExtractionFn implements ExtractionFn +public class TimeFormatExtractionFn extends AbstractExtractionFn { private final DateTimeZone tz; private final String pattern; @@ -85,36 +86,15 @@ public byte[] getCacheKey() .array(); } - @Override - public String apply(long value) - { - return formatter.print(value); - } - @Override public String apply(Object value) { + if (accessor.type() == ValueType.LONG) { + return formatter.print(accessor.getLong(value)); + } return formatter.print(new DateTime(value)); } - @Override - public String apply(String value) - { - return apply((Object) value); - } - - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } - @Override public boolean equals(Object o) { diff --git a/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java index c43891fcb7d2..0af8bfc36f77 100644 --- a/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java @@ -31,7 +31,7 @@ import java.util.Locale; @JsonTypeName("upper") -public class UpperExtractionFn extends DimExtractionFn +public class UpperExtractionFn extends AbstractExtractionFn { private final Locale locale; @@ -51,24 +51,10 @@ public UpperExtractionFn(@JsonProperty("locale") String localeString) */ @Nullable @Override - public String apply(String key) + public String apply(Object key) { - if (Strings.isNullOrEmpty(key)) { - return null; - } - return key.toUpperCase(locale); - } - - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; + final String value = accessor.getString(key, false); + return Strings.isNullOrEmpty(value) ? null : value.toUpperCase(locale); } @Override diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java index a3c77245a832..78cd2bc5e045 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java @@ -65,6 +65,7 @@ import io.druid.query.extraction.ExtractionFn; import io.druid.query.filter.DimFilter; import io.druid.query.spec.MultipleIntervalSegmentSpec; +import io.druid.segment.column.ValueAccessor; import io.druid.segment.incremental.IncrementalIndex; import io.druid.segment.incremental.IncrementalIndexStorageAdapter; import org.joda.time.DateTime; @@ -382,7 +383,9 @@ public String apply(DimensionSpec input) for (DimensionSpec dimensionSpec : query.getDimensions()) { final String dimension = dimensionSpec.getOutputName(); if (optimizedDims.contains(dimension)) { - extractionFnMap.put(dimension, dimensionSpec.getExtractionFn()); + final ExtractionFn extractionFn = dimensionSpec.getExtractionFn(); + extractionFn.init(ValueAccessor.STRING); // use accessor for the dimension type + extractionFnMap.put(dimension, extractionFn); } } diff --git a/processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java b/processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java index a31ed6bd7e1b..cf26b2e81d2a 100644 --- a/processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java +++ b/processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java @@ -24,6 +24,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import io.druid.query.extraction.ExtractionFn; +import io.druid.segment.column.ValueAccessor; import javax.annotation.Nullable; @@ -100,19 +101,13 @@ public byte[] getCacheKey() } @Override - public String apply(Object value) - { - return delegate.apply(value); - } - - @Override - public String apply(String value) + public void init(ValueAccessor accessor) { - return delegate.apply(value); + delegate.init(accessor); } @Override - public String apply(long value) + public String apply(Object value) { return delegate.apply(value); } diff --git a/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java b/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java index e6fb921ff024..da556de5ea67 100644 --- a/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java +++ b/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java @@ -53,6 +53,7 @@ import io.druid.segment.StorageAdapter; import io.druid.segment.column.BitmapIndex; import io.druid.segment.column.Column; +import io.druid.segment.column.ValueAccessor; import io.druid.segment.data.IndexedInts; import io.druid.segment.filter.Filters; import org.apache.commons.lang.mutable.MutableInt; @@ -120,6 +121,8 @@ public Sequence> run( if (extractionFn == null) { extractionFn = IdentityExtractionFn.getInstance(); } + extractionFn.init(ValueAccessor.STRING); // use accessor for the dimension type + if (bitmapIndex != null) { for (int i = 0; i < bitmapIndex.getCardinality(); ++i) { String dimVal = Strings.nullToEmpty(extractionFn.apply(bitmapIndex.getValue(i))); diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index 20ca60c66128..601924f15117 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -51,7 +51,9 @@ import io.druid.query.aggregation.PostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; +import io.druid.query.extraction.ExtractionFn; import io.druid.query.filter.DimFilter; +import io.druid.segment.column.ValueAccessor; import org.joda.time.DateTime; import javax.annotation.Nullable; @@ -467,6 +469,11 @@ public Sequence> run( if (!TopNQueryEngine.canApplyExtractionInPost(topNQuery)) { return resultSequence; } else { + + final DimensionSpec dimensionSpec = topNQuery.getDimensionSpec(); + final ExtractionFn extractionFn = dimensionSpec.getExtractionFn(); + extractionFn.init(ValueAccessor.STRING); // use accessor for the dimension type + return Sequences.map( resultSequence, new Function, Result>() { @@ -488,12 +495,9 @@ public DimensionAndMetricValueExtractor apply( ) { String dimOutputName = topNQuery.getDimensionSpec().getOutputName(); - Object dimValue = input.getDimensionValue(dimOutputName); + Object dimValue = input.getStringDimensionValue(dimOutputName); Map map = input.getBaseObject(); - map.put( - dimOutputName, - topNQuery.getDimensionSpec().getExtractionFn().apply(dimValue) - ); + map.put(dimOutputName, extractionFn.apply(dimValue)); return input; } } diff --git a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java index fbbaf27dc977..50cbdf168b3b 100644 --- a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java @@ -41,6 +41,7 @@ import io.druid.segment.column.ComplexColumn; import io.druid.segment.column.DictionaryEncodedColumn; import io.druid.segment.column.GenericColumn; +import io.druid.segment.column.ValueAccessor; import io.druid.segment.column.ValueType; import io.druid.segment.data.Indexed; import io.druid.segment.data.IndexedInts; @@ -393,7 +394,9 @@ private DimensionSelector makeDimensionSelectorUndecorated( descending ); } - + if (extractionFn != null) { + extractionFn.init(ValueAccessor.STRING); // use accessor for the dimension type + } DictionaryEncodedColumn cachedColumn = dictionaryColumnCache.get(dimension); if (cachedColumn == null) { cachedColumn = columnDesc.getDictionaryEncoding(); diff --git a/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java b/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java index 2b5f085e2cbe..ed93908f11d3 100644 --- a/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java +++ b/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java @@ -22,6 +22,7 @@ import com.google.common.collect.Iterators; import com.google.common.collect.Maps; import io.druid.query.extraction.ExtractionFn; +import io.druid.segment.column.ValueAccessor; import io.druid.segment.data.IndexedInts; import java.io.IOException; @@ -49,6 +50,7 @@ public SingleScanTimeDimSelector(LongColumnSelector selector, ExtractionFn extra if (extractionFn == null) { throw new UnsupportedOperationException("time dimension must provide an extraction function"); } + extractionFn.init(ValueAccessor.LONG); this.extractionFn = extractionFn; this.selector = selector; diff --git a/processing/src/main/java/io/druid/segment/column/ValueAccessor.java b/processing/src/main/java/io/druid/segment/column/ValueAccessor.java new file mode 100644 index 000000000000..3272ed6cfa25 --- /dev/null +++ b/processing/src/main/java/io/druid/segment/column/ValueAccessor.java @@ -0,0 +1,106 @@ +/* + * 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.segment.column; + +/** + */ +public interface ValueAccessor +{ + ValueType type(); + + long getLong(Object value); + + String getString(Object value, boolean nullToEmpty); + + Object getObject(Object value); + + abstract class AbstractValueAccessor implements ValueAccessor + { + + @Override + public long getLong(Object value) + { + throw new UnsupportedOperationException("getLong"); + } + + @Override + public String getString(Object value, boolean nullToEmpty) + { + throw new UnsupportedOperationException("getString"); + } + + @Override + public Object getObject(Object value) + { + return value; + } + } + + ValueAccessor STRING = new AbstractValueAccessor() + { + @Override + public ValueType type() + { + return ValueType.STRING; + } + + @Override + public long getLong(Object value) + { + return Long.valueOf((String) value); + } + + @Override + public String getString(Object value, boolean nullToEmpty) + { + return (String) value; + } + }; + + ValueAccessor LONG = new AbstractValueAccessor() + { + @Override + public ValueType type() + { + return ValueType.LONG; + } + + @Override + public long getLong(Object value) + { + return (Long) value; + } + + @Override + public String getString(Object value, boolean nullToEmpty) + { + return value != null ? String.valueOf(value) : nullToEmpty ? "" : null; + } + }; + + ValueAccessor COMPLEX = new AbstractValueAccessor() + { + @Override + public ValueType type() + { + return ValueType.COMPLEX; + } + }; +} diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java index 673a79d66868..b8078fd1eb38 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java @@ -54,6 +54,7 @@ import io.druid.segment.column.Column; import io.druid.segment.column.ColumnCapabilities; import io.druid.segment.column.ColumnCapabilitiesImpl; +import io.druid.segment.column.ValueAccessor; import io.druid.segment.column.ValueType; import io.druid.segment.data.IndexedInts; import io.druid.segment.serde.ComplexMetricExtractor; @@ -267,6 +268,9 @@ private DimensionSelector makeDimensionSelectorUndecorated( { final String dimension = dimensionSpec.getDimension(); final ExtractionFn extractionFn = dimensionSpec.getExtractionFn(); + if (extractionFn != null) { + extractionFn.init(ValueAccessor.STRING); // use accessor for the dimension type + } return new DimensionSelector() { diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java index 64acece175ef..dbc27756ec8b 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java @@ -49,6 +49,7 @@ import io.druid.segment.StorageAdapter; import io.druid.segment.column.Column; import io.druid.segment.column.ColumnCapabilities; +import io.druid.segment.column.ValueAccessor; import io.druid.segment.data.Indexed; import io.druid.segment.data.IndexedInts; import io.druid.segment.data.ListIndexed; @@ -350,6 +351,10 @@ private DimensionSelector makeDimensionSelectorUndecorated( return NULL_DIMENSION_SELECTOR; } + if (extractionFn != null) { + extractionFn.init(ValueAccessor.STRING); // use accessor for the dimension type + } + final int dimIndex = dimensionDesc.getIndex(); final IncrementalIndex.DimDim dimValLookup = dimensionDesc.getValues(); diff --git a/processing/src/test/java/io/druid/query/extraction/StringFormatExtractionFnTest.java b/processing/src/test/java/io/druid/query/extraction/StringFormatExtractionFnTest.java index 5e824d9585ef..a01b75e3ce41 100644 --- a/processing/src/test/java/io/druid/query/extraction/StringFormatExtractionFnTest.java +++ b/processing/src/test/java/io/druid/query/extraction/StringFormatExtractionFnTest.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import io.druid.jackson.DefaultObjectMapper; +import io.druid.segment.column.ValueAccessor; import org.junit.Assert; import org.junit.Test; @@ -35,6 +36,7 @@ public class StringFormatExtractionFnTest public void testApply() throws Exception { StringFormatExtractionFn fn = new StringFormatExtractionFn("[%s]"); + fn.init(ValueAccessor.LONG); long test = 1000L; Assert.assertEquals("[1000]", fn.apply(test)); } 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 6b7526f500d0..3007613b5cc7 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java @@ -2462,18 +2462,6 @@ public String apply(String dimValue) { return dimValue.equals("total_market") ? null : dimValue; } - - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } }; final TopNQuery query = new TopNQueryBuilder() diff --git a/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java index 0e3e61ef541d..0d58838d9ae4 100644 --- a/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java @@ -145,18 +145,6 @@ public String apply(String dimValue) final String retval = EXTRACTION_VALUES.get(dimValue); return retval == null ? dimValue : retval; } - - @Override - public boolean preservesOrdering() - { - return false; - } - - @Override - public ExtractionType getExtractionType() - { - return ExtractionType.MANY_TO_ONE; - } }; @Test From ad1d6348ce63622149da71ae870e829328a968b6 Mon Sep 17 00:00:00 2001 From: "navis.ryu" Date: Wed, 20 Apr 2016 14:10:50 +0900 Subject: [PATCH 2/2] rebased to master --- .../query/extraction/DimExtractionFn.java | 2 +- .../extraction/JavaScriptExtractionFn.java | 4 +++ .../query/extraction/LowerExtractionFn.java | 2 +- .../query/extraction/UpperExtractionFn.java | 2 +- .../druid/segment/column/ValueAccessor.java | 31 ++++++++++++++----- .../JavaScriptExtractionFnTest.java | 24 ++++++++++---- 6 files changed, 49 insertions(+), 16 deletions(-) diff --git a/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java index fa22594d3d20..8557023837fb 100644 --- a/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java @@ -22,7 +22,7 @@ public abstract class DimExtractionFn extends AbstractExtractionFn { public String apply(Object value) { - return apply(accessor.getString(value, false)); + return apply(accessor.getString(value)); } protected abstract String apply(String value); diff --git a/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java index 79ced6667776..3cf22b783f00 100644 --- a/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java @@ -25,6 +25,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.metamx.common.StringUtils; +import io.druid.segment.column.ValueAccessor; import org.mozilla.javascript.Context; import org.mozilla.javascript.ContextFactory; import org.mozilla.javascript.ScriptableObject; @@ -103,6 +104,9 @@ public byte[] getCacheKey() @Override public String apply(Object value) { + if (accessor == ValueAccessor.STRING) { + value = Strings.emptyToNull(accessor.getString(value)); + } return Strings.emptyToNull(fn.apply(value)); } diff --git a/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java index 19efb6f1dd7e..3c8742c30172 100644 --- a/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java @@ -54,7 +54,7 @@ public LowerExtractionFn(@JsonProperty("locale") String localeString) @Override public String apply(Object key) { - final String value = accessor.getString(key, false); + final String value = accessor.getString(key); return Strings.isNullOrEmpty(value) ? null : value.toLowerCase(locale); } diff --git a/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java index 0af8bfc36f77..eb874a63b86c 100644 --- a/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java @@ -53,7 +53,7 @@ public UpperExtractionFn(@JsonProperty("locale") String localeString) @Override public String apply(Object key) { - final String value = accessor.getString(key, false); + final String value = accessor.getString(key); return Strings.isNullOrEmpty(value) ? null : value.toUpperCase(locale); } diff --git a/processing/src/main/java/io/druid/segment/column/ValueAccessor.java b/processing/src/main/java/io/druid/segment/column/ValueAccessor.java index 3272ed6cfa25..f95df98080fe 100644 --- a/processing/src/main/java/io/druid/segment/column/ValueAccessor.java +++ b/processing/src/main/java/io/druid/segment/column/ValueAccessor.java @@ -27,7 +27,9 @@ public interface ValueAccessor long getLong(Object value); - String getString(Object value, boolean nullToEmpty); + float getFloat(Object value); + + String getString(Object value); Object getObject(Object value); @@ -41,9 +43,15 @@ public long getLong(Object value) } @Override - public String getString(Object value, boolean nullToEmpty) + public float getFloat(Object value) + { + throw new UnsupportedOperationException("getFloat"); + } + + @Override + public String getString(Object value) { - throw new UnsupportedOperationException("getString"); + return value == null ? null : String.valueOf(value); } @Override @@ -68,9 +76,9 @@ public long getLong(Object value) } @Override - public String getString(Object value, boolean nullToEmpty) + public float getFloat(Object value) { - return (String) value; + return Float.valueOf((String) value); } }; @@ -87,11 +95,20 @@ public long getLong(Object value) { return (Long) value; } + }; + + ValueAccessor FLOAT = new AbstractValueAccessor() + { + @Override + public ValueType type() + { + return ValueType.FLOAT; + } @Override - public String getString(Object value, boolean nullToEmpty) + public float getFloat(Object value) { - return value != null ? String.valueOf(value) : nullToEmpty ? "" : null; + return (Float) value; } }; diff --git a/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java b/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java index 0a9b86b020f3..60ce8080edc0 100644 --- a/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java +++ b/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.Iterators; import com.google.common.collect.Lists; import io.druid.jackson.DefaultObjectMapper; +import io.druid.segment.column.ValueAccessor; import org.joda.time.DateTime; import org.junit.Assert; import org.junit.Test; @@ -59,21 +60,27 @@ public void testTimeExample() throws Exception { String utcHour = "function(t) {\nreturn 'Second ' + Math.floor((t % 60000) / 1000);\n}"; final long millis = new DateTime("2015-01-02T13:00:59.999Z").getMillis(); - Assert.assertEquals("Second 59" , new JavaScriptExtractionFn(utcHour, false).apply(millis)); + JavaScriptExtractionFn extractionFn = new JavaScriptExtractionFn(utcHour, false); + extractionFn.init(ValueAccessor.LONG); + Assert.assertEquals("Second 59", extractionFn.apply(millis)); } @Test public void testLongs() throws Exception { String typeOf = "function(x) {\nreturn typeof x\n}"; - Assert.assertEquals("number", new JavaScriptExtractionFn(typeOf, false).apply(1234L)); + JavaScriptExtractionFn extractionFn = new JavaScriptExtractionFn(typeOf, false); + extractionFn.init(ValueAccessor.LONG); + Assert.assertEquals("number", extractionFn.apply(1234L)); } @Test public void testFloats() throws Exception { String typeOf = "function(x) {\nreturn typeof x\n}"; - Assert.assertEquals("number", new JavaScriptExtractionFn(typeOf, false).apply(1234.0)); + JavaScriptExtractionFn extractionFn = new JavaScriptExtractionFn(typeOf, false); + extractionFn.init(ValueAccessor.FLOAT); + Assert.assertEquals("number", extractionFn.apply(1234.0)); } @Test @@ -110,12 +117,17 @@ public void testJavascriptIsNull() String function = "function(x) { if (x == null) { return 'yes'; } else { return 'no' } }"; ExtractionFn extractionFn = new JavaScriptExtractionFn(function, false); - Assert.assertEquals("yes", extractionFn.apply((String) null)); - Assert.assertEquals("yes", extractionFn.apply((Object) null)); + extractionFn.init(ValueAccessor.STRING); + Assert.assertEquals("yes", extractionFn.apply(null)); Assert.assertEquals("yes", extractionFn.apply("")); Assert.assertEquals("no", extractionFn.apply("abc")); + + extractionFn.init(ValueAccessor.COMPLEX); Assert.assertEquals("no", extractionFn.apply(new Object())); - Assert.assertEquals("no", extractionFn.apply(1)); + Assert.assertEquals("yes", extractionFn.apply(null)); + + extractionFn.init(ValueAccessor.LONG); + Assert.assertEquals("no", extractionFn.apply(1L)); } @Test