diff --git a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotConfig.java b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotConfig.java index 872e7a9fd4f4d..bd16e0469f19e 100644 --- a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotConfig.java +++ b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotConfig.java @@ -105,6 +105,7 @@ public class PinotConfig private boolean useDateTrunc; private int nonAggregateLimitForBrokerQueries = DEFAULT_NON_AGGREGATE_LIMIT_FOR_BROKER_QUERIES; private boolean pushdownTopNBrokerQueries = true; + private boolean pushdownProjectExpressions = true; private String grpcHost; private int grpcPort = DEFAULT_PROXY_GRPC_PORT; private boolean useProxy; @@ -515,6 +516,18 @@ public PinotConfig setPushdownTopNBrokerQueries(boolean pushdownTopNBrokerQuerie return this; } + public boolean isPushdownProjectExpressions() + { + return pushdownProjectExpressions; + } + + @Config("pinot.pushdown-project-expressions") + public PinotConfig setPushdownProjectExpressions(boolean pushdownProjectExpressions) + { + this.pushdownProjectExpressions = pushdownProjectExpressions; + return this; + } + public boolean isUseStreamingForSegmentQueries() { return useStreamingForSegmentQueries; diff --git a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotSessionProperties.java b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotSessionProperties.java index 673291207bdf5..387cd8586f804 100644 --- a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotSessionProperties.java +++ b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/PinotSessionProperties.java @@ -41,6 +41,7 @@ public class PinotSessionProperties public static final String USE_PINOT_SQL_FOR_BROKER_QUERIES = "use_pinot_sql_for_broker_queries"; public static final String NON_AGGREGATE_LIMIT_FOR_BROKER_QUERIES = "non_aggregate_limit_for_broker_queries"; public static final String PUSHDOWN_TOPN_BROKER_QUERIES = "pushdown_topn_broker_queries"; + public static final String PUSHDOWN_PROJECT_EXPRESSIONS = "pushdown_project_expressions"; public static final String FORBID_SEGMENT_QUERIES = "forbid_segment_queries"; public static final String NUM_SEGMENTS_PER_SPLIT = "num_segments_per_split"; public static final String TOPN_LARGE = "topn_large"; @@ -105,6 +106,11 @@ public static boolean getPushdownTopnBrokerQueries(ConnectorSession session) return session.getProperty(PUSHDOWN_TOPN_BROKER_QUERIES, Boolean.class); } + public static boolean getPushdownProjectExpressions(ConnectorSession session) + { + return session.getProperty(PUSHDOWN_PROJECT_EXPRESSIONS, Boolean.class); + } + public static int getTopNLarge(ConnectorSession session) { return session.getProperty(TOPN_LARGE, Integer.class); @@ -184,6 +190,11 @@ public PinotSessionProperties(PinotConfig pinotConfig) "Push down order by to pinot broker for top queries", pinotConfig.isPushdownTopNBrokerQueries(), false), + booleanProperty( + PUSHDOWN_PROJECT_EXPRESSIONS, + "Push down expressions in projection to Pinot broker", + pinotConfig.isPushdownProjectExpressions(), + false), new PropertyMetadata<>( CONNECTION_TIMEOUT, "Connection Timeout to talk to Pinot servers", diff --git a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotAggregationProjectConverter.java b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotAggregationProjectConverter.java index 4041043518aa4..364faf3472dc0 100644 --- a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotAggregationProjectConverter.java +++ b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotAggregationProjectConverter.java @@ -18,6 +18,7 @@ import com.facebook.presto.pinot.PinotException; import com.facebook.presto.pinot.PinotSessionProperties; import com.facebook.presto.spi.ConnectorSession; +import com.facebook.presto.spi.function.FunctionHandle; import com.facebook.presto.spi.function.FunctionMetadata; import com.facebook.presto.spi.function.FunctionMetadataManager; import com.facebook.presto.spi.function.StandardFunctionResolution; @@ -29,7 +30,6 @@ import io.airlift.slice.Slice; import java.time.ZoneId; -import java.util.List; import java.util.Map; import java.util.Optional; @@ -43,12 +43,6 @@ public class PinotAggregationProjectConverter extends PinotProjectExpressionConverter { - // Pinot does not support modulus yet - private static final Map PRESTO_TO_PINOT_OPERATORS = ImmutableMap.of( - "-", "SUB", - "+", "ADD", - "*", "MULT", - "/", "DIV"); private static final String FROM_UNIXTIME = "from_unixtime"; private static final Map PRESTO_TO_PINOT_ARRAY_AGGREGATIONS = ImmutableMap.builder() @@ -58,8 +52,6 @@ public class PinotAggregationProjectConverter .put("array_sum", "arraySum") .build(); - private final FunctionMetadataManager functionMetadataManager; - private final ConnectorSession session; private final VariableReferenceExpression arrayVariableHint; public PinotAggregationProjectConverter(TypeManager typeManager, FunctionMetadataManager functionMetadataManager, StandardFunctionResolution standardFunctionResolution, ConnectorSession session) @@ -69,9 +61,7 @@ public PinotAggregationProjectConverter(TypeManager typeManager, FunctionMetadat public PinotAggregationProjectConverter(TypeManager typeManager, FunctionMetadataManager functionMetadataManager, StandardFunctionResolution standardFunctionResolution, ConnectorSession session, VariableReferenceExpression arrayVariableHint) { - super(typeManager, standardFunctionResolution); - this.functionMetadataManager = requireNonNull(functionMetadataManager, "functionMetadataManager is null"); - this.session = requireNonNull(session, "session is null"); + super(typeManager, functionMetadataManager, standardFunctionResolution, session); this.arrayVariableHint = arrayVariableHint; } @@ -80,11 +70,15 @@ public PinotExpression visitCall( CallExpression call, Map context) { - Optional basicCallHandlingResult = basicCallHandling(call, context); - if (basicCallHandlingResult.isPresent()) { - return basicCallHandlingResult.get(); + FunctionHandle functionHandle = call.getFunctionHandle(); + if (standardFunctionResolution.isCastFunction(functionHandle)) { + return handleCast(call, context); } - FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(call.getFunctionHandle()); + if (standardFunctionResolution.isNotFunction(functionHandle) || standardFunctionResolution.isBetweenFunction(functionHandle)) { + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Unsupported function in pinot aggregation: " + functionHandle); + } + + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(functionHandle); Optional operatorTypeOptional = functionMetadata.getOperatorType(); if (operatorTypeOptional.isPresent()) { OperatorType operatorType = operatorTypeOptional.get(); @@ -204,29 +198,6 @@ private PinotExpression handleDateTruncationViaDateTruncation( return derived("dateTrunc(" + inputColumn + "," + inputFormat + ", " + inputTimeZone + ", " + getStringFromConstant(intervalParameter) + ")"); } - private PinotExpression handleArithmeticExpression( - CallExpression expression, - OperatorType operatorType, - Map context) - { - List arguments = expression.getArguments(); - if (arguments.size() == 1) { - String prefix = operatorType == OperatorType.NEGATION ? "-" : ""; - return derived(prefix + arguments.get(0).accept(this, context).getDefinition()); - } - if (arguments.size() == 2) { - PinotExpression left = arguments.get(0).accept(this, context); - PinotExpression right = arguments.get(1).accept(this, context); - String prestoOperator = operatorType.getOperator(); - String pinotOperator = PRESTO_TO_PINOT_OPERATORS.get(prestoOperator); - if (pinotOperator == null) { - throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Unsupported binary expression " + prestoOperator); - } - return derived(format("%s(%s, %s)", pinotOperator, left.getDefinition(), right.getDefinition())); - } - throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), format("Don't know how to interpret %s as an arithmetic expression", expression)); - } - private PinotExpression handleFunction( CallExpression function, Map context) diff --git a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotFilterExpressionConverter.java b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotFilterExpressionConverter.java index 279c7e1b211e1..d07b0f26ad449 100644 --- a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotFilterExpressionConverter.java +++ b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotFilterExpressionConverter.java @@ -252,7 +252,7 @@ private PinotExpression handleNot(CallExpression not, Function context) @@ -313,16 +313,14 @@ public PinotExpression visitCall(CallExpression call, Function operatorTypeOptional = functionMetadata.getOperatorType(); - if (operatorTypeOptional.isPresent()) { - OperatorType operatorType = operatorTypeOptional.get(); - if (operatorType.isArithmeticOperator()) { - throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Arithmetic expressions are not supported in filter: " + call); - } - if (operatorType.isComparisonOperator()) { - return handleLogicalBinary(operatorType.getOperator(), call, context); - } + Optional operatorType = functionMetadata.getOperatorType(); + if (standardFunctionResolution.isComparisonFunction(functionHandle) && operatorType.isPresent()) { + return handleLogicalBinary(operatorType.get().getOperator(), call, context); } if ("contains".equals(functionMetadata.getName().getObjectName())) { return handleContains(call, context); @@ -330,7 +328,7 @@ public PinotExpression visitCall(CallExpression call, Function context) + { + if (expression instanceof ConstantExpression) { + return new PinotExpression( + getLiteralAsString((ConstantExpression) expression), + PinotQueryGeneratorContext.Origin.LITERAL + ).getDefinition(); + } + return expression.accept(this, context).getDefinition(); + } + @Override public PinotExpression visitSpecialForm(SpecialFormExpression specialForm, Function context) { switch (specialForm.getForm()) { case IF: case NULL_IF: - case SWITCH: - case WHEN: case COALESCE: case DEREFERENCE: case ROW_CONSTRUCTOR: case BIND: throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Pinot does not support the special form " + specialForm); + case SWITCH: + int numArguments = specialForm.getArguments().size(); + String searchExpression = getExpressionOrConstantString(specialForm.getArguments().get(0), context); + return derived(format( + "CASE %s %s ELSE %s END", + searchExpression, + specialForm.getArguments().subList(1, numArguments - 1).stream() + .map(argument -> argument.accept(this, context).getDefinition()) + .collect(Collectors.joining(" ")), + getExpressionOrConstantString(specialForm.getArguments().get(numArguments - 1), context))); + case WHEN: + return derived(format( + "%s %s THEN %s", + specialForm.getForm().toString(), + getExpressionOrConstantString(specialForm.getArguments().get(0), context), + getExpressionOrConstantString(specialForm.getArguments().get(1), context))); case IN: return handleIn(specialForm, true, context); case AND: diff --git a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotProjectExpressionConverter.java b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotProjectExpressionConverter.java index 770027514b7f9..7e36a21cb8a2a 100644 --- a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotProjectExpressionConverter.java +++ b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotProjectExpressionConverter.java @@ -13,12 +13,17 @@ */ package com.facebook.presto.pinot.query; +import com.facebook.presto.common.function.OperatorType; import com.facebook.presto.common.type.StandardTypes; import com.facebook.presto.common.type.Type; import com.facebook.presto.common.type.TypeManager; import com.facebook.presto.pinot.PinotException; +import com.facebook.presto.pinot.PinotSessionProperties; import com.facebook.presto.pinot.query.PinotQueryGeneratorContext.Selection; +import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.function.FunctionHandle; +import com.facebook.presto.spi.function.FunctionMetadata; +import com.facebook.presto.spi.function.FunctionMetadataManager; import com.facebook.presto.spi.function.StandardFunctionResolution; import com.facebook.presto.spi.relation.CallExpression; import com.facebook.presto.spi.relation.ConstantExpression; @@ -28,30 +33,48 @@ import com.facebook.presto.spi.relation.RowExpressionVisitor; import com.facebook.presto.spi.relation.SpecialFormExpression; import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import static com.facebook.presto.pinot.PinotErrorCode.PINOT_UNSUPPORTED_EXPRESSION; +import static com.facebook.presto.pinot.PinotPushdownUtils.getLiteralAsString; +import static com.facebook.presto.pinot.query.PinotExpression.derived; import static java.lang.String.format; import static java.util.Objects.requireNonNull; class PinotProjectExpressionConverter implements RowExpressionVisitor> { + private static final Set LOGICAL_BINARY_OPS_FILTER = ImmutableSet.of("=", "<", "<=", ">", ">=", "<>"); + // Pinot does not support modulus yet + private static final Map PRESTO_TO_PINOT_OPERATORS = ImmutableMap.of( + "-", "SUB", + "+", "ADD", + "*", "MULT", + "/", "DIV"); private static final Set TIME_EQUIVALENT_TYPES = ImmutableSet.of(StandardTypes.BIGINT, StandardTypes.INTEGER, StandardTypes.TINYINT, StandardTypes.SMALLINT); protected final TypeManager typeManager; + protected final FunctionMetadataManager functionMetadataManager; protected final StandardFunctionResolution standardFunctionResolution; + protected final ConnectorSession session; public PinotProjectExpressionConverter( TypeManager typeManager, - StandardFunctionResolution standardFunctionResolution) + FunctionMetadataManager functionMetadataManager, + StandardFunctionResolution standardFunctionResolution, + ConnectorSession session) { this.typeManager = requireNonNull(typeManager, "type manager"); + this.functionMetadataManager = requireNonNull(functionMetadataManager, "functionMetadataManager"); this.standardFunctionResolution = requireNonNull(standardFunctionResolution, "standardFunctionResolution is null"); + this.session = requireNonNull(session, "session is null"); } @Override @@ -78,8 +101,26 @@ protected boolean isImplicitCast(Type inputType, Type resultType) } return resultType.getTypeSignature().getBase().equals(StandardTypes.TIMESTAMP) && TIME_EQUIVALENT_TYPES.contains(inputType.getTypeSignature().getBase()); } + protected PinotExpression handleArithmeticExpression( + CallExpression expression, + OperatorType operatorType, + Map context) + { + List arguments = expression.getArguments(); + if (arguments.size() == 2) { + PinotExpression left = arguments.get(0).accept(this, context); + PinotExpression right = arguments.get(1).accept(this, context); + String prestoOperator = operatorType.getOperator(); + String pinotOperator = PRESTO_TO_PINOT_OPERATORS.get(prestoOperator); + if (pinotOperator == null) { + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Unsupported binary expression " + prestoOperator); + } + return derived(format("%s(%s, %s)", pinotOperator, left.getDefinition(), right.getDefinition())); + } + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), format("Don't know how to interpret %s as an arithmetic expression", expression)); + } - private PinotExpression handleCast( + protected PinotExpression handleCast( CallExpression cast, Map context) { @@ -95,16 +136,35 @@ private PinotExpression handleCast( throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), format("This type of CAST operator not supported. Received: %s", cast)); } - protected Optional basicCallHandling(CallExpression call, Map context) + // Borrowed from filter expr; doesn't handle date/time column and constant comparison + private PinotExpression handleLogicalBinary( + CallExpression call, + String operator, + Map context) { - FunctionHandle functionHandle = call.getFunctionHandle(); - if (standardFunctionResolution.isNotFunction(functionHandle) || standardFunctionResolution.isBetweenFunction(functionHandle)) { - throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Unsupported function in pinot aggregation: " + functionHandle); + if (!LOGICAL_BINARY_OPS_FILTER.contains(operator)) { + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), format("'%s' is not supported in filter", operator)); } - if (standardFunctionResolution.isCastFunction(functionHandle)) { - return Optional.of(handleCast(call, context)); + List arguments = call.getArguments(); + if (arguments.size() == 2) { + return derived(format( + "(%s %s %s)", + getExpressionOrConstantString(arguments.get(0), context), + operator, + getExpressionOrConstantString(arguments.get(1), context))); } - return Optional.empty(); + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), format("Unknown logical binary: '%s'", call)); + } + + protected String getExpressionOrConstantString( + RowExpression expression, + Map context) + { + if (expression instanceof ConstantExpression) { + return new PinotExpression(getLiteralAsString((ConstantExpression) expression), + PinotQueryGeneratorContext.Origin.LITERAL).getDefinition(); + } + return expression.accept(this, context).getDefinition(); } @Override @@ -112,7 +172,7 @@ public PinotExpression visitInputReference( InputReferenceExpression reference, Map context) { - throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Input reference not supported: " + reference); + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Pinot does not support struct dereferencing: " + reference); } @Override @@ -120,13 +180,66 @@ public PinotExpression visitSpecialForm( SpecialFormExpression specialForm, Map context) { - throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Special form not supported: " + specialForm); + if (!PinotSessionProperties.getPushdownProjectExpressions(session)) { + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Special form not supported: " + specialForm); + } + switch (specialForm.getForm()) { + // (SWITCH (WHEN ) (WHEN ) ) + // Presto generates "simple" CASE expressions from the "searched" form with true as pattern to match + case SWITCH: + int numArguments = specialForm.getArguments().size(); + String searchExpression = getExpressionOrConstantString(specialForm.getArguments().get(0), context); + + return derived(format( + "CASE %s %s ELSE %s END", + searchExpression, + specialForm.getArguments().subList(1, numArguments - 1).stream() + .map(argument -> argument.accept(this, context).getDefinition()) + .collect(Collectors.joining(" ")), + getExpressionOrConstantString(specialForm.getArguments().get(numArguments - 1), context))); + case WHEN: + return derived(format( + "%s %s THEN %s", + specialForm.getForm().toString(), + getExpressionOrConstantString(specialForm.getArguments().get(0), context), + getExpressionOrConstantString(specialForm.getArguments().get(1), context))); + case IF: + case NULL_IF: + case DEREFERENCE: + case ROW_CONSTRUCTOR: + case BIND: + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Pinot does not support the special form" + specialForm); + case IN: + case AND: + case OR: + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Special form not supported: " + specialForm); + default: + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Unexpected special form: " + specialForm); + } } @Override public PinotExpression visitCall(CallExpression call, Map context) { - return basicCallHandling(call, context).orElseThrow(() -> new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Call not supported: " + call)); + FunctionHandle functionHandle = call.getFunctionHandle(); + if (standardFunctionResolution.isCastFunction(functionHandle)) { + return handleCast(call, context); + } + if (!PinotSessionProperties.getPushdownProjectExpressions(session)) { + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Call not supported: " + call); + } + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(call.getFunctionHandle()); + Optional operatorType = functionMetadata.getOperatorType(); + if (standardFunctionResolution.isComparisonFunction(functionHandle) && operatorType.isPresent()) { + return handleLogicalBinary(call, operatorType.get().getOperator(), context); + } + if (standardFunctionResolution.isArithmeticFunction(functionHandle) && operatorType.isPresent()) { + return handleArithmeticExpression(call, operatorType.get(), context); + } + if (standardFunctionResolution.isNegateFunction(functionHandle)) { + return derived('-' + call.getArguments().get(0).accept(this, context).getDefinition()); + } + throw new PinotException(PINOT_UNSUPPORTED_EXPRESSION, Optional.empty(), "Call not supported: " + call); } @Override diff --git a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java index 4882b67706c04..e24b77fcab65c 100644 --- a/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java +++ b/presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java @@ -96,7 +96,6 @@ public class PinotQueryGenerator private final FunctionMetadataManager functionMetadataManager; private final StandardFunctionResolution standardFunctionResolution; private final PinotFilterExpressionConverter pinotFilterExpressionConverter; - private final PinotProjectExpressionConverter pinotProjectExpressionConverter; @Inject public PinotQueryGenerator( @@ -110,7 +109,6 @@ public PinotQueryGenerator( this.functionMetadataManager = requireNonNull(functionMetadataManager, "function metadata manager is null"); this.standardFunctionResolution = requireNonNull(standardFunctionResolution, "standardFunctionResolution is null"); this.pinotFilterExpressionConverter = new PinotFilterExpressionConverter(this.typeManager, this.functionMetadataManager, standardFunctionResolution); - this.pinotProjectExpressionConverter = new PinotProjectExpressionConverter(typeManager, standardFunctionResolution); } public static class PinotQueryGeneratorResult @@ -297,6 +295,7 @@ public PinotQueryGeneratorContext visitProject(ProjectNode node, PinotQueryGener requireNonNull(context, "context is null"); Map newSelections = new HashMap<>(); LinkedHashSet newOutputs = new LinkedHashSet<>(); + PinotProjectExpressionConverter pinotProjectExpressionConverter = new PinotProjectExpressionConverter(typeManager, functionMetadataManager, standardFunctionResolution, session); node.getOutputVariables().forEach(variable -> { RowExpression expression = node.getAssignments().get(variable); PinotExpression pinotExpression = expression.accept( @@ -577,7 +576,7 @@ public PinotQueryGeneratorContext visitAggregation(AggregationNode node, PinotQu @Override public PinotQueryGeneratorContext visitLimit(LimitNode node, PinotQueryGeneratorContext context) { - checkSupported(!node.isPartial(), String.format("pinot query generator cannot handle partial limit")); + checkSupported(!node.isPartial(), "pinot query generator cannot handle partial limit"); checkSupported(!forbidBrokerQueries, "Cannot push limit in segment mode"); context = node.getSource().accept(this, context); requireNonNull(context, "context is null"); diff --git a/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/TestPinotConfig.java b/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/TestPinotConfig.java index 3b76421a789ad..38054b6f07252 100644 --- a/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/TestPinotConfig.java +++ b/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/TestPinotConfig.java @@ -68,6 +68,7 @@ public void testDefaults() .setFetchRetryCount(2) .setMarkDataFetchExceptionsAsRetriable(true) .setPushdownTopNBrokerQueries(true) + .setPushdownProjectExpressions(true) .setIgnoreEmptyResponses(false) .setUseDateTrunc(false) .setForbidSegmentQueries(false) @@ -114,6 +115,7 @@ public void testExplicitPropertyMappings() .put("pinot.use-date-trunc", "true") .put("pinot.limit-large-for-segment", "100") .put("pinot.pushdown-topn-broker-queries", "false") + .put("pinot.pushdown-project-expressions", "false") .put("pinot.forbid-segment-queries", "true") .put("pinot.use-streaming-for-segment-queries", "true") .put("pinot.streaming-server-grpc-max-inbound-message-bytes", "65536") @@ -161,6 +163,7 @@ public void testExplicitPropertyMappings() .setNonAggregateLimitForBrokerQueries(10) .setLimitLargeForSegment(100) .setPushdownTopNBrokerQueries(false) + .setPushdownProjectExpressions(false) .setForbidSegmentQueries(true) .setUseStreamingForSegmentQueries(true) .setStreamingServerGrpcMaxInboundMessageBytes(65536) diff --git a/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotExpressionConverters.java b/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotExpressionConverters.java index 78509a3b8a5de..ee787d0f73aee 100644 --- a/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotExpressionConverters.java +++ b/presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotExpressionConverters.java @@ -45,13 +45,20 @@ public void testProjectExpressionConverterSql() public void testProjectExpressionConverter(SessionHolder sessionHolder) { testProject("secondssinceepoch", "\"secondsSinceEpoch\"", sessionHolder); - // functions + testProject("secondssinceepoch > 1559978258", "(\"secondsSinceEpoch\" > 1559978258)", sessionHolder); + testProject("secondssinceepoch != 0", "(\"secondsSinceEpoch\" <> 0)", sessionHolder); + testProject("secondssinceepoch <> 0", "(\"secondsSinceEpoch\" <> 0)", sessionHolder); + testProject( + "CASE WHEN secondssinceepoch > 0 THEN distinctCountDim ELSE fare END", + "CASE true WHEN (\"secondsSinceEpoch\" > 0) THEN \"distinctCountDim\" ELSE \"fare\" END", + sessionHolder); testAggregationProject( "date_trunc('hour', from_unixtime(secondssinceepoch))", "dateTimeConvert(\"secondsSinceEpoch\", '1:SECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '1:HOURS')", sessionHolder); // arithmetic + testProject("-secondssinceepoch", "-\"secondsSinceEpoch\"", sessionHolder); testAggregationProject("regionid + 1", "ADD(\"regionId\", 1)", sessionHolder); testAggregationProject("regionid - 1", "SUB(\"regionId\", 1)", sessionHolder); testAggregationProject("1 * regionid", "MULT(1, \"regionId\")", sessionHolder); @@ -68,13 +75,21 @@ public void testProjectExpressionConverter(SessionHolder sessionHolder) "date_trunc('hour', from_unixtime(secondssinceepoch + 2))", "dateTimeConvert(ADD(\"secondsSinceEpoch\", 2), '1:SECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '1:HOURS')", sessionHolder); + testAggregationProject( + "CASE WHEN false THEN distinctCountDim ELSE fare END", + "CASE true WHEN false THEN \"distinctCountDim\" ELSE \"fare\" END", + sessionHolder); } private void testProject(String sqlExpression, String expectedPinotExpression, SessionHolder sessionHolder) { RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); String actualPinotExpression = pushDownExpression.accept( - new PinotProjectExpressionConverter(functionAndTypeManager, standardFunctionResolution), + new PinotProjectExpressionConverter( + functionAndTypeManager, + functionAndTypeManager, + standardFunctionResolution, + sessionHolder.getConnectorSession()), testInput).getDefinition(); assertEquals(actualPinotExpression, expectedPinotExpression); } @@ -159,6 +174,8 @@ public void testFilterExpressionConverter(SessionHolder sessionHolder) // combinations testFilter("totalfare between 20 and 30 AND regionid > 20 OR city = 'Campbell'", "((((\"fare\" + \"trip\") BETWEEN 20 AND 30) AND (\"regionId\" > 20)) OR (\"city\" = 'Campbell'))", sessionHolder); + testFilter("CASE WHEN regionid IS NOT NULL THEN regionid WHEN city IS NOT NULL THEN 300 ELSE secondssinceepoch END", + "CASE true WHEN (\"regionId\" IS NOT NULL) THEN \"regionId\" WHEN (\"city\" IS NOT NULL) THEN 300 ELSE \"secondsSinceEpoch\" END", sessionHolder); testFilter("secondssinceepoch > 1559978258", "(\"secondsSinceEpoch\" > 1559978258)", sessionHolder); testFilter("DATE '2019-11-15'", "18215", sessionHolder);