diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpFilterToKqlConverter.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpFilterToKqlConverter.java index 7e6fae13fdb8f..4fdecf943de90 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpFilterToKqlConverter.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpFilterToKqlConverter.java @@ -95,38 +95,35 @@ * */ public class ClpFilterToKqlConverter - implements RowExpressionVisitor + implements RowExpressionVisitor> { private static final Set LOGICAL_BINARY_OPS_FILTER = ImmutableSet.of(EQUAL, NOT_EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL); private final StandardFunctionResolution standardFunctionResolution; private final FunctionMetadataManager functionMetadataManager; - private final Map assignments; private final Set metadataFilterColumns; public ClpFilterToKqlConverter( StandardFunctionResolution standardFunctionResolution, FunctionMetadataManager functionMetadataManager, - Map assignments, Set metadataFilterColumns) { this.standardFunctionResolution = requireNonNull(standardFunctionResolution, "standardFunctionResolution is null"); this.functionMetadataManager = requireNonNull(functionMetadataManager, "function metadata manager is null"); - this.assignments = requireNonNull(assignments, "assignments is null"); this.metadataFilterColumns = requireNonNull(metadataFilterColumns, "metadataFilterColumns is null"); } @Override - public ClpExpression visitCall(CallExpression node, Void context) + public ClpExpression visitCall(CallExpression node, Map context) { FunctionHandle functionHandle = node.getFunctionHandle(); if (standardFunctionResolution.isNotFunction(functionHandle)) { - return handleNot(node); + return handleNot(node, context); } if (standardFunctionResolution.isLikeFunction(functionHandle)) { - return handleLike(node); + return handleLike(node, context); } FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(node.getFunctionHandle()); @@ -134,10 +131,10 @@ public ClpExpression visitCall(CallExpression node, Void context) if (operatorTypeOptional.isPresent()) { OperatorType operatorType = operatorTypeOptional.get(); if (operatorType.isComparisonOperator() && operatorType != IS_DISTINCT_FROM) { - return handleLogicalBinary(operatorType, node); + return handleLogicalBinary(operatorType, node, context); } if (BETWEEN == operatorType) { - return handleBetween(node); + return handleBetween(node, context); } } @@ -145,38 +142,38 @@ public ClpExpression visitCall(CallExpression node, Void context) } @Override - public ClpExpression visitConstant(ConstantExpression node, Void context) + public ClpExpression visitConstant(ConstantExpression node, Map context) { return new ClpExpression(getLiteralString(node)); } @Override - public ClpExpression visitVariableReference(VariableReferenceExpression node, Void context) + public ClpExpression visitVariableReference(VariableReferenceExpression node, Map context) { - return new ClpExpression(getVariableName(node)); + return new ClpExpression(getVariableName(node, context)); } @Override - public ClpExpression visitSpecialForm(SpecialFormExpression node, Void context) + public ClpExpression visitSpecialForm(SpecialFormExpression node, Map context) { switch (node.getForm()) { case AND: - return handleAnd(node); + return handleAnd(node, context); case OR: - return handleOr(node); + return handleOr(node, context); case IN: - return handleIn(node); + return handleIn(node, context); case IS_NULL: - return handleIsNull(node); + return handleIsNull(node, context); case DEREFERENCE: - return handleDereference(node); + return handleDereference(node, context); default: return new ClpExpression(node); } } @Override - public ClpExpression visitExpression(RowExpression node, Void context) + public ClpExpression visitExpression(RowExpression node, Map context) { // For all other expressions, return the original expression return new ClpExpression(node); @@ -200,11 +197,12 @@ private String getLiteralString(ConstantExpression literal) * Retrieves the original column name from a variable reference. * * @param variable the variable reference expression + * @param context a mapping from variable references to column handles used for pushdown * @return the original column name as a string */ - private String getVariableName(VariableReferenceExpression variable) + private String getVariableName(VariableReferenceExpression variable, Map context) { - return ((ClpColumnHandle) assignments.get(variable)).getOriginalColumnName(); + return ((ClpColumnHandle) context.get(variable)).getOriginalColumnName(); } /** @@ -219,11 +217,12 @@ private String getVariableName(VariableReferenceExpression variable) *

* Example: col1 BETWEEN 0 AND 5 → col1 >= 0 AND col1 <= 5 * - * @param node the {@code BETWEEN} call expression + * @param node the BETWEEN call expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be translated */ - private ClpExpression handleBetween(CallExpression node) + private ClpExpression handleBetween(CallExpression node, Map context) { List arguments = node.getArguments(); if (arguments.size() != 3) { @@ -243,7 +242,7 @@ private ClpExpression handleBetween(CallExpression node) || !isClpCompatibleNumericType(third.getType())) { return new ClpExpression(node); } - Optional variableOpt = first.accept(this, null).getPushDownExpression(); + Optional variableOpt = first.accept(this, context).getPushDownExpression(); if (!variableOpt.isPresent()) { return new ClpExpression(node); } @@ -263,10 +262,11 @@ private ClpExpression handleBetween(CallExpression node) * Example: NOT (col1 = 5) → NOT col1: 5 * * @param node the NOT call expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be translated */ - private ClpExpression handleNot(CallExpression node) + private ClpExpression handleNot(CallExpression node, Map context) { if (node.getArguments().size() != 1) { throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, @@ -274,7 +274,7 @@ private ClpExpression handleNot(CallExpression node) } RowExpression input = node.getArguments().get(0); - ClpExpression expression = input.accept(this, null); + ClpExpression expression = input.accept(this, context); if (expression.getRemainingExpression().isPresent() || !expression.getPushDownExpression().isPresent()) { return new ClpExpression(node); } @@ -297,15 +297,16 @@ private ClpExpression handleNot(CallExpression node) * Example: col1 LIKE 'a_bc%' → col1: "a?bc*" * * @param node the LIKE call expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be translated */ - private ClpExpression handleLike(CallExpression node) + private ClpExpression handleLike(CallExpression node, Map context) { if (node.getArguments().size() != 2) { throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "LIKE operator must have exactly two arguments. Received: " + node); } - ClpExpression variable = node.getArguments().get(0).accept(this, null); + ClpExpression variable = node.getArguments().get(0).accept(this, context); if (!variable.getPushDownExpression().isPresent()) { return new ClpExpression(node); } @@ -346,10 +347,11 @@ else if (argument instanceof CallExpression) { * * @param operator the binary operator (e.g., EQUAL, NOT_EQUAL) * @param node the call expression representing the binary operation + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be translated */ - private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression node) + private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression node, Map context) { if (node.getArguments().size() != 2) { throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, @@ -358,21 +360,21 @@ private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression RowExpression left = node.getArguments().get(0); RowExpression right = node.getArguments().get(1); - ClpExpression maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right); - if (maybeLeftSubstring.getPushDownExpression().isPresent()) { - return maybeLeftSubstring; + Optional maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right, context); + if (maybeLeftSubstring.isPresent()) { + return maybeLeftSubstring.get(); } - ClpExpression maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left); - if (maybeRightSubstring.getPushDownExpression().isPresent()) { - return maybeRightSubstring; + Optional maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left, context); + if (maybeRightSubstring.isPresent()) { + return maybeRightSubstring.get(); } - ClpExpression leftExpression = left.accept(this, null); - ClpExpression rightExpression = right.accept(this, null); - Optional leftDefinition = leftExpression.getPushDownExpression(); - Optional rightDefinition = rightExpression.getPushDownExpression(); - if (!leftDefinition.isPresent() || !rightDefinition.isPresent()) { + ClpExpression leftExpression = left.accept(this, context); + ClpExpression rightExpression = right.accept(this, context); + Optional leftPushDownExpression = leftExpression.getPushDownExpression(); + Optional rightPushDownExpression = rightExpression.getPushDownExpression(); + if (!leftPushDownExpression.isPresent() || !rightPushDownExpression.isPresent()) { return new ClpExpression(node); } @@ -384,8 +386,8 @@ private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression if (rightIsConstant) { return buildClpExpression( - leftDefinition.get(), // variable - rightDefinition.get(), // literal + leftPushDownExpression.get(), // variable + rightPushDownExpression.get(), // literal operator, rightType, node); @@ -393,8 +395,8 @@ private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression else if (leftIsConstant) { OperatorType newOperator = flip(operator); return buildClpExpression( - rightDefinition.get(), // variable - leftDefinition.get(), // literal + rightPushDownExpression.get(), // variable + leftPushDownExpression.get(), // literal newOperator, leftType, node); @@ -471,26 +473,27 @@ else if (LOGICAL_BINARY_OPS_FILTER.contains(operator) && !(literalType instanceo * @param operator the comparison operator (should be EQUAL) * @param possibleSubstring the left or right expression, possibly a SUBSTR call * @param possibleLiteral the opposite expression, possibly a string constant - * @return a ClpExpression containing either the equivalent KQL query, or nothing if it couldn't - * be translated + * @param context a mapping from variable references to column handles used for pushdown + * @return an Optional containing a ClpExpression with the equivalent KQL query */ - private ClpExpression tryInterpretSubstringEquality( + private Optional tryInterpretSubstringEquality( OperatorType operator, RowExpression possibleSubstring, - RowExpression possibleLiteral) + RowExpression possibleLiteral, + Map context) { if (!operator.equals(EQUAL)) { - return new ClpExpression(); + return Optional.empty(); } if (!(possibleSubstring instanceof CallExpression) || !(possibleLiteral instanceof ConstantExpression)) { - return new ClpExpression(); + return Optional.empty(); } - Optional maybeSubstringCall = parseSubstringCall((CallExpression) possibleSubstring); + Optional maybeSubstringCall = parseSubstringCall((CallExpression) possibleSubstring, context); if (!maybeSubstringCall.isPresent()) { - return new ClpExpression(); + return Optional.empty(); } String targetString = getLiteralString((ConstantExpression) possibleLiteral); @@ -501,9 +504,10 @@ private ClpExpression tryInterpretSubstringEquality( * Parses a SUBSTR(x, start [, length]) call into a SubstrInfo object if valid. * * @param callExpression the call expression to inspect + * @param context a mapping from variable references to column handles used for pushdown * @return an Optional containing SubstrInfo if the expression is a valid SUBSTR call */ - private Optional parseSubstringCall(CallExpression callExpression) + private Optional parseSubstringCall(CallExpression callExpression, Map context) { FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(callExpression.getFunctionHandle()); String functionName = functionMetadata.getName().getObjectName(); @@ -516,7 +520,7 @@ private Optional parseSubstringCall(CallExpression callExpression) return Optional.empty(); } - ClpExpression variable = callExpression.getArguments().get(0).accept(this, null); + ClpExpression variable = callExpression.getArguments().get(0).accept(this, context); if (!variable.getPushDownExpression().isPresent()) { return Optional.empty(); } @@ -545,10 +549,9 @@ private Optional parseSubstringCall(CallExpression callExpression) * * @param info parsed SUBSTR call info * @param targetString the literal string being compared to - * @return a ClpExpression containing either the equivalent KQL query, or nothing if it couldn't - * be translated + * @return an Optional containing either a ClpExpression with the equivalent KQL query */ - private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetString) + private Optional interpretSubstringEquality(SubstrInfo info, String targetString) { if (info.lengthExpression != null) { Optional maybeStart = parseIntValue(info.startExpression); @@ -564,7 +567,7 @@ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetS result.append("?"); } result.append(targetString).append("*\""); - return new ClpExpression(result.toString()); + return Optional.of(new ClpExpression(result.toString())); } } } @@ -579,15 +582,15 @@ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetS result.append("?"); } result.append(targetString).append("\""); - return new ClpExpression(result.toString()); + return Optional.of(new ClpExpression(result.toString())); } if (start == -targetString.length()) { - return new ClpExpression(format("%s: \"*%s\"", info.variableName, targetString)); + return Optional.of(new ClpExpression(format("%s: \"*%s\"", info.variableName, targetString))); } } } - return new ClpExpression(); + return Optional.empty(); } /** @@ -655,9 +658,10 @@ private Optional parseLengthLiteral(RowExpression lengthExpression, Str * Example: col1 = 5 AND col2 = 'abc' → (col1: 5 AND col2: "abc") * * @param node the AND special form expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing the KQL query and any remaining sub-expressions */ - private ClpExpression handleAnd(SpecialFormExpression node) + private ClpExpression handleAnd(SpecialFormExpression node, Map context) { StringBuilder metadataQueryBuilder = new StringBuilder(); metadataQueryBuilder.append("("); @@ -667,7 +671,7 @@ private ClpExpression handleAnd(SpecialFormExpression node) boolean hasMetadataSql = false; boolean hasPushDownExpression = false; for (RowExpression argument : node.getArguments()) { - ClpExpression expression = argument.accept(this, null); + ClpExpression expression = argument.accept(this, context); if (expression.getPushDownExpression().isPresent()) { hasPushDownExpression = true; queryBuilder.append(expression.getPushDownExpression().get()); @@ -713,20 +717,25 @@ else if (!remainingExpressions.isEmpty()) { * Example: col1 = 5 OR col1 = 10 → (col1: 5 OR col1: 10) * * @param node the OR special form expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be fully translated */ - private ClpExpression handleOr(SpecialFormExpression node) + private ClpExpression handleOr(SpecialFormExpression node, Map context) { StringBuilder metadataQueryBuilder = new StringBuilder(); metadataQueryBuilder.append("("); StringBuilder queryBuilder = new StringBuilder(); queryBuilder.append("("); + boolean allPushedDown = true; boolean hasAllMetadataSql = true; for (RowExpression argument : node.getArguments()) { - ClpExpression expression = argument.accept(this, null); + ClpExpression expression = argument.accept(this, context); + // Note: It is possible in the future that an expression cannot be pushed down as a KQL query, but can be + // pushed down as a metadata SQL query. if (expression.getRemainingExpression().isPresent() || !expression.getPushDownExpression().isPresent()) { - return new ClpExpression(node); + allPushedDown = false; + continue; } queryBuilder.append(expression.getPushDownExpression().get()); queryBuilder.append(" OR "); @@ -738,10 +747,13 @@ private ClpExpression handleOr(SpecialFormExpression node) hasAllMetadataSql = false; } } - // Remove the last " OR " from the query - return new ClpExpression( - queryBuilder.substring(0, queryBuilder.length() - 4) + ")", - hasAllMetadataSql ? metadataQueryBuilder.substring(0, metadataQueryBuilder.length() - 4) + ")" : null); + if (allPushedDown) { + // Remove the last " OR " from the query + return new ClpExpression( + queryBuilder.substring(0, queryBuilder.length() - 4) + ")", + hasAllMetadataSql ? metadataQueryBuilder.substring(0, metadataQueryBuilder.length() - 4) + ")" : null); + } + return new ClpExpression(node); } /** @@ -750,12 +762,13 @@ private ClpExpression handleOr(SpecialFormExpression node) * Example: col1 IN (1, 2, 3) → (col1: 1 OR col1: 2 OR col1: 3) * * @param node the IN special form expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be translated */ - private ClpExpression handleIn(SpecialFormExpression node) + private ClpExpression handleIn(SpecialFormExpression node, Map context) { - ClpExpression variable = node.getArguments().get(0).accept(this, null); + ClpExpression variable = node.getArguments().get(0).accept(this, context); if (!variable.getPushDownExpression().isPresent()) { return new ClpExpression(node); } @@ -788,17 +801,18 @@ private ClpExpression handleIn(SpecialFormExpression node) * Example: col1 IS NULL → NOT col1: * * * @param node the IS_NULL special form expression + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the equivalent KQL query, or the original * expression if it couldn't be translated */ - private ClpExpression handleIsNull(SpecialFormExpression node) + private ClpExpression handleIsNull(SpecialFormExpression node, Map context) { if (node.getArguments().size() != 1) { throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "IS NULL operator must have exactly one argument. Received: " + node); } - ClpExpression expression = node.getArguments().get(0).accept(this, null); + ClpExpression expression = node.getArguments().get(0).accept(this, context); if (!expression.getPushDownExpression().isPresent()) { return new ClpExpression(node); } @@ -816,13 +830,14 @@ private ClpExpression handleIsNull(SpecialFormExpression node) * * @param expression the dereference expression ({@link SpecialFormExpression} or * {@link VariableReferenceExpression}) + * @param context a mapping from variable references to column handles used for pushdown * @return a ClpExpression containing either the dot-separated field name, or the original * expression if it couldn't be translated */ - private ClpExpression handleDereference(RowExpression expression) + private ClpExpression handleDereference(RowExpression expression, Map context) { if (expression instanceof VariableReferenceExpression) { - return expression.accept(this, null); + return expression.accept(this, context); } if (!(expression instanceof SpecialFormExpression)) { @@ -862,7 +877,7 @@ private ClpExpression handleDereference(RowExpression expression) RowType.Field field = rowType.getFields().get(fieldIndex); String fieldName = field.getName().orElse("field" + fieldIndex); - ClpExpression baseString = handleDereference(base); + ClpExpression baseString = handleDereference(base, context); if (!baseString.getPushDownExpression().isPresent()) { return new ClpExpression(expression); } diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java index 157af459cb80e..7c907fc1d3191 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java @@ -29,6 +29,7 @@ import com.facebook.presto.spi.relation.RowExpression; import com.facebook.presto.spi.relation.VariableReferenceExpression; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -75,7 +76,7 @@ public PlanNode visitFilter(FilterNode node, RewriteContext context) } TableScanNode tableScanNode = (TableScanNode) node.getSource(); - Map assignments = tableScanNode.getAssignments(); + Map assignments = new HashMap<>(tableScanNode.getAssignments()); TableHandle tableHandle = tableScanNode.getTable(); ClpTableHandle clpTableHandle = (ClpTableHandle) tableHandle.getConnectorHandle(); String scope = CONNECTOR_NAME + "." + clpTableHandle.getSchemaTableName().toString(); @@ -83,8 +84,8 @@ public PlanNode visitFilter(FilterNode node, RewriteContext context) new ClpFilterToKqlConverter( functionResolution, functionManager, - assignments, - metadataFilterProvider.getColumnNames(scope)), null); + metadataFilterProvider.getColumnNames(scope)), + assignments); Optional kqlQuery = clpExpression.getPushDownExpression(); Optional metadataSqlQuery = clpExpression.getMetadataSqlQuery(); Optional remainingPredicate = clpExpression.getRemainingExpression(); diff --git a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java index 5007a0ff10bd3..9e9e9db13a665 100644 --- a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java @@ -13,10 +13,14 @@ */ package com.facebook.presto.plugin.clp; +import com.facebook.presto.spi.ColumnHandle; import com.facebook.presto.spi.relation.RowExpression; +import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.google.common.collect.ImmutableSet; import org.testng.annotations.Test; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -288,20 +292,16 @@ private void testPushDown(SessionHolder sessionHolder, String sql, String expect private ClpExpression tryPushDown(String sqlExpression, SessionHolder sessionHolder, Set metadataFilterColumns) { RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); + Map assignments = new HashMap<>(variableToColumnHandleMap); return pushDownExpression.accept( new ClpFilterToKqlConverter( standardFunctionResolution, functionAndTypeManager, - variableToColumnHandleMap, metadataFilterColumns), - null); + assignments); } - private void testFilter( - ClpExpression clpExpression, - String expectedKqlExpression, - String expectedRemainingExpression, - SessionHolder sessionHolder) + private void testFilter(ClpExpression clpExpression, String expectedKqlExpression, String expectedRemainingExpression, SessionHolder sessionHolder) { Optional kqlExpression = clpExpression.getPushDownExpression(); Optional remainingExpression = clpExpression.getRemainingExpression();