-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: add pinot function registry to translate functions (#21)
* feat: add pinot function registry to translate functions * Apply suggestions from code review
- Loading branch information
1 parent
2bafdd4
commit b3513b0
Showing
9 changed files
with
399 additions
and
52 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
15 changes: 15 additions & 0 deletions
15
query-service-impl/src/main/java/org/hypertrace/core/query/service/ConfigUtils.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package org.hypertrace.core.query.service; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.function.Supplier; | ||
|
|
||
| public class ConfigUtils { | ||
|
|
||
| public static <T> Optional<T> optionallyGet(Supplier<T> strictGet) { | ||
| try { | ||
| return Optional.ofNullable(strictGet.get()); | ||
| } catch (Throwable t) { | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
| } |
16 changes: 16 additions & 0 deletions
16
...-service-impl/src/main/java/org/hypertrace/core/query/service/QueryFunctionConstants.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package org.hypertrace.core.query.service; | ||
|
|
||
|
|
||
| /** | ||
| * Canonical query function names received in requests. These should be converted to data-store | ||
| * specific names when handling a request. | ||
| */ | ||
| public interface QueryFunctionConstants { | ||
| String QUERY_FUNCTION_SUM = "SUM"; | ||
| String QUERY_FUNCTION_AVG = "AVG"; | ||
| String QUERY_FUNCTION_MIN = "MIN"; | ||
| String QUERY_FUNCTION_MAX = "MAX"; | ||
| String QUERY_FUNCTION_COUNT = "COUNT"; | ||
| String QUERY_FUNCTION_PERCENTILE = "PERCENTILE"; | ||
| String QUERY_FUNCTION_DISTINCTCOUNT = "DISTINCTCOUNT"; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
130 changes: 130 additions & 0 deletions
130
.../main/java/org/hypertrace/core/query/service/pinot/converters/PinotFunctionConverter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| package org.hypertrace.core.query.service.pinot.converters; | ||
|
|
||
| import static org.hypertrace.core.query.service.QueryFunctionConstants.QUERY_FUNCTION_COUNT; | ||
| import static org.hypertrace.core.query.service.QueryFunctionConstants.QUERY_FUNCTION_PERCENTILE; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.stream.Collectors; | ||
| import org.hypertrace.core.query.service.api.Expression; | ||
| import org.hypertrace.core.query.service.api.Function; | ||
| import org.hypertrace.core.query.service.api.LiteralConstant; | ||
| import org.hypertrace.core.query.service.api.Value; | ||
| import org.hypertrace.core.query.service.api.ValueType; | ||
|
|
||
| public class PinotFunctionConverter { | ||
| /** | ||
| * Computing PERCENTILE in Pinot is resource intensive. T-Digest calculation is much faster and | ||
| * reasonably accurate, hence use that as the default. | ||
| */ | ||
| private static final String DEFAULT_PERCENTILE_AGGREGATION_FUNCTION = "PERCENTILETDIGEST"; | ||
|
|
||
| private final String percentileAggFunction; | ||
|
|
||
| public PinotFunctionConverter(String configuredPercentileFunction) { | ||
| this.percentileAggFunction = | ||
| Optional.ofNullable(configuredPercentileFunction) | ||
| .orElse(DEFAULT_PERCENTILE_AGGREGATION_FUNCTION); | ||
| } | ||
|
|
||
| public PinotFunctionConverter() { | ||
| this.percentileAggFunction = DEFAULT_PERCENTILE_AGGREGATION_FUNCTION; | ||
| } | ||
|
|
||
| public String convert( | ||
| Function function, java.util.function.Function<Expression, String> argumentConverter) { | ||
| switch (function.getFunctionName().toUpperCase()) { | ||
| case QUERY_FUNCTION_COUNT: | ||
| return this.convertCount(); | ||
| case QUERY_FUNCTION_PERCENTILE: | ||
| return this.functionToString(this.toPinotPercentile(function), argumentConverter); | ||
| default: | ||
| // TODO remove once pinot-specific logic removed from gateway - this normalization reverts | ||
| // that logic | ||
| if (this.isHardcodedPercentile(function)) { | ||
| return this.convert(this.normalizeHardcodedPercentile(function), argumentConverter); | ||
| } | ||
| return this.functionToString(function, argumentConverter); | ||
| } | ||
| } | ||
|
|
||
| private String functionToString( | ||
| Function function, java.util.function.Function<Expression, String> argumentConverter) { | ||
| String argumentString = | ||
| function.getArgumentsList().stream() | ||
| .map(argumentConverter::apply) | ||
| .collect(Collectors.joining(",")); | ||
|
|
||
| return function.getFunctionName() + "(" + argumentString + ")"; | ||
| } | ||
|
|
||
| private String convertCount() { | ||
| return "COUNT(*)"; | ||
| } | ||
|
|
||
| private Function toPinotPercentile(Function function) { | ||
| int percentileValue = | ||
| this.getPercentileValueFromFunction(function) | ||
| .orElseThrow( | ||
| () -> | ||
| new UnsupportedOperationException( | ||
| String.format( | ||
| "%s must include an integer convertible value as its first argument. Got: %s", | ||
| QUERY_FUNCTION_PERCENTILE, function.getArguments(0)))); | ||
| return Function.newBuilder(function) | ||
| .removeArguments(0) | ||
| .setFunctionName(this.percentileAggFunction + percentileValue) | ||
| .build(); | ||
| } | ||
|
|
||
| private boolean isHardcodedPercentile(Function function) { | ||
| String functionName = function.getFunctionName().toUpperCase(); | ||
| return functionName.startsWith(QUERY_FUNCTION_PERCENTILE) | ||
| && this.getPercentileValueFromName(functionName).isPresent(); | ||
| } | ||
|
|
||
| private Function normalizeHardcodedPercentile(Function function) { | ||
| // Verified in isHardcodedPercentile | ||
| int percentileValue = this.getPercentileValueFromName(function.getFunctionName()).orElseThrow(); | ||
| return Function.newBuilder(function) | ||
| .setFunctionName(QUERY_FUNCTION_PERCENTILE) | ||
| .addArguments(0, this.literalInt(percentileValue)) | ||
| .build(); | ||
| } | ||
|
|
||
| private Optional<Integer> getPercentileValueFromName(String functionName) { | ||
| try { | ||
| return Optional.of( | ||
| Integer.parseInt(functionName.substring(QUERY_FUNCTION_PERCENTILE.length()))); | ||
| } catch (Throwable t) { | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
|
|
||
| private Optional<Integer> getPercentileValueFromFunction(Function percentileFunction) { | ||
| return Optional.of(percentileFunction) | ||
| .filter(function -> function.getArgumentsCount() > 0) | ||
| .map(function -> function.getArguments(0)) | ||
| .map(Expression::getLiteral) | ||
| .map(LiteralConstant::getValue) | ||
| .flatMap(this::intFromValue); | ||
| } | ||
|
|
||
| Expression literalInt(int value) { | ||
| return Expression.newBuilder() | ||
| .setLiteral( | ||
| LiteralConstant.newBuilder() | ||
| .setValue(Value.newBuilder().setValueType(ValueType.INT).setInt(value))) | ||
| .build(); | ||
| } | ||
|
|
||
| Optional<Integer> intFromValue(Value value) { | ||
| switch (value.getValueType()) { | ||
| case INT: | ||
| return Optional.of(value.getInt()); | ||
| case LONG: | ||
| return Optional.of(Math.toIntExact(value.getLong())); | ||
| default: | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
| } |
25 changes: 25 additions & 0 deletions
25
query-service-impl/src/test/java/org/hypertrace/core/query/service/ConfigUtilsTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package org.hypertrace.core.query.service; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| import java.util.Optional; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class ConfigUtilsTest { | ||
|
|
||
| @Test | ||
| void optionallyGetReturnsEmptyIfThrows() { | ||
| assertEquals( | ||
| Optional.empty(), | ||
| ConfigUtils.optionallyGet( | ||
| () -> { | ||
| throw new RuntimeException(); | ||
| })); | ||
| } | ||
|
|
||
| @Test | ||
| void optionallyGetReturnsValueIfProvided() { | ||
| assertEquals(Optional.of("value"), ConfigUtils.optionallyGet(() -> "value")); | ||
| assertEquals(Optional.of(10), ConfigUtils.optionallyGet(() -> 10)); | ||
| } | ||
| } |
Oops, something went wrong.