From 460bceb7d7fef7531add70724607455faff7ef3c Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Thu, 10 Jun 2021 14:05:44 -0700 Subject: [PATCH 1/4] enrich expression cache key information to support expressions which depend on external state such as lookups --- .../java/org/apache/druid/math/expr/Expr.java | 11 +++++++- .../druid/query/cache/CacheKeyBuilder.java | 0 .../druid/math/expr/ApplyFunctionTest.java | 6 +++++ .../apache/druid/math/expr/FunctionTest.java | 5 ++++ .../query/cache/CacheKeyBuilderTest.java | 0 .../DoubleMaxAggregatorFactory.java | 6 ++++- .../DoubleMinAggregatorFactory.java | 6 ++++- .../DoubleSumAggregatorFactory.java | 6 ++++- .../ExpressionLambdaAggregatorFactory.java | 8 +++--- .../FloatMaxAggregatorFactory.java | 6 ++++- .../FloatMinAggregatorFactory.java | 6 ++++- .../FloatSumAggregatorFactory.java | 6 ++++- .../aggregation/LongMaxAggregatorFactory.java | 6 ++++- .../aggregation/LongMinAggregatorFactory.java | 6 ++++- .../aggregation/LongSumAggregatorFactory.java | 6 ++++- .../post/ExpressionPostAggregator.java | 2 +- .../query/expression/LookupExprMacro.java | 8 ++++++ .../query/filter/ExpressionDimFilter.java | 2 +- .../virtual/ExpressionVirtualColumn.java | 2 +- .../LookupEnabledTestExprMacroTable.java | 18 ++++++++----- .../query/expression/LookupExprMacroTest.java | 27 +++++++++++++++++++ 21 files changed, 119 insertions(+), 24 deletions(-) rename {processing => core}/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java (100%) rename {processing => core}/src/test/java/org/apache/druid/query/cache/CacheKeyBuilderTest.java (100%) diff --git a/core/src/main/java/org/apache/druid/math/expr/Expr.java b/core/src/main/java/org/apache/druid/math/expr/Expr.java index 4df2bf845d2a..fa89ae924979 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Expr.java +++ b/core/src/main/java/org/apache/druid/math/expr/Expr.java @@ -23,8 +23,10 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; +import org.apache.druid.java.util.common.Cacheable; import org.apache.druid.java.util.common.ISE; import org.apache.druid.math.expr.vector.ExprVectorProcessor; +import org.apache.druid.query.cache.CacheKeyBuilder; import javax.annotation.Nullable; import java.util.ArrayList; @@ -38,8 +40,9 @@ * immutable. */ @SubclassesMustOverrideEqualsAndHashCode -public interface Expr +public interface Expr extends Cacheable { + byte EXPR_CACHE_KEY = 0x00; String NULL_LITERAL = "null"; Joiner ARG_JOINER = Joiner.on(", "); @@ -171,6 +174,12 @@ default ExprVectorProcessor buildVectorized(VectorInputBindingInspector i throw Exprs.cannotVectorize(this); } + @Override + default byte[] getCacheKey() + { + return new CacheKeyBuilder(EXPR_CACHE_KEY).appendString(stringify()).build(); + } + /** * Mechanism to supply input types for the bindings which will back {@link IdentifierExpr}, to use in the aid of * inferring the output type of an expression with {@link #getOutputType}. A null value means that either the binding diff --git a/processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java b/core/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java similarity index 100% rename from processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java rename to core/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java diff --git a/core/src/test/java/org/apache/druid/math/expr/ApplyFunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/ApplyFunctionTest.java index d352bfd67725..a63f0ecb5a4d 100644 --- a/core/src/test/java/org/apache/druid/math/expr/ApplyFunctionTest.java +++ b/core/src/test/java/org/apache/druid/math/expr/ApplyFunctionTest.java @@ -171,6 +171,8 @@ private void assertExpr(final String expression, final Object expectedResult) Assert.assertEquals(expr.stringify(), roundTrip.stringify()); Assert.assertEquals(expr.stringify(), roundTripFlatten.stringify()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTrip.getCacheKey()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTripFlatten.getCacheKey()); } private void assertExpr(final String expression, final Object[] expectedResult) @@ -196,6 +198,8 @@ private void assertExpr(final String expression, final Object[] expectedResult) Assert.assertEquals(expr.stringify(), roundTrip.stringify()); Assert.assertEquals(expr.stringify(), roundTripFlatten.stringify()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTrip.getCacheKey()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTripFlatten.getCacheKey()); } private void assertExpr(final String expression, final Double[] expectedResult) @@ -224,5 +228,7 @@ private void assertExpr(final String expression, final Double[] expectedResult) Assert.assertEquals(expr.stringify(), roundTrip.stringify()); Assert.assertEquals(expr.stringify(), roundTripFlatten.stringify()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTrip.getCacheKey()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTripFlatten.getCacheKey()); } } diff --git a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java index 155774958b43..5ded90fd25f9 100644 --- a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java +++ b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java @@ -605,11 +605,14 @@ private void assertExpr(final String expression, @Nullable final Object expected final Expr roundTrip = Parser.parse(exprNoFlatten.stringify(), ExprMacroTable.nil()); Assert.assertEquals(expr.stringify(), expectedResult, roundTrip.eval(bindings).value()); + final Expr roundTripFlatten = Parser.parse(expr.stringify(), ExprMacroTable.nil()); Assert.assertEquals(expr.stringify(), expectedResult, roundTripFlatten.eval(bindings).value()); Assert.assertEquals(expr.stringify(), roundTrip.stringify()); Assert.assertEquals(expr.stringify(), roundTripFlatten.stringify()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTrip.getCacheKey()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTripFlatten.getCacheKey()); } private void assertArrayExpr(final String expression, @Nullable final Object[] expectedResult) @@ -626,5 +629,7 @@ private void assertArrayExpr(final String expression, @Nullable final Object[] e Assert.assertEquals(expr.stringify(), roundTrip.stringify()); Assert.assertEquals(expr.stringify(), roundTripFlatten.stringify()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTrip.getCacheKey()); + Assert.assertArrayEquals(expr.getCacheKey(), roundTripFlatten.getCacheKey()); } } diff --git a/processing/src/test/java/org/apache/druid/query/cache/CacheKeyBuilderTest.java b/core/src/test/java/org/apache/druid/query/cache/CacheKeyBuilderTest.java similarity index 100% rename from processing/src/test/java/org/apache/druid/query/cache/CacheKeyBuilderTest.java rename to core/src/test/java/org/apache/druid/query/cache/CacheKeyBuilderTest.java diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java index ce22009f2577..9afbf0093b92 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseDoubleColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.DOUBLE_MAX_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java index 9c068ce6404b..f4921d230bfc 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseDoubleColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.DOUBLE_MIN_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java index 1753d18103cf..fd1e91e37df9 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseDoubleColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.DOUBLE_SUM_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/ExpressionLambdaAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/ExpressionLambdaAggregatorFactory.java index e40000df3a83..fd918fe428ce 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/ExpressionLambdaAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/ExpressionLambdaAggregatorFactory.java @@ -234,10 +234,10 @@ public byte[] getCacheKey() .appendStrings(fields) .appendString(initialValueExpressionString) .appendString(initialCombineValueExpressionString) - .appendString(foldExpressionString) - .appendString(combineExpressionString) - .appendString(compareExpressionString) - .appendString(finalizeExpressionString) + .appendCacheable(foldExpression.get()) + .appendCacheable(combineExpression.get()) + .appendCacheable(combineExpression.get()) + .appendCacheable(finalizeExpression.get()) .appendInt(maxSizeBytes.getBytesInInt()) .build(); } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java index 9398f622250a..7e9f7e9d4647 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.FLOAT_MAX_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java index 465aa381d41e..3f562f404224 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.FLOAT_MIN_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java index 3175b401ad29..bd1d14e7df53 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.FLOAT_SUM_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java index a9bc7c5ae57b..87bab787d8a7 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.LONG_MAX_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java index 18a3ee684849..a752919fab55 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.LONG_MIN_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java index 8c9364f0de59..dacd67b3f388 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -32,6 +33,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; +import java.util.Optional; /** */ @@ -115,7 +117,9 @@ public List getRequiredColumns() public byte[] getCacheKey() { byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = StringUtils.toUtf8WithNullToEmpty(expression); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) .put(AggregatorUtil.LONG_SUM_CACHE_TYPE_ID) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java b/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java index b05ecbe35089..c81d404550b1 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java @@ -230,7 +230,7 @@ public String toString() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.EXPRESSION) - .appendString(expression) + .appendCacheable(parsed.get()) .appendString(ordering) .build(); } diff --git a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java index 9b2f96e88559..2f3a403eb8ef 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java @@ -27,6 +27,7 @@ import org.apache.druid.math.expr.ExprEval; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.math.expr.ExprType; +import org.apache.druid.query.cache.CacheKeyBuilder; import org.apache.druid.query.lookup.LookupExtractorFactoryContainerProvider; import org.apache.druid.query.lookup.RegisteredLookupExtractionFn; @@ -36,6 +37,7 @@ public class LookupExprMacro implements ExprMacroTable.ExprMacro { + private static final byte LOOKUP_EXPR_KEY = 0x01; private static final String FN_NAME = "lookup"; private final LookupExtractorFactoryContainerProvider lookupExtractorFactoryContainerProvider; @@ -108,6 +110,12 @@ public String stringify() { return StringUtils.format("%s(%s, %s)", FN_NAME, arg.stringify(), lookupExpr.stringify()); } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(LOOKUP_EXPR_KEY).appendString(stringify()).appendCacheable(extractionFn).build(); + } } return new LookupExpr(arg); diff --git a/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java index 66927332df24..f9d23b50ed32 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java @@ -103,7 +103,7 @@ public Set getRequiredColumns() public byte[] getCacheKey() { return new CacheKeyBuilder(DimFilterUtils.EXPRESSION_CACHE_ID) - .appendString(expression) + .appendCacheable(parsed.get()) .build(); } diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java index 343cd8a4978e..76351c0dc309 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java @@ -261,7 +261,7 @@ public byte[] getCacheKey() { CacheKeyBuilder builder = new CacheKeyBuilder(VirtualColumnCacheHelper.CACHE_TYPE_ID_EXPRESSION) .appendString(name) - .appendString(expression); + .appendCacheable(parsedExpression.get()); if (outputType != null) { builder.appendString(outputType.toString()); diff --git a/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java b/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java index 503fa1cd8243..07f225d7eef2 100644 --- a/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java +++ b/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java @@ -33,6 +33,7 @@ import javax.annotation.Nullable; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -48,13 +49,16 @@ public class LookupEnabledTestExprMacroTable extends ExprMacroTable private LookupEnabledTestExprMacroTable() { - super( - Lists.newArrayList( - Iterables.concat( - TestExprMacroTable.INSTANCE.getMacros(), - Collections.singletonList( - new LookupExprMacro(createTestLookupProvider(ImmutableMap.of("foo", "xfoo"))) - ) + super(makeTestMacros(ImmutableMap.of("foo", "xfoo"))); + } + + public static List makeTestMacros(final Map theLookup) + { + return Lists.newArrayList( + Iterables.concat( + TestExprMacroTable.INSTANCE.getMacros(), + Collections.singletonList( + new LookupExprMacro(createTestLookupProvider(theLookup)) ) ) ); diff --git a/server/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java b/server/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java index ce8c9e3b41a6..b330bda870d2 100644 --- a/server/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java +++ b/server/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.math.expr.InputBindings; import org.apache.druid.math.expr.Parser; import org.apache.druid.testing.InitializedNullHandlingTest; @@ -54,6 +55,32 @@ public void testLookupNotFound() assertExpr("lookup(x, 'lookylook')", null); } + @Test + public void testCacheKeyChangesWhenLookupChanges() + { + final String expression = "lookup(x, 'lookyloo')"; + final Expr expr = Parser.parse(expression, LookupEnabledTestExprMacroTable.INSTANCE); + final Expr exprSameLookup = Parser.parse(expression, LookupEnabledTestExprMacroTable.INSTANCE); + final Expr exprChangedLookup = Parser.parse( + expression, + new ExprMacroTable(LookupEnabledTestExprMacroTable.makeTestMacros(ImmutableMap.of("x", "y", "a", "b"))) + ); + // same should have same cache key + Assert.assertArrayEquals(expr.getCacheKey(), exprSameLookup.getCacheKey()); + // different should not have same key + final byte[] exprBytes = expr.getCacheKey(); + final byte[] expr2Bytes = exprChangedLookup.getCacheKey(); + if (exprBytes.length == expr2Bytes.length) { + // only check for equality if lengths are equal + boolean allEqual = true; + for (int i = 0; i < exprBytes.length; i++) { + allEqual = allEqual && (exprBytes[i] == expr2Bytes[i]); + } + Assert.assertFalse(allEqual); + } + } + + private void assertExpr(final String expression, final Object expectedResult) { final Expr expr = Parser.parse(expression, LookupEnabledTestExprMacroTable.INSTANCE); From fedd28e5b27e80090ec69ef6435d1b7418436726 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Thu, 10 Jun 2021 20:51:21 -0700 Subject: [PATCH 2/4] cache rules everything around me --- .../java/org/apache/druid/math/expr/Expr.java | 4 +-- .../org/apache/druid/math/expr/Exprs.java | 3 ++ .../DoubleMaxAggregatorFactory.java | 29 ++++++++++++------- .../DoubleMinAggregatorFactory.java | 29 ++++++++++++------- .../DoubleSumAggregatorFactory.java | 29 ++++++++++++------- .../FloatMaxAggregatorFactory.java | 29 ++++++++++++------- .../FloatMinAggregatorFactory.java | 29 ++++++++++++------- .../FloatSumAggregatorFactory.java | 29 ++++++++++++------- .../aggregation/LongMaxAggregatorFactory.java | 29 ++++++++++++------- .../aggregation/LongMinAggregatorFactory.java | 29 ++++++++++++------- .../aggregation/LongSumAggregatorFactory.java | 29 ++++++++++++------- .../SimpleDoubleAggregatorFactory.java | 1 + .../post/ExpressionPostAggregator.java | 12 +++++--- .../query/expression/LookupExprMacro.java | 6 ++-- .../query/filter/ExpressionDimFilter.java | 11 +++++-- .../virtual/ExpressionVirtualColumn.java | 26 ++++++++++++----- 16 files changed, 206 insertions(+), 118 deletions(-) diff --git a/core/src/main/java/org/apache/druid/math/expr/Expr.java b/core/src/main/java/org/apache/druid/math/expr/Expr.java index fa89ae924979..a61a6d50717c 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Expr.java +++ b/core/src/main/java/org/apache/druid/math/expr/Expr.java @@ -42,7 +42,7 @@ @SubclassesMustOverrideEqualsAndHashCode public interface Expr extends Cacheable { - byte EXPR_CACHE_KEY = 0x00; + String NULL_LITERAL = "null"; Joiner ARG_JOINER = Joiner.on(", "); @@ -177,7 +177,7 @@ default ExprVectorProcessor buildVectorized(VectorInputBindingInspector i @Override default byte[] getCacheKey() { - return new CacheKeyBuilder(EXPR_CACHE_KEY).appendString(stringify()).build(); + return new CacheKeyBuilder(Exprs.EXPR_CACHE_KEY).appendString(stringify()).build(); } /** diff --git a/core/src/main/java/org/apache/druid/math/expr/Exprs.java b/core/src/main/java/org/apache/druid/math/expr/Exprs.java index cdff172361f9..c779eb2840e9 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Exprs.java +++ b/core/src/main/java/org/apache/druid/math/expr/Exprs.java @@ -29,6 +29,9 @@ public class Exprs { + public static final byte EXPR_CACHE_KEY = 0x00; + public static final byte LOOKUP_EXPR_KEY = 0x01; + public static UnsupportedOperationException cannotVectorize(Expr expr) { return new UOE("Unable to vectorize expression:[%s]", expr.stringify()); diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java index 9afbf0093b92..6dd14dead673 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class DoubleMaxAggregatorFactory extends SimpleDoubleAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public DoubleMaxAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public DoubleMaxAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.DOUBLE_MAX_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public DoubleMaxAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.DOUBLE_MAX_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java index f4921d230bfc..3085b61820e6 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class DoubleMinAggregatorFactory extends SimpleDoubleAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public DoubleMinAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public DoubleMinAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.DOUBLE_MIN_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public DoubleMinAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.DOUBLE_MIN_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java index fd1e91e37df9..1ce51d0ea24a 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class DoubleSumAggregatorFactory extends SimpleDoubleAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public DoubleSumAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public DoubleSumAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.DOUBLE_SUM_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public DoubleSumAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.DOUBLE_SUM_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java index 7e9f7e9d4647..4f94c0922415 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class FloatMaxAggregatorFactory extends SimpleFloatAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public FloatMaxAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public FloatMaxAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.FLOAT_MAX_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public FloatMaxAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.FLOAT_MAX_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java index 3f562f404224..b31128d2b810 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class FloatMinAggregatorFactory extends SimpleFloatAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public FloatMinAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public FloatMinAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.FLOAT_MIN_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public FloatMinAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.FLOAT_MIN_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java index bd1d14e7df53..944331685e48 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class FloatSumAggregatorFactory extends SimpleFloatAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public FloatSumAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public FloatSumAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.FLOAT_SUM_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public FloatSumAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.FLOAT_SUM_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java index 87bab787d8a7..83a0be4d88e6 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class LongMaxAggregatorFactory extends SimpleLongAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public LongMaxAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public LongMaxAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.LONG_MAX_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public LongMaxAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.LONG_MAX_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java index a752919fab55..de3ba25b3474 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class LongMinAggregatorFactory extends SimpleLongAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public LongMinAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public LongMinAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.LONG_MIN_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public LongMinAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.LONG_MIN_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java index dacd67b3f388..e7c97fef8c84 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -39,6 +41,8 @@ */ public class LongSumAggregatorFactory extends SimpleLongAggregatorFactory { + private final Supplier cacheKey; + @JsonCreator public LongSumAggregatorFactory( @JsonProperty("name") String name, @@ -48,6 +52,19 @@ public LongSumAggregatorFactory( ) { super(macroTable, name, fieldName, expression); + this.cacheKey = Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(AggregatorUtil.LONG_SUM_CACHE_TYPE_ID) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); } public LongSumAggregatorFactory(String name, String fieldName) @@ -116,17 +133,7 @@ public List getRequiredColumns() @Override public byte[] getCacheKey() { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.LONG_SUM_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SimpleDoubleAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/SimpleDoubleAggregatorFactory.java index c540018dc6c4..bd44361e38ec 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SimpleDoubleAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SimpleDoubleAggregatorFactory.java @@ -59,6 +59,7 @@ public abstract class SimpleDoubleAggregatorFactory extends NullableNumericAggre protected final boolean storeDoubleAsFloat; protected final Supplier fieldExpression; + public SimpleDoubleAggregatorFactory( ExprMacroTable macroTable, String name, diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java b/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java index c81d404550b1..cab72bd20de8 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/post/ExpressionPostAggregator.java @@ -75,6 +75,7 @@ public class ExpressionPostAggregator implements PostAggregator private final Supplier parsed; private final Supplier> dependentFields; + private final Supplier cacheKey; /** * Constructor for serialization. @@ -144,6 +145,12 @@ private ExpressionPostAggregator( this.parsed = parsed; this.dependentFields = dependentFields; + this.cacheKey = Suppliers.memoize(() -> { + return new CacheKeyBuilder(PostAggregatorIds.EXPRESSION) + .appendCacheable(parsed.get()) + .appendString(ordering) + .build(); + }); } @@ -229,10 +236,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.EXPRESSION) - .appendCacheable(parsed.get()) - .appendString(ordering) - .build(); + return cacheKey.get(); } public enum Ordering implements Comparator diff --git a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java index 2f3a403eb8ef..059f1dc8fbb3 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java @@ -27,6 +27,7 @@ import org.apache.druid.math.expr.ExprEval; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.math.expr.ExprType; +import org.apache.druid.math.expr.Exprs; import org.apache.druid.query.cache.CacheKeyBuilder; import org.apache.druid.query.lookup.LookupExtractorFactoryContainerProvider; import org.apache.druid.query.lookup.RegisteredLookupExtractionFn; @@ -37,7 +38,6 @@ public class LookupExprMacro implements ExprMacroTable.ExprMacro { - private static final byte LOOKUP_EXPR_KEY = 0x01; private static final String FN_NAME = "lookup"; private final LookupExtractorFactoryContainerProvider lookupExtractorFactoryContainerProvider; @@ -114,7 +114,9 @@ public String stringify() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(LOOKUP_EXPR_KEY).appendString(stringify()).appendCacheable(extractionFn).build(); + return new CacheKeyBuilder(Exprs.LOOKUP_EXPR_KEY).appendString(stringify()) + .appendCacheable(extractionFn) + .build(); } } diff --git a/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java index f9d23b50ed32..32fc2c39dfd3 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java @@ -25,6 +25,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.RangeSet; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; @@ -40,6 +41,7 @@ public class ExpressionDimFilter extends AbstractOptimizableDimFilter implements { private final String expression; private final Supplier parsed; + private final Supplier cacheKey; @Nullable private final FilterTuning filterTuning; @@ -53,6 +55,11 @@ public ExpressionDimFilter( this.expression = expression; this.filterTuning = filterTuning; this.parsed = Parser.lazyParse(expression, macroTable); + this.cacheKey = Suppliers.memoize(() -> { + return new CacheKeyBuilder(DimFilterUtils.EXPRESSION_CACHE_ID) + .appendCacheable(parsed.get()) + .build(); + }); } @VisibleForTesting @@ -102,9 +109,7 @@ public Set getRequiredColumns() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(DimFilterUtils.EXPRESSION_CACHE_ID) - .appendCacheable(parsed.get()) - .build(); + return cacheKey.get(); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java index 76351c0dc309..c0de0876c5a6 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java @@ -60,6 +60,7 @@ public class ExpressionVirtualColumn implements VirtualColumn @Nullable private final ValueType outputType; private final Supplier parsedExpression; + private final Supplier cacheKey; @JsonCreator public ExpressionVirtualColumn( @@ -73,6 +74,7 @@ public ExpressionVirtualColumn( this.expression = Preconditions.checkNotNull(expression, "expression"); this.outputType = outputType; this.parsedExpression = Parser.lazyParse(expression, macroTable); + this.cacheKey = makeCacheKeySupplier(); } /** @@ -90,6 +92,7 @@ public ExpressionVirtualColumn( this.expression = parsedExpression.toString(); this.outputType = outputType; this.parsedExpression = Suppliers.ofInstance(parsedExpression); + this.cacheKey = makeCacheKeySupplier(); } @JsonProperty("name") @@ -259,14 +262,7 @@ public boolean usesDotNotation() @Override public byte[] getCacheKey() { - CacheKeyBuilder builder = new CacheKeyBuilder(VirtualColumnCacheHelper.CACHE_TYPE_ID_EXPRESSION) - .appendString(name) - .appendCacheable(parsedExpression.get()); - - if (outputType != null) { - builder.appendString(outputType.toString()); - } - return builder.build(); + return cacheKey.get(); } @Override @@ -299,4 +295,18 @@ public String toString() ", outputType=" + outputType + '}'; } + + private Supplier makeCacheKeySupplier() + { + return Suppliers.memoize(() -> { + CacheKeyBuilder builder = new CacheKeyBuilder(VirtualColumnCacheHelper.CACHE_TYPE_ID_EXPRESSION) + .appendString(name) + .appendCacheable(parsedExpression.get()); + + if (outputType != null) { + builder.appendString(outputType.toString()); + } + return builder.build(); + }); + } } From 60d304ce43d7e8b804a1f97a24d533eab886732a Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Thu, 10 Jun 2021 21:01:55 -0700 Subject: [PATCH 3/4] low carb --- .../query/aggregation/AggregatorUtil.java | 25 +++++++++++++++++++ .../DoubleMaxAggregatorFactory.java | 23 ++++------------- .../DoubleMinAggregatorFactory.java | 23 ++++------------- .../DoubleSumAggregatorFactory.java | 25 +++++-------------- .../FloatMaxAggregatorFactory.java | 23 ++++------------- .../FloatMinAggregatorFactory.java | 23 ++++------------- .../FloatSumAggregatorFactory.java | 23 ++++------------- .../aggregation/LongMaxAggregatorFactory.java | 23 ++++------------- .../aggregation/LongMinAggregatorFactory.java | 23 ++++------------- .../aggregation/LongSumAggregatorFactory.java | 23 ++++------------- 10 files changed, 71 insertions(+), 163 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorUtil.java b/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorUtil.java index 34a6eada4994..cb91b5b3c33c 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorUtil.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorUtil.java @@ -20,9 +20,11 @@ package org.apache.druid.query.aggregation; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.Lists; import org.apache.druid.guice.annotations.PublicApi; import org.apache.druid.java.util.common.Pair; +import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprEval; import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector; @@ -40,10 +42,12 @@ import org.apache.druid.segment.virtual.ExpressionVectorSelectors; import javax.annotation.Nullable; +import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; @PublicApi @@ -360,4 +364,25 @@ public static VectorValueSelector makeVectorValueSelector( } return columnSelectorFactory.makeValueSelector(fieldName); } + + public static Supplier getSimpleAggregatorCacheKeySupplier( + byte aggregatorType, + String fieldName, + Supplier fieldExpression + ) + { + return Suppliers.memoize(() -> { + byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); + byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) + .map(Expr::getCacheKey) + .orElse(StringUtils.EMPTY_BYTES); + + return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) + .put(aggregatorType) + .put(fieldNameBytes) + .put(AggregatorUtil.STRING_SEPARATOR) + .put(expressionBytes) + .array(); + }); + } } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java index 6dd14dead673..79bd6c6aaaf1 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMaxAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseDoubleColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public DoubleMaxAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.DOUBLE_MAX_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.DOUBLE_MAX_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public DoubleMaxAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java index 3085b61820e6..124200cdf45a 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleMinAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseDoubleColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public DoubleMinAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.DOUBLE_MIN_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.DOUBLE_MIN_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public DoubleMinAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java index 1ce51d0ea24a..1958c0deddf5 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/DoubleSumAggregatorFactory.java @@ -23,26 +23,21 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseDoubleColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ public class DoubleSumAggregatorFactory extends SimpleDoubleAggregatorFactory { private final Supplier cacheKey; - + @JsonCreator public DoubleSumAggregatorFactory( @JsonProperty("name") String name, @@ -52,19 +47,11 @@ public DoubleSumAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.DOUBLE_SUM_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.DOUBLE_SUM_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public DoubleSumAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java index 4f94c0922415..8b6dccf9206d 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMaxAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public FloatMaxAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.FLOAT_MAX_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.FLOAT_MAX_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public FloatMaxAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java index b31128d2b810..cf35efaeceef 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatMinAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public FloatMinAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.FLOAT_MIN_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.FLOAT_MIN_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public FloatMinAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java index 944331685e48..7d47a36d62ec 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/FloatSumAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public FloatSumAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.FLOAT_SUM_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.FLOAT_SUM_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public FloatSumAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java index 83a0be4d88e6..9260b6d453a5 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongMaxAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public LongMaxAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.LONG_MAX_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.LONG_MAX_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public LongMaxAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java index de3ba25b3474..5509014bfe51 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongMinAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public LongMinAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.LONG_MIN_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.LONG_MIN_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public LongMinAggregatorFactory(String name, String fieldName) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java index e7c97fef8c84..3fd8cf0fb276 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/LongSumAggregatorFactory.java @@ -23,19 +23,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; -import java.util.Optional; /** */ @@ -52,19 +47,11 @@ public LongSumAggregatorFactory( ) { super(macroTable, name, fieldName, expression); - this.cacheKey = Suppliers.memoize(() -> { - byte[] fieldNameBytes = StringUtils.toUtf8WithNullToEmpty(fieldName); - byte[] expressionBytes = Optional.ofNullable(fieldExpression.get()) - .map(Expr::getCacheKey) - .orElse(StringUtils.EMPTY_BYTES); - - return ByteBuffer.allocate(2 + fieldNameBytes.length + expressionBytes.length) - .put(AggregatorUtil.LONG_SUM_CACHE_TYPE_ID) - .put(fieldNameBytes) - .put(AggregatorUtil.STRING_SEPARATOR) - .put(expressionBytes) - .array(); - }); + this.cacheKey = AggregatorUtil.getSimpleAggregatorCacheKeySupplier( + AggregatorUtil.LONG_SUM_CACHE_TYPE_ID, + fieldName, + fieldExpression + ); } public LongSumAggregatorFactory(String name, String fieldName) From d0ac4804256caf661b6df41a7a0e6bcb08a20cbe Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 11 Jun 2021 11:40:58 -0700 Subject: [PATCH 4/4] rename --- core/src/main/java/org/apache/druid/math/expr/Exprs.java | 2 +- .../org/apache/druid/query/expression/LookupExprMacro.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/druid/math/expr/Exprs.java b/core/src/main/java/org/apache/druid/math/expr/Exprs.java index c779eb2840e9..618afa5b4f04 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Exprs.java +++ b/core/src/main/java/org/apache/druid/math/expr/Exprs.java @@ -30,7 +30,7 @@ public class Exprs { public static final byte EXPR_CACHE_KEY = 0x00; - public static final byte LOOKUP_EXPR_KEY = 0x01; + public static final byte LOOKUP_EXPR_CACHE_KEY = 0x01; public static UnsupportedOperationException cannotVectorize(Expr expr) { diff --git a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java index 059f1dc8fbb3..9b25e5de62a7 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java @@ -114,9 +114,9 @@ public String stringify() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(Exprs.LOOKUP_EXPR_KEY).appendString(stringify()) - .appendCacheable(extractionFn) - .build(); + return new CacheKeyBuilder(Exprs.LOOKUP_EXPR_CACHE_KEY).appendString(stringify()) + .appendCacheable(extractionFn) + .build(); } }