From e94e218ae5149cd1b435df829aa4dfd1b1f29dac Mon Sep 17 00:00:00 2001 From: wraymo Date: Thu, 19 Jun 2025 18:19:58 -0400 Subject: [PATCH 1/7] add plan optimizer for clp connector --- .../presto/plugin/clp/ClpExpression.java | 70 ++ .../plugin/clp/ClpFilterToKqlConverter.java | 679 ++++++++++++++++++ .../presto/plugin/clp/ClpPlanOptimizer.java | 115 +++ .../plugin/clp/ClpPlanOptimizerProvider.java | 51 ++ .../presto/plugin/clp/TestClpFilterToKql.java | 215 ++++++ .../presto/plugin/clp/TestClpQueryBase.java | 145 ++++ 6 files changed, 1275 insertions(+) create mode 100644 presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java create mode 100644 presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpFilterToKqlConverter.java create mode 100644 presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java create mode 100644 presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java create mode 100644 presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java create mode 100644 presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java new file mode 100644 index 0000000000000..6b9fabbcfecad --- /dev/null +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java @@ -0,0 +1,70 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.plugin.clp; + +import com.facebook.presto.spi.relation.RowExpression; + +import java.util.Optional; + +/** + * Represents the result of converting a Presto RowExpression into a CLP-compatible KQL query. + * There are three possible cases: + * 1. The entire RowExpression is convertible to KQL: `definition` is set, `remainingExpression` is empty. + * 2. Part of the RowExpression is convertible: the KQL part is stored in `definition`, + * and the remaining untranslatable part is stored in `remainingExpression`. + * 3. None of the expression is convertible: the full RowExpression is stored in `remainingExpression`, + * and `definition` is empty. + */ +public class ClpExpression +{ + // Optional KQL query string representing the fully or partially translatable part of the expression. + private final Optional definition; + + // The remaining (non-translatable) portion of the RowExpression, if any. + private final Optional remainingExpression; + + public ClpExpression(String definition, RowExpression remainingExpression) + { + this.definition = Optional.ofNullable(definition); + this.remainingExpression = Optional.ofNullable(remainingExpression); + } + + // Creates an empty ClpExpression (no KQL definition, no remaining expression). + public ClpExpression() + { + this (null, null); + } + + // Creates a ClpExpression from a fully translatable KQL string. + public ClpExpression(String definition) + { + this(definition, null); + } + + // Creates a ClpExpression from a non-translatable RowExpression. + public ClpExpression(RowExpression remainingExpression) + { + this(null, remainingExpression); + } + + public Optional getDefinition() + { + return definition; + } + + public Optional getRemainingExpression() + { + return remainingExpression; + } +} 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 new file mode 100644 index 0000000000000..b9c65dfd2ce22 --- /dev/null +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpFilterToKqlConverter.java @@ -0,0 +1,679 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.plugin.clp; + +import com.facebook.presto.common.function.OperatorType; +import com.facebook.presto.common.type.RowType; +import com.facebook.presto.common.type.Type; +import com.facebook.presto.common.type.VarcharType; +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.PrestoException; +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; +import com.facebook.presto.spi.relation.RowExpression; +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.ImmutableSet; +import io.airlift.slice.Slice; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static com.facebook.presto.common.function.OperatorType.EQUAL; +import static com.facebook.presto.common.function.OperatorType.GREATER_THAN; +import static com.facebook.presto.common.function.OperatorType.GREATER_THAN_OR_EQUAL; +import static com.facebook.presto.common.function.OperatorType.LESS_THAN; +import static com.facebook.presto.common.function.OperatorType.LESS_THAN_OR_EQUAL; +import static com.facebook.presto.common.function.OperatorType.NOT_EQUAL; +import static com.facebook.presto.common.type.BooleanType.BOOLEAN; +import static com.facebook.presto.plugin.clp.ClpErrorCode.CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION; +import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.AND; +import static java.util.Objects.requireNonNull; + +/** + * ClpFilterToKqlConverter translates Presto RowExpressions into KQL (Kibana Query Language) filters + * used as CLP queries. This is used primarily for pushing down supported filters to the CLP engine. + * This class implements the RowExpressionVisitor interface and recursively walks Presto filter expressions, + * attempting to convert supported expressions (e.g., comparisons, logical AND/OR, LIKE, IN, IS NULL, + * and SUBSTR-based expressions) into corresponding KQL filter strings. Any part of the expression that + * cannot be translated is preserved as a "remaining expression" for potential fallback processing. + * Supported translations include: + * - Variable-to-literal comparisons (e.g., =, !=, <, >, <=, >=) + * - String pattern matches using LIKE + * - Membership checks using IN + * - NULL checks via IS NULL + * - Substring comparisons (e.g., SUBSTR(x, start, len) = "val") mapped to wildcard KQL queries + * - Dereferencing fields from row-typed variables + * - Logical operators AND, OR, and NOT + */ +public class ClpFilterToKqlConverter + 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; + + public ClpFilterToKqlConverter(StandardFunctionResolution standardFunctionResolution, + FunctionMetadataManager functionMetadataManager, + Map assignments) + { + this.standardFunctionResolution = requireNonNull(standardFunctionResolution, "standardFunctionResolution is null"); + this.functionMetadataManager = requireNonNull(functionMetadataManager, "function metadata manager is null"); + this.assignments = requireNonNull(assignments, "assignments is null"); + } + + @Override + public ClpExpression visitCall(CallExpression node, Void context) + { + FunctionHandle functionHandle = node.getFunctionHandle(); + if (standardFunctionResolution.isNotFunction(functionHandle)) { + return handleNot(node); + } + + if (standardFunctionResolution.isLikeFunction(functionHandle)) { + return handleLike(node); + } + + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(node.getFunctionHandle()); + Optional operatorTypeOptional = functionMetadata.getOperatorType(); + if (operatorTypeOptional.isPresent()) { + OperatorType operatorType = operatorTypeOptional.get(); + if (operatorType.isComparisonOperator() && operatorType != OperatorType.IS_DISTINCT_FROM) { + return handleLogicalBinary(operatorType, node); + } + } + + return new ClpExpression(node); + } + + @Override + public ClpExpression visitConstant(ConstantExpression node, Void context) + { + return new ClpExpression(getLiteralString(node)); + } + + @Override + public ClpExpression visitVariableReference(VariableReferenceExpression node, Void context) + { + return new ClpExpression(getVariableName(node)); + } + + @Override + public ClpExpression visitSpecialForm(SpecialFormExpression node, Void context) + { + switch (node.getForm()) { + case AND: + return handleAnd(node); + case OR: + return handleOr(node); + case IN: + return handleIn(node); + case IS_NULL: + return handleIsNull(node); + case DEREFERENCE: + return handleDereference(node); + default: + return new ClpExpression(node); + } + } + + // For all other expressions, return the original expression + @Override + public ClpExpression visitExpression(RowExpression node, Void context) + { + return new ClpExpression(node); + } + + private static String getLiteralString(ConstantExpression literal) + { + if (literal.getValue() instanceof Slice) { + return ((Slice) literal.getValue()).toStringUtf8(); + } + return literal.toString(); + } + + private String getVariableName(VariableReferenceExpression variable) + { + return ((ClpColumnHandle) assignments.get(variable)).getOriginalColumnName(); + } + + /** + * Handles the logical NOT expression. + * Example: + * Input: NOT (col1 = 5) + * Output: NOT col1: 5 + */ + private ClpExpression handleNot(CallExpression node) + { + if (node.getArguments().size() != 1) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, + "NOT operator must have exactly one argument. Received: " + node); + } + + RowExpression input = node.getArguments().get(0); + ClpExpression expression = input.accept(this, null); + if (expression.getRemainingExpression().isPresent() || !expression.getDefinition().isPresent()) { + return new ClpExpression(node); + } + return new ClpExpression("NOT " + expression.getDefinition().get()); + } + + /** + * Handles the logical AND expression. + * Combines all definable child expressions into a single KQL query joined by AND. + * Any unsupported children are collected into remaining expressions. + * Example: + * Input: col1 = 5 AND col2 = 'abc' + * Output: (col1: 5 AND col2: "abc") + */ + private ClpExpression handleAnd(SpecialFormExpression node) + { + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append("("); + ArrayList remainingExpressions = new ArrayList<>(); + boolean hasDefinition = false; + for (RowExpression argument : node.getArguments()) { + ClpExpression expression = argument.accept(this, null); + if (expression.getDefinition().isPresent()) { + hasDefinition = true; + queryBuilder.append(expression.getDefinition().get()); + queryBuilder.append(" AND "); + } + if (expression.getRemainingExpression().isPresent()) { + remainingExpressions.add(expression.getRemainingExpression().get()); + } + } + if (!hasDefinition) { + return new ClpExpression(node); + } + else if (!remainingExpressions.isEmpty()) { + if (remainingExpressions.size() == 1) { + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 5) + ")", remainingExpressions.get(0)); + } + else { + return new ClpExpression( + queryBuilder.substring(0, queryBuilder.length() - 5) + ")", + new SpecialFormExpression(node.getSourceLocation(), AND, BOOLEAN, remainingExpressions)); + } + } + // Remove the last " AND " from the query + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 5) + ")"); + } + + /** + * Handles the logical OR expression. + * Combines all fully convertible child expressions into a single CLP query joined by OR. + * Returns the original node if any child is unsupported. + * Example: + * Input: col1 = 5 OR col1 = 10 + * Output: (col1: 5 OR col1: 10) + */ + private ClpExpression handleOr(SpecialFormExpression node) + { + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append("("); + for (RowExpression argument : node.getArguments()) { + ClpExpression expression = argument.accept(this, null); + if (expression.getRemainingExpression().isPresent() || !expression.getDefinition().isPresent()) { + return new ClpExpression(node); + } + queryBuilder.append(expression.getDefinition().get()); + queryBuilder.append(" OR "); + } + // Remove the last " OR " from the query + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); + } + + /** + * Handles the IN predicate. + * Example: + * Input: col1 IN (1, 2, 3) + * Output: (col1: 1 OR col1: 2 OR col1: 3) + */ + private ClpExpression handleIn(SpecialFormExpression node) + { + ClpExpression variable = node.getArguments().get(0).accept(this, null); + if (!variable.getDefinition().isPresent()) { + return new ClpExpression(node); + } + String variableName = variable.getDefinition().get(); + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append("("); + for (RowExpression argument : node.getArguments().subList(1, node.getArguments().size())) { + if (!(argument instanceof ConstantExpression)) { + return new ClpExpression(node); + } + ConstantExpression literal = (ConstantExpression) argument; + String literalString = getLiteralString(literal); + queryBuilder.append(variableName).append(": "); + if (literal.getType() instanceof VarcharType) { + queryBuilder.append("\"").append(literalString).append("\""); + } + else { + queryBuilder.append(literalString); + } + queryBuilder.append(" OR "); + } + // Remove the last " OR " from the query + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); + } + + /** + * Handles the IS NULL predicate. + * Example: + * Input: col1 IS NULL + * Output: NOT col1: * + */ + private ClpExpression handleIsNull(SpecialFormExpression node) + { + 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); + if (!expression.getDefinition().isPresent()) { + return new ClpExpression(node); + } + + String variableName = expression.getDefinition().get(); + return new ClpExpression(String.format("NOT %s: *", variableName)); + } + + /** + * Handles dereference expressions on RowTypes (e.g., col.row_field). + * Converts row dereferences into dot-separated field access. + * Example: + * Input: address.city (from a RowType 'address') + * Output: address.city + */ + private ClpExpression handleDereference(RowExpression expression) + { + if (expression instanceof VariableReferenceExpression) { + return expression.accept(this, null); + } + + if (!(expression instanceof SpecialFormExpression)) { + return new ClpExpression(expression); + } + + SpecialFormExpression specialForm = (SpecialFormExpression) expression; + List arguments = specialForm.getArguments(); + if (arguments.size() != 2) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE expects 2 arguments"); + } + + RowExpression base = arguments.get(0); + RowExpression index = arguments.get(1); + if (!(index instanceof ConstantExpression)) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE index must be a constant"); + } + + ConstantExpression constExpr = (ConstantExpression) index; + Object value = constExpr.getValue(); + if (!(value instanceof Long)) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE index constant is not a long"); + } + + int fieldIndex = ((Long) value).intValue(); + + Type baseType = base.getType(); + if (!(baseType instanceof RowType)) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE base is not a RowType: " + baseType); + } + + RowType rowType = (RowType) baseType; + if (fieldIndex < 0 || fieldIndex >= rowType.getFields().size()) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "Invalid field index " + fieldIndex + " for RowType: " + rowType); + } + + RowType.Field field = rowType.getFields().get(fieldIndex); + String fieldName = field.getName().orElse("field" + fieldIndex); + + ClpExpression baseString = handleDereference(base); + if (!baseString.getDefinition().isPresent()) { + return new ClpExpression(expression); + } + return new ClpExpression(baseString.getDefinition().get() + "." + fieldName); + } + + /** + * Handles LIKE expressions. + * Transforms SQL LIKE into KQL queries using wildcards (* and ?). + * Supports constant patterns or constant casts only. + * Example: + * Input: col1 LIKE 'a_bc%' + * Output: col1: "a?bc*" + */ + private ClpExpression handleLike(CallExpression node) + { + 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); + if (!variable.getDefinition().isPresent()) { + return new ClpExpression(node); + } + + String variableName = variable.getDefinition().get(); + RowExpression argument = node.getArguments().get(1); + + String pattern; + if (argument instanceof ConstantExpression) { + ConstantExpression literal = (ConstantExpression) argument; + pattern = getLiteralString(literal); + } + else if (argument instanceof CallExpression) { + CallExpression callExpression = (CallExpression) argument; + if (!standardFunctionResolution.isCastFunction(callExpression.getFunctionHandle())) { + return new ClpExpression(node); + } + if (callExpression.getArguments().size() != 1) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "CAST function must have exactly one argument. Received: " + callExpression); + } + if (!(callExpression.getArguments().get(0) instanceof ConstantExpression)) { + return new ClpExpression(node); + } + pattern = getLiteralString((ConstantExpression) callExpression.getArguments().get(0)); + } + else { + return new ClpExpression(node); + } + pattern = pattern.replace("%", "*").replace("_", "?"); + return new ClpExpression(String.format("%s: \"%s\"", variableName, pattern)); + } + + private static class SubstrInfo + { + String variableName; + RowExpression startExpression; + RowExpression lengthExpression; + SubstrInfo(String variableName, RowExpression start, RowExpression length) + { + this.variableName = variableName; + this.startExpression = start; + this.lengthExpression = length; + } + } + + /** + * Parse SUBSTR(...) calls that appear either as: + * SUBSTR(x, start) + * or + * SUBSTR(x, start, length) + */ + private Optional parseSubstringCall(CallExpression callExpression) + { + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(callExpression.getFunctionHandle()); + String functionName = functionMetadata.getName().getObjectName(); + if (!functionName.equals("substr")) { + return Optional.empty(); + } + + int argCount = callExpression.getArguments().size(); + if (argCount < 2 || argCount > 3) { + return Optional.empty(); + } + + ClpExpression variable = callExpression.getArguments().get(0).accept(this, null); + if (!variable.getDefinition().isPresent()) { + return Optional.empty(); + } + + String varName = variable.getDefinition().get(); + RowExpression startExpression = callExpression.getArguments().get(1); + RowExpression lengthExpression = null; + if (argCount == 3) { + lengthExpression = callExpression.getArguments().get(2); + } + + return Optional.of(new SubstrInfo(varName, startExpression, lengthExpression)); + } + + /** + * Attempt to parse "start" or "length" as an integer. + */ + private Optional parseIntValue(RowExpression expression) + { + if (expression instanceof ConstantExpression) { + try { + return Optional.of(Integer.parseInt(getLiteralString((ConstantExpression) expression))); + } + catch (NumberFormatException ignored) { } + } + else if (expression instanceof CallExpression) { + CallExpression call = (CallExpression) expression; + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(call.getFunctionHandle()); + Optional operatorTypeOptional = functionMetadata.getOperatorType(); + if (operatorTypeOptional.isPresent() && operatorTypeOptional.get().equals(OperatorType.NEGATION)) { + RowExpression arg0 = call.getArguments().get(0); + if (arg0 instanceof ConstantExpression) { + try { + return Optional.of(-Integer.parseInt(getLiteralString((ConstantExpression) arg0))); + } + catch (NumberFormatException ignored) { } + } + } + } + return Optional.empty(); + } + + /** + * If lengthExpression is a constant integer that matches targetString.length(), + * return that length. Otherwise empty. + */ + private Optional parseLengthLiteralOrFunction(RowExpression lengthExpression, String targetString) + { + if (lengthExpression instanceof ConstantExpression) { + String val = getLiteralString((ConstantExpression) lengthExpression); + try { + int parsed = Integer.parseInt(val); + if (parsed == targetString.length()) { + return Optional.of(parsed); + } + } + catch (NumberFormatException ignored) { } + } + return Optional.empty(); + } + + /** + * Translate SUBSTR(x, start) or SUBSTR(x, start, length) = 'someString' to KQL. + * Examples: + * SUBSTR(message, 1, 3) = 'abc' + * → message: "abc*" + * SUBSTR(message, 4, 3) = 'abc' + * → message: "???abc*" + * SUBSTR(message, 2) = 'hello' + * → message: "?hello" + * SUBSTR(message, -5) = 'hello' + * → message: "*hello" + */ + private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetString) + { + if (info.lengthExpression != null) { + Optional maybeStart = parseIntValue(info.startExpression); + Optional maybeLen = parseLengthLiteralOrFunction(info.lengthExpression, targetString); + + if (maybeStart.isPresent() && maybeLen.isPresent()) { + int start = maybeStart.get(); + int len = maybeLen.get(); + if (start > 0 && len == targetString.length()) { + StringBuilder result = new StringBuilder(); + result.append(info.variableName).append(": \""); + for (int i = 1; i < start; i++) { + result.append("?"); + } + result.append(targetString).append("*\""); + return new ClpExpression(result.toString()); + } + } + } + else { + Optional maybeStart = parseIntValue(info.startExpression); + if (maybeStart.isPresent()) { + int start = maybeStart.get(); + if (start > 0) { + StringBuilder result = new StringBuilder(); + result.append(info.variableName).append(": \""); + for (int i = 1; i < start; i++) { + result.append("?"); + } + result.append(targetString).append("\""); + return new ClpExpression(result.toString()); + } + if (start == -targetString.length()) { + return new ClpExpression(String.format("%s: \"*%s\"", info.variableName, targetString)); + } + } + } + + return new ClpExpression(); + } + + /** + * Checks whether the given expression matches the pattern SUBSTR(x, ...) = 'someString', + * and if so, attempts to convert it into a KQL query using wildcards and construct a CLP expression. + */ + private ClpExpression tryInterpretSubstringEquality( + OperatorType operator, + RowExpression possibleSubstring, + RowExpression possibleLiteral) + { + if (!operator.equals(OperatorType.EQUAL)) { + return new ClpExpression(); + } + + if (!(possibleSubstring instanceof CallExpression) || + !(possibleLiteral instanceof ConstantExpression)) { + return new ClpExpression(); + } + + Optional maybeSubstringCall = parseSubstringCall((CallExpression) possibleSubstring); + if (!maybeSubstringCall.isPresent()) { + return new ClpExpression(); + } + + String targetString = getLiteralString((ConstantExpression) possibleLiteral); + return interpretSubstringEquality(maybeSubstringCall.get(), targetString); + } + + /** + * Builds a CLP expression from a basic comparison between a variable and a literal. + * Handles different operator types (EQUAL, NOT_EQUAL, and logical binary ops like <, >, etc.) + * and formats them appropriately based on whether the literal is a string or a non-string type. + * Examples: + * col = 'abc' → col: "abc" + * col != 42 → NOT col: 42 + * 5 < col → col > 5 + */ + private ClpExpression buildClpExpression( + String variableName, + String literalString, + OperatorType operator, + Type literalType, + RowExpression originalNode) + { + if (operator.equals(OperatorType.EQUAL)) { + if (literalType instanceof VarcharType) { + return new ClpExpression(String.format("%s: \"%s\"", variableName, literalString)); + } + else { + return new ClpExpression(String.format("%s: %s", variableName, literalString)); + } + } + else if (operator.equals(OperatorType.NOT_EQUAL)) { + if (literalType instanceof VarcharType) { + return new ClpExpression(String.format("NOT %s: \"%s\"", variableName, literalString)); + } + else { + return new ClpExpression(String.format("NOT %s: %s", variableName, literalString)); + } + } + else if (LOGICAL_BINARY_OPS_FILTER.contains(operator) && !(literalType instanceof VarcharType)) { + return new ClpExpression(String.format("%s %s %s", variableName, operator.getOperator(), literalString)); + } + return new ClpExpression(originalNode); + } + + /** + * Handles logical binary operators (e.g., =, !=, <, >) between two expressions. + * Supports constant on either side by flipping the operator when needed. + * Also checks for SUBSTR(x, ...) = 'value' patterns and delegates to substring handler. + */ + private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression node) + { + if (node.getArguments().size() != 2) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, + "Logical binary operator must have exactly two arguments. Received: " + node); + } + RowExpression left = node.getArguments().get(0); + RowExpression right = node.getArguments().get(1); + + ClpExpression maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right); + if (maybeLeftSubstring.getDefinition().isPresent()) { + return maybeLeftSubstring; + } + + ClpExpression maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left); + if (maybeRightSubstring.getDefinition().isPresent()) { + return maybeRightSubstring; + } + + ClpExpression leftExpression = left.accept(this, null); + ClpExpression rightExpression = right.accept(this, null); + Optional leftDefinition = leftExpression.getDefinition(); + Optional rightDefinition = rightExpression.getDefinition(); + if (!leftDefinition.isPresent() || !rightDefinition.isPresent()) { + return new ClpExpression(node); + } + + boolean leftIsConstant = (left instanceof ConstantExpression); + boolean rightIsConstant = (right instanceof ConstantExpression); + + Type leftType = left.getType(); + Type rightType = right.getType(); + + if (rightIsConstant) { + return buildClpExpression( + leftDefinition.get(), // variable + rightDefinition.get(), // literal + operator, + rightType, + node); + } + else if (leftIsConstant) { + OperatorType newOperator = OperatorType.flip(operator); + return buildClpExpression( + rightDefinition.get(), // variable + leftDefinition.get(), // literal + newOperator, + leftType, + node); + } + // fallback + return new ClpExpression(node); + } +} 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 new file mode 100644 index 0000000000000..bfa76edce63b5 --- /dev/null +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java @@ -0,0 +1,115 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.plugin.clp; + +import com.facebook.airlift.log.Logger; +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.ConnectorPlanOptimizer; +import com.facebook.presto.spi.ConnectorPlanRewriter; +import com.facebook.presto.spi.ConnectorSession; +import com.facebook.presto.spi.TableHandle; +import com.facebook.presto.spi.VariableAllocator; +import com.facebook.presto.spi.function.FunctionMetadataManager; +import com.facebook.presto.spi.function.StandardFunctionResolution; +import com.facebook.presto.spi.plan.FilterNode; +import com.facebook.presto.spi.plan.PlanNode; +import com.facebook.presto.spi.plan.PlanNodeIdAllocator; +import com.facebook.presto.spi.plan.TableScanNode; +import com.facebook.presto.spi.relation.RowExpression; +import com.facebook.presto.spi.relation.VariableReferenceExpression; + +import java.util.Map; +import java.util.Optional; + +import static com.facebook.presto.spi.ConnectorPlanRewriter.rewriteWith; +import static java.util.Objects.requireNonNull; + +public class ClpPlanOptimizer + implements ConnectorPlanOptimizer +{ + private static final Logger log = Logger.get(ClpPlanOptimizer.class); + private final FunctionMetadataManager functionManager; + private final StandardFunctionResolution functionResolution; + + public ClpPlanOptimizer(FunctionMetadataManager functionManager, + StandardFunctionResolution functionResolution) + { + this.functionManager = requireNonNull(functionManager, "functionManager is null"); + this.functionResolution = requireNonNull(functionResolution, "functionResolution is null"); + } + + @Override + public PlanNode optimize(PlanNode maxSubplan, + ConnectorSession session, + VariableAllocator variableAllocator, + PlanNodeIdAllocator idAllocator) + { + return rewriteWith(new Rewriter(idAllocator), maxSubplan); + } + + private class Rewriter + extends ConnectorPlanRewriter + { + private final PlanNodeIdAllocator idAllocator; + + public Rewriter(PlanNodeIdAllocator idAllocator) + { + this.idAllocator = idAllocator; + } + + @Override + public PlanNode visitFilter(FilterNode node, RewriteContext context) + { + if (!(node.getSource() instanceof TableScanNode)) { + return node; + } + + TableScanNode tableScanNode = (TableScanNode) node.getSource(); + Map assignments = tableScanNode.getAssignments(); + TableHandle tableHandle = tableScanNode.getTable(); + ClpTableHandle clpTableHandle = (ClpTableHandle) tableHandle.getConnectorHandle(); + ClpExpression clpExpression = node.getPredicate() + .accept(new ClpFilterToKqlConverter(functionResolution, functionManager, assignments), null); + Optional kqlQuery = clpExpression.getDefinition(); + Optional remainingPredicate = clpExpression.getRemainingExpression(); + if (!kqlQuery.isPresent()) { + return node; + } + log.debug("KQL query: %s", kqlQuery.get()); + ClpTableLayoutHandle clpTableLayoutHandle = new ClpTableLayoutHandle(clpTableHandle, kqlQuery); + TableScanNode newTableScanNode = new TableScanNode( + tableScanNode.getSourceLocation(), + idAllocator.getNextId(), + new TableHandle( + tableHandle.getConnectorId(), + clpTableHandle, + tableHandle.getTransaction(), + Optional.of(clpTableLayoutHandle)), + tableScanNode.getOutputVariables(), + tableScanNode.getAssignments(), + tableScanNode.getTableConstraints(), + tableScanNode.getCurrentConstraint(), + tableScanNode.getEnforcedConstraint(), + tableScanNode.getCteMaterializationInfo()); + if (!remainingPredicate.isPresent()) { + return newTableScanNode; + } + + return new FilterNode(node.getSourceLocation(), + idAllocator.getNextId(), + newTableScanNode, + remainingPredicate.get()); + } + } +} diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java new file mode 100644 index 0000000000000..8dc884acc2682 --- /dev/null +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java @@ -0,0 +1,51 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.plugin.clp; + +import com.facebook.presto.spi.ConnectorPlanOptimizer; +import com.facebook.presto.spi.connector.ConnectorPlanOptimizerProvider; +import com.facebook.presto.spi.function.FunctionMetadataManager; +import com.facebook.presto.spi.function.StandardFunctionResolution; +import com.google.common.collect.ImmutableSet; + +import javax.inject.Inject; + +import java.util.Set; + +public class ClpPlanOptimizerProvider + implements ConnectorPlanOptimizerProvider +{ + private final FunctionMetadataManager functionManager; + private final StandardFunctionResolution functionResolution; + + @Inject + public ClpPlanOptimizerProvider(FunctionMetadataManager functionManager, + StandardFunctionResolution functionResolution) + { + this.functionManager = functionManager; + this.functionResolution = functionResolution; + } + + @Override + public Set getLogicalPlanOptimizers() + { + return ImmutableSet.of(); + } + + @Override + public Set getPhysicalPlanOptimizers() + { + return ImmutableSet.of(new ClpPlanOptimizer(functionManager, functionResolution)); + } +} 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 new file mode 100644 index 0000000000000..51eb670fd9d39 --- /dev/null +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java @@ -0,0 +1,215 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.plugin.clp; + +import com.facebook.presto.spi.relation.RowExpression; +import org.testng.annotations.Test; + +import java.util.Optional; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +@Test +public class TestClpFilterToKql + extends TestClpQueryBase +{ + private void testFilter(String sqlExpression, Optional expectedKqlExpression, + Optional expectedRemainingExpression, SessionHolder sessionHolder) + { + RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); + ClpExpression clpExpression = pushDownExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); + Optional kqlExpression = clpExpression.getDefinition(); + Optional remainingExpression = clpExpression.getRemainingExpression(); + if (expectedKqlExpression.isPresent()) { + assertTrue(kqlExpression.isPresent()); + assertEquals(kqlExpression.get(), expectedKqlExpression.get()); + } + + if (expectedRemainingExpression.isPresent()) { + assertTrue(remainingExpression.isPresent()); + assertEquals(remainingExpression.get(), getRowExpression(expectedRemainingExpression.get(), sessionHolder)); + } + else { + assertFalse(remainingExpression.isPresent()); + } + } + + @Test + public void testStringMatchPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + // Exact match + testFilter("city.Name = 'hello world'", Optional.of("city.Name: \"hello world\""), Optional.empty(), sessionHolder); + testFilter("'hello world' = city.Name", Optional.of("city.Name: \"hello world\""), Optional.empty(), sessionHolder); + + // Like predicates that are transformed into substring match + testFilter("city.Name like 'hello%'", Optional.of("city.Name: \"hello*\""), Optional.empty(), sessionHolder); + testFilter("city.Name like '%hello'", Optional.of("city.Name: \"*hello\""), Optional.empty(), sessionHolder); + + // Like predicates that are transformed into CARDINALITY(SPLIT(x, 'some string', 2)) = 2 form, and they are not pushed down for now + testFilter("city.Name like '%hello%'", Optional.empty(), Optional.of("city.Name like '%hello%'"), sessionHolder); + + // Like predicates that are kept in the original forms + testFilter("city.Name like 'hello_'", Optional.of("city.Name: \"hello?\""), Optional.empty(), sessionHolder); + testFilter("city.Name like '_hello'", Optional.of("city.Name: \"?hello\""), Optional.empty(), sessionHolder); + testFilter("city.Name like 'hello_w%'", Optional.of("city.Name: \"hello?w*\""), Optional.empty(), sessionHolder); + testFilter("city.Name like '%hello_w'", Optional.of("city.Name: \"*hello?w\""), Optional.empty(), sessionHolder); + testFilter("city.Name like 'hello%world'", Optional.of("city.Name: \"hello*world\""), Optional.empty(), sessionHolder); + testFilter("city.Name like 'hello%wor%ld'", Optional.of("city.Name: \"hello*wor*ld\""), Optional.empty(), sessionHolder); + } + + @Test + public void testSubStringPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("substr(city.Name, 1, 2) = 'he'", Optional.of("city.Name: \"he*\""), Optional.empty(), sessionHolder); + testFilter("substr(city.Name, 5, 2) = 'he'", Optional.of("city.Name: \"????he*\""), Optional.empty(), sessionHolder); + testFilter("substr(city.Name, 5) = 'he'", Optional.of("city.Name: \"????he\""), Optional.empty(), sessionHolder); + testFilter("substr(city.Name, -2) = 'he'", Optional.of("city.Name: \"*he\""), Optional.empty(), sessionHolder); + + // Invalid substring index is not pushed down + testFilter("substr(city.Name, 1, 5) = 'he'", Optional.empty(), Optional.of("substr(city.Name, 1, 5) = 'he'"), sessionHolder); + testFilter("substr(city.Name, -5) = 'he'", Optional.empty(), Optional.of("substr(city.Name, -5) = 'he'"), sessionHolder); + } + + @Test + public void testNumericComparisonPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("fare > 0", Optional.of("fare > 0"), Optional.empty(), sessionHolder); + testFilter("fare >= 0", Optional.of("fare >= 0"), Optional.empty(), sessionHolder); + testFilter("fare < 0", Optional.of("fare < 0"), Optional.empty(), sessionHolder); + testFilter("fare <= 0", Optional.of("fare <= 0"), Optional.empty(), sessionHolder); + testFilter("fare = 0", Optional.of("fare: 0"), Optional.empty(), sessionHolder); + testFilter("fare != 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("fare <> 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("0 < fare", Optional.of("fare > 0"), Optional.empty(), sessionHolder); + testFilter("0 <= fare", Optional.of("fare >= 0"), Optional.empty(), sessionHolder); + testFilter("0 > fare", Optional.of("fare < 0"), Optional.empty(), sessionHolder); + testFilter("0 >= fare", Optional.of("fare <= 0"), Optional.empty(), sessionHolder); + testFilter("0 = fare", Optional.of("fare: 0"), Optional.empty(), sessionHolder); + testFilter("0 != fare", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("0 <> fare", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + } + + @Test + public void testOrPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("fare > 0 OR city.Name like 'b%'", Optional.of("(fare > 0 OR city.Name: \"b*\")"), Optional.empty(), sessionHolder); + testFilter( + "lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1", + Optional.empty(), + Optional.of("(lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1)"), + sessionHolder); + + // Multiple ORs + testFilter( + "fare > 0 OR city.Name like 'b%' OR lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1", + Optional.empty(), + Optional.of("fare > 0 OR city.Name like 'b%' OR lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1"), + sessionHolder); + testFilter( + "fare > 0 OR city.Name like 'b%' OR city.Region.Id != 1", + Optional.of("((fare > 0 OR city.Name: \"b*\") OR NOT city.Region.Id: 1)"), + Optional.empty(), + sessionHolder); + } + + @Test + public void testAndPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("fare > 0 AND city.Name like 'b%'", Optional.of("(fare > 0 AND city.Name: \"b*\")"), Optional.empty(), sessionHolder); + testFilter( + "lower(city.Region.Name) = 'hello world' AND city.Region.Id != 1", + Optional.of("(NOT city.Region.Id: 1)"), + Optional.of("lower(city.Region.Name) = 'hello world'"), + sessionHolder); + + // Multiple ANDs + testFilter( + "fare > 0 AND city.Name like 'b%' AND lower(city.Region.Name) = 'hello world' AND city.Region.Id != 1", + Optional.of("(((fare > 0 AND city.Name: \"b*\")) AND NOT city.Region.Id: 1)"), + Optional.of("(lower(city.Region.Name) = 'hello world')"), + sessionHolder); + testFilter( + "fare > 0 AND city.Name like '%b%' AND lower(city.Region.Name) = 'hello world' AND city.Region.Id != 1", + Optional.of("(((fare > 0)) AND NOT city.Region.Id: 1)"), + Optional.of("city.Name like '%b%' AND lower(city.Region.Name) = 'hello world'"), + sessionHolder); + } + + @Test + public void testNotPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("city.Region.Name NOT LIKE 'hello%'", Optional.of("NOT city.Region.Name: \"hello*\""), Optional.empty(), sessionHolder); + testFilter("NOT (city.Region.Name LIKE 'hello%')", Optional.of("NOT city.Region.Name: \"hello*\""), Optional.empty(), sessionHolder); + testFilter("city.Name != 'hello world'", Optional.of("NOT city.Name: \"hello world\""), Optional.empty(), sessionHolder); + testFilter("city.Name <> 'hello world'", Optional.of("NOT city.Name: \"hello world\""), Optional.empty(), sessionHolder); + testFilter("NOT (city.Name = 'hello world')", Optional.of("NOT city.Name: \"hello world\""), Optional.empty(), sessionHolder); + testFilter("fare != 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("fare <> 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("NOT (fare = 0)", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + + // Multiple NOTs + testFilter("NOT (NOT fare = 0)", Optional.of("NOT NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("NOT (fare = 0 AND city.Name = 'hello world')", Optional.of("NOT (fare: 0 AND city.Name: \"hello world\")"), Optional.empty(), sessionHolder); + testFilter("NOT (fare = 0 OR city.Name = 'hello world')", Optional.of("NOT (fare: 0 OR city.Name: \"hello world\")"), Optional.empty(), sessionHolder); + } + + @Test + public void testInPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("city.Name IN ('hello world', 'hello world 2')", Optional.of("(city.Name: \"hello world\" OR city.Name: \"hello world 2\")"), Optional.empty(), sessionHolder); + } + + @Test + public void testIsNullPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter("city.Name IS NULL", Optional.of("NOT city.Name: *"), Optional.empty(), sessionHolder); + testFilter("city.Name IS NOT NULL", Optional.of("NOT NOT city.Name: *"), Optional.empty(), sessionHolder); + testFilter("NOT (city.Name IS NULL)", Optional.of("NOT NOT city.Name: *"), Optional.empty(), sessionHolder); + } + + @Test + public void testComplexPushdown() + { + SessionHolder sessionHolder = new SessionHolder(); + + testFilter( + "(fare > 0 OR city.Name like 'b%') AND (lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)", + Optional.of("((fare > 0 OR city.Name: \"b*\"))"), + Optional.of("(lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)"), + sessionHolder); + testFilter( + "city.Region.Id = 1 AND (fare > 0 OR city.Name NOT like 'b%') AND (lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)", + Optional.of("((city.Region.Id: 1 AND (fare > 0 OR NOT city.Name: \"b*\")))"), + Optional.of("lower(city.Region.Name) = 'hello world' OR city.Name IS NULL"), + sessionHolder); + } +} diff --git a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java new file mode 100644 index 0000000000000..7a5eebd751d4b --- /dev/null +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java @@ -0,0 +1,145 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.plugin.clp; + +import com.facebook.presto.Session; +import com.facebook.presto.SystemSessionProperties; +import com.facebook.presto.common.block.BlockEncodingManager; +import com.facebook.presto.common.type.RowType; +import com.facebook.presto.common.type.Type; +import com.facebook.presto.metadata.AnalyzePropertyManager; +import com.facebook.presto.metadata.CatalogManager; +import com.facebook.presto.metadata.ColumnPropertyManager; +import com.facebook.presto.metadata.FunctionAndTypeManager; +import com.facebook.presto.metadata.Metadata; +import com.facebook.presto.metadata.MetadataManager; +import com.facebook.presto.metadata.SchemaPropertyManager; +import com.facebook.presto.metadata.TablePropertyManager; +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.ConnectorSession; +import com.facebook.presto.spi.SchemaTableName; +import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.function.StandardFunctionResolution; +import com.facebook.presto.spi.relation.RowExpression; +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.ExpressionUtils; +import com.facebook.presto.sql.parser.ParsingOptions; +import com.facebook.presto.sql.parser.SqlParser; +import com.facebook.presto.sql.planner.TypeProvider; +import com.facebook.presto.sql.relational.FunctionResolution; +import com.facebook.presto.sql.relational.SqlToRowExpressionTranslator; +import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.NodeRef; +import com.facebook.presto.testing.TestingSession; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +import java.util.Map; +import java.util.Optional; +import java.util.stream.Stream; + +import static com.facebook.presto.common.type.BigintType.BIGINT; +import static com.facebook.presto.common.type.BooleanType.BOOLEAN; +import static com.facebook.presto.common.type.DoubleType.DOUBLE; +import static com.facebook.presto.common.type.VarcharType.VARCHAR; +import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager; +import static com.facebook.presto.metadata.SessionPropertyManager.createTestingSessionPropertyManager; +import static com.facebook.presto.sql.analyzer.ExpressionAnalyzer.getExpressionTypes; +import static com.facebook.presto.testing.TestingConnectorSession.SESSION; +import static com.facebook.presto.transaction.InMemoryTransactionManager.createTestTransactionManager; +import static java.util.stream.Collectors.toMap; + +public class TestClpQueryBase +{ + protected static final FunctionAndTypeManager functionAndTypeManager = createTestFunctionAndTypeManager(); + protected static final StandardFunctionResolution standardFunctionResolution = new FunctionResolution(functionAndTypeManager.getFunctionAndTypeResolver()); + protected static final Metadata metadata = new MetadataManager( + functionAndTypeManager, + new BlockEncodingManager(), + createTestingSessionPropertyManager(), + new SchemaPropertyManager(), + new TablePropertyManager(), + new ColumnPropertyManager(), + new AnalyzePropertyManager(), + createTestTransactionManager(new CatalogManager())); + + protected static final ClpTableHandle table = new ClpTableHandle(new SchemaTableName("default", "test"), "", ClpTableHandle.StorageType.FS); + protected static final ClpColumnHandle city = new ClpColumnHandle( + "city", + RowType.from(ImmutableList.of( + RowType.field("Region", RowType.from(ImmutableList.of( + RowType.field("Id", BIGINT), + RowType.field("Name", VARCHAR)))), + RowType.field("Name", VARCHAR))), + true); + protected static final ClpColumnHandle fare = new ClpColumnHandle("fare", DOUBLE, true); + protected static final ClpColumnHandle isHoliday = new ClpColumnHandle("isHoliday", BOOLEAN, true); + protected static final Map variableToColumnHandleMap = + Stream.of(city, fare, isHoliday) + .collect(toMap( + ch -> new VariableReferenceExpression(Optional.empty(), ch.getColumnName(), ch.getColumnType()), + ch -> ch)); + protected final TypeProvider typeProvider = TypeProvider.fromVariables(variableToColumnHandleMap.keySet()); + + public static Expression expression(String sql) + { + return ExpressionUtils.rewriteIdentifiersToSymbolReferences( + new SqlParser().createExpression(sql, new ParsingOptions(ParsingOptions.DecimalLiteralTreatment.AS_DECIMAL))); + } + + protected RowExpression toRowExpression(Expression expression, TypeProvider typeProvider, Session session) + { + Map, Type> expressionTypes = getExpressionTypes( + session, + metadata, + new SqlParser(), + typeProvider, + expression, + ImmutableMap.of(), + WarningCollector.NOOP); + return SqlToRowExpressionTranslator.translate(expression, expressionTypes, ImmutableMap.of(), functionAndTypeManager, session); + } + + protected RowExpression getRowExpression(String sqlExpression, SessionHolder sessionHolder) + { + return toRowExpression(expression(sqlExpression), typeProvider, sessionHolder.getSession()); + } + + protected RowExpression getRowExpression(String sqlExpression, TypeProvider typeProvider, SessionHolder sessionHolder) + { + return toRowExpression(expression(sqlExpression), typeProvider, sessionHolder.getSession()); + } + + protected static class SessionHolder + { + private final ConnectorSession connectorSession; + private final Session session; + + public SessionHolder() + { + connectorSession = SESSION; + session = TestingSession.testSessionBuilder(createTestingSessionPropertyManager(new SystemSessionProperties().getSessionProperties())).build(); + } + + public ConnectorSession getConnectorSession() + { + return connectorSession; + } + + public Session getSession() + { + return session; + } + } +} From b0fc053d966aacd752f5147a7a03d0d0b8447feb Mon Sep 17 00:00:00 2001 From: wraymo Date: Fri, 20 Jun 2025 11:12:08 -0400 Subject: [PATCH 2/7] apply review suggestions --- .../presto/plugin/clp/ClpExpression.java | 16 +- .../plugin/clp/ClpFilterToKqlConverter.java | 761 ++++++++++-------- .../presto/plugin/clp/TestClpFilterToKql.java | 41 +- .../presto/plugin/clp/TestClpQueryBase.java | 11 +- 4 files changed, 448 insertions(+), 381 deletions(-) diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java index 6b9fabbcfecad..ee60ff1f38555 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java @@ -40,19 +40,27 @@ public ClpExpression(String definition, RowExpression remainingExpression) this.remainingExpression = Optional.ofNullable(remainingExpression); } - // Creates an empty ClpExpression (no KQL definition, no remaining expression). + /** + * Creates an empty ClpExpression (no KQL definition, no remaining expression). + */ public ClpExpression() { - this (null, null); + this(null, null); } - // Creates a ClpExpression from a fully translatable KQL string. + /** + * Creates a ClpExpression from a fully translatable KQL string. + * @param definition + */ public ClpExpression(String definition) { this(definition, null); } - // Creates a ClpExpression from a non-translatable RowExpression. + /** + * Creates a ClpExpression from a non-translatable RowExpression. + * @param remainingExpression + */ public ClpExpression(RowExpression remainingExpression) { this(null, remainingExpression); 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 b9c65dfd2ce22..49ee7ce0ba0d5 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 @@ -41,12 +41,17 @@ import static com.facebook.presto.common.function.OperatorType.EQUAL; import static com.facebook.presto.common.function.OperatorType.GREATER_THAN; import static com.facebook.presto.common.function.OperatorType.GREATER_THAN_OR_EQUAL; +import static com.facebook.presto.common.function.OperatorType.IS_DISTINCT_FROM; import static com.facebook.presto.common.function.OperatorType.LESS_THAN; import static com.facebook.presto.common.function.OperatorType.LESS_THAN_OR_EQUAL; +import static com.facebook.presto.common.function.OperatorType.NEGATION; import static com.facebook.presto.common.function.OperatorType.NOT_EQUAL; +import static com.facebook.presto.common.function.OperatorType.flip; import static com.facebook.presto.common.type.BooleanType.BOOLEAN; import static com.facebook.presto.plugin.clp.ClpErrorCode.CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION; import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.AND; +import static java.lang.Integer.parseInt; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; /** @@ -75,7 +80,8 @@ public class ClpFilterToKqlConverter private final FunctionMetadataManager functionMetadataManager; private final Map assignments; - public ClpFilterToKqlConverter(StandardFunctionResolution standardFunctionResolution, + public ClpFilterToKqlConverter( + StandardFunctionResolution standardFunctionResolution, FunctionMetadataManager functionMetadataManager, Map assignments) { @@ -100,7 +106,7 @@ public ClpExpression visitCall(CallExpression node, Void context) Optional operatorTypeOptional = functionMetadata.getOperatorType(); if (operatorTypeOptional.isPresent()) { OperatorType operatorType = operatorTypeOptional.get(); - if (operatorType.isComparisonOperator() && operatorType != OperatorType.IS_DISTINCT_FROM) { + if (operatorType.isComparisonOperator() && operatorType != IS_DISTINCT_FROM) { return handleLogicalBinary(operatorType, node); } } @@ -139,14 +145,20 @@ public ClpExpression visitSpecialForm(SpecialFormExpression node, Void context) } } - // For all other expressions, return the original expression @Override public ClpExpression visitExpression(RowExpression node, Void context) { + // For all other expressions, return the original expression return new ClpExpression(node); } - private static String getLiteralString(ConstantExpression literal) + /** + * Extracts the string representation of a constant expression. + * + * @param literal the constant expression + * @return the string representation of the literal + */ + private String getLiteralString(ConstantExpression literal) { if (literal.getValue() instanceof Slice) { return ((Slice) literal.getValue()).toStringUtf8(); @@ -154,6 +166,12 @@ private static String getLiteralString(ConstantExpression literal) return literal.toString(); } + /** + * Retrieves the original column name from a variable reference. + * + * @param variable the variable reference expression + * @return the original column name as a string + */ private String getVariableName(VariableReferenceExpression variable) { return ((ClpColumnHandle) assignments.get(variable)).getOriginalColumnName(); @@ -162,8 +180,10 @@ private String getVariableName(VariableReferenceExpression variable) /** * Handles the logical NOT expression. * Example: - * Input: NOT (col1 = 5) - * Output: NOT col1: 5 + * - NOT (col1 = 5) → NOT col1: 5 + * + * @param node the NOT call expression + * @return a ClpExpression with the translated KQL query, or the original expression if unsupported */ private ClpExpression handleNot(CallExpression node) { @@ -180,192 +200,15 @@ private ClpExpression handleNot(CallExpression node) return new ClpExpression("NOT " + expression.getDefinition().get()); } - /** - * Handles the logical AND expression. - * Combines all definable child expressions into a single KQL query joined by AND. - * Any unsupported children are collected into remaining expressions. - * Example: - * Input: col1 = 5 AND col2 = 'abc' - * Output: (col1: 5 AND col2: "abc") - */ - private ClpExpression handleAnd(SpecialFormExpression node) - { - StringBuilder queryBuilder = new StringBuilder(); - queryBuilder.append("("); - ArrayList remainingExpressions = new ArrayList<>(); - boolean hasDefinition = false; - for (RowExpression argument : node.getArguments()) { - ClpExpression expression = argument.accept(this, null); - if (expression.getDefinition().isPresent()) { - hasDefinition = true; - queryBuilder.append(expression.getDefinition().get()); - queryBuilder.append(" AND "); - } - if (expression.getRemainingExpression().isPresent()) { - remainingExpressions.add(expression.getRemainingExpression().get()); - } - } - if (!hasDefinition) { - return new ClpExpression(node); - } - else if (!remainingExpressions.isEmpty()) { - if (remainingExpressions.size() == 1) { - return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 5) + ")", remainingExpressions.get(0)); - } - else { - return new ClpExpression( - queryBuilder.substring(0, queryBuilder.length() - 5) + ")", - new SpecialFormExpression(node.getSourceLocation(), AND, BOOLEAN, remainingExpressions)); - } - } - // Remove the last " AND " from the query - return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 5) + ")"); - } - - /** - * Handles the logical OR expression. - * Combines all fully convertible child expressions into a single CLP query joined by OR. - * Returns the original node if any child is unsupported. - * Example: - * Input: col1 = 5 OR col1 = 10 - * Output: (col1: 5 OR col1: 10) - */ - private ClpExpression handleOr(SpecialFormExpression node) - { - StringBuilder queryBuilder = new StringBuilder(); - queryBuilder.append("("); - for (RowExpression argument : node.getArguments()) { - ClpExpression expression = argument.accept(this, null); - if (expression.getRemainingExpression().isPresent() || !expression.getDefinition().isPresent()) { - return new ClpExpression(node); - } - queryBuilder.append(expression.getDefinition().get()); - queryBuilder.append(" OR "); - } - // Remove the last " OR " from the query - return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); - } - - /** - * Handles the IN predicate. - * Example: - * Input: col1 IN (1, 2, 3) - * Output: (col1: 1 OR col1: 2 OR col1: 3) - */ - private ClpExpression handleIn(SpecialFormExpression node) - { - ClpExpression variable = node.getArguments().get(0).accept(this, null); - if (!variable.getDefinition().isPresent()) { - return new ClpExpression(node); - } - String variableName = variable.getDefinition().get(); - StringBuilder queryBuilder = new StringBuilder(); - queryBuilder.append("("); - for (RowExpression argument : node.getArguments().subList(1, node.getArguments().size())) { - if (!(argument instanceof ConstantExpression)) { - return new ClpExpression(node); - } - ConstantExpression literal = (ConstantExpression) argument; - String literalString = getLiteralString(literal); - queryBuilder.append(variableName).append(": "); - if (literal.getType() instanceof VarcharType) { - queryBuilder.append("\"").append(literalString).append("\""); - } - else { - queryBuilder.append(literalString); - } - queryBuilder.append(" OR "); - } - // Remove the last " OR " from the query - return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); - } - - /** - * Handles the IS NULL predicate. - * Example: - * Input: col1 IS NULL - * Output: NOT col1: * - */ - private ClpExpression handleIsNull(SpecialFormExpression node) - { - 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); - if (!expression.getDefinition().isPresent()) { - return new ClpExpression(node); - } - - String variableName = expression.getDefinition().get(); - return new ClpExpression(String.format("NOT %s: *", variableName)); - } - - /** - * Handles dereference expressions on RowTypes (e.g., col.row_field). - * Converts row dereferences into dot-separated field access. - * Example: - * Input: address.city (from a RowType 'address') - * Output: address.city - */ - private ClpExpression handleDereference(RowExpression expression) - { - if (expression instanceof VariableReferenceExpression) { - return expression.accept(this, null); - } - - if (!(expression instanceof SpecialFormExpression)) { - return new ClpExpression(expression); - } - - SpecialFormExpression specialForm = (SpecialFormExpression) expression; - List arguments = specialForm.getArguments(); - if (arguments.size() != 2) { - throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE expects 2 arguments"); - } - - RowExpression base = arguments.get(0); - RowExpression index = arguments.get(1); - if (!(index instanceof ConstantExpression)) { - throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE index must be a constant"); - } - - ConstantExpression constExpr = (ConstantExpression) index; - Object value = constExpr.getValue(); - if (!(value instanceof Long)) { - throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE index constant is not a long"); - } - - int fieldIndex = ((Long) value).intValue(); - - Type baseType = base.getType(); - if (!(baseType instanceof RowType)) { - throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE base is not a RowType: " + baseType); - } - - RowType rowType = (RowType) baseType; - if (fieldIndex < 0 || fieldIndex >= rowType.getFields().size()) { - throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "Invalid field index " + fieldIndex + " for RowType: " + rowType); - } - - RowType.Field field = rowType.getFields().get(fieldIndex); - String fieldName = field.getName().orElse("field" + fieldIndex); - - ClpExpression baseString = handleDereference(base); - if (!baseString.getDefinition().isPresent()) { - return new ClpExpression(expression); - } - return new ClpExpression(baseString.getDefinition().get() + "." + fieldName); - } - /** * Handles LIKE expressions. - * Transforms SQL LIKE into KQL queries using wildcards (* and ?). - * Supports constant patterns or constant casts only. + * Converts SQL LIKE patterns into equivalent KQL queries using * (for %) and ? (for _). + * Only supports constant patterns or constant cast patterns. * Example: - * Input: col1 LIKE 'a_bc%' - * Output: col1: "a?bc*" + * - col1 LIKE 'a_bc%' → col1: "a?bc*" + * + * @param node the LIKE call expression + * @return a ClpExpression with the KQL equivalent of the LIKE expression, or the original node if unsupported */ private ClpExpression handleLike(CallExpression node) { @@ -402,120 +245,201 @@ else if (argument instanceof CallExpression) { return new ClpExpression(node); } pattern = pattern.replace("%", "*").replace("_", "?"); - return new ClpExpression(String.format("%s: \"%s\"", variableName, pattern)); - } - - private static class SubstrInfo - { - String variableName; - RowExpression startExpression; - RowExpression lengthExpression; - SubstrInfo(String variableName, RowExpression start, RowExpression length) - { - this.variableName = variableName; - this.startExpression = start; - this.lengthExpression = length; - } + return new ClpExpression(format("%s: \"%s\"", variableName, pattern)); } /** - * Parse SUBSTR(...) calls that appear either as: - * SUBSTR(x, start) - * or - * SUBSTR(x, start, length) + * Handles logical binary operators (e.g., =, !=, <, >) between two expressions. + * Supports constant on either side by flipping the operator when needed. + * Also checks for SUBSTR(x, ...) = 'value' patterns and delegates to substring handler. + * If the expression cannot be translated, it returns a fallback expression that preserves the original node. + * + * @param operator the logical binary operator (e.g., EQUAL, NOT_EQUAL, LESS_THAN) + * @param node the call expression representing the binary operation + * @return an expression with a KQL query if possible, otherwise a fallback expression */ - private Optional parseSubstringCall(CallExpression callExpression) + private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression node) { - FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(callExpression.getFunctionHandle()); - String functionName = functionMetadata.getName().getObjectName(); - if (!functionName.equals("substr")) { - return Optional.empty(); + if (node.getArguments().size() != 2) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, + "Logical binary operator must have exactly two arguments. Received: " + node); } + RowExpression left = node.getArguments().get(0); + RowExpression right = node.getArguments().get(1); - int argCount = callExpression.getArguments().size(); - if (argCount < 2 || argCount > 3) { - return Optional.empty(); + ClpExpression maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right); + if (maybeLeftSubstring.getDefinition().isPresent()) { + return maybeLeftSubstring; } - ClpExpression variable = callExpression.getArguments().get(0).accept(this, null); - if (!variable.getDefinition().isPresent()) { - return Optional.empty(); + ClpExpression maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left); + if (maybeRightSubstring.getDefinition().isPresent()) { + return maybeRightSubstring; } - String varName = variable.getDefinition().get(); - RowExpression startExpression = callExpression.getArguments().get(1); - RowExpression lengthExpression = null; - if (argCount == 3) { - lengthExpression = callExpression.getArguments().get(2); + ClpExpression leftExpression = left.accept(this, null); + ClpExpression rightExpression = right.accept(this, null); + Optional leftDefinition = leftExpression.getDefinition(); + Optional rightDefinition = rightExpression.getDefinition(); + if (!leftDefinition.isPresent() || !rightDefinition.isPresent()) { + return new ClpExpression(node); } - return Optional.of(new SubstrInfo(varName, startExpression, lengthExpression)); + boolean leftIsConstant = (left instanceof ConstantExpression); + boolean rightIsConstant = (right instanceof ConstantExpression); + + Type leftType = left.getType(); + Type rightType = right.getType(); + + if (rightIsConstant) { + return buildClpExpression( + leftDefinition.get(), // variable + rightDefinition.get(), // literal + operator, + rightType, + node); + } + else if (leftIsConstant) { + OperatorType newOperator = flip(operator); + return buildClpExpression( + rightDefinition.get(), // variable + leftDefinition.get(), // literal + newOperator, + leftType, + node); + } + // fallback + return new ClpExpression(node); } /** - * Attempt to parse "start" or "length" as an integer. + * Builds a CLP expression from a basic comparison between a variable and a literal. + * Handles different operator types (EQUAL, NOT_EQUAL, and logical binary ops like <, >, etc.) + * and formats them appropriately based on whether the literal is a string or a non-string type. + * Examples: + * - col = 'abc' → col: "abc" + * - col != 42 → NOT col: 42 + * - 5 < col → col > 5 + * + * @param variableName name of the variable + * @param literalString string representation of the literal + * @param operator operator used in the comparison + * @param literalType type of the literal + * @param originalNode the original RowExpression node + * @return a ClpExpression containing the KQL filter or fallback to the original node if unsupported */ - private Optional parseIntValue(RowExpression expression) + private ClpExpression buildClpExpression( + String variableName, + String literalString, + OperatorType operator, + Type literalType, + RowExpression originalNode) { - if (expression instanceof ConstantExpression) { - try { - return Optional.of(Integer.parseInt(getLiteralString((ConstantExpression) expression))); + if (operator.equals(EQUAL)) { + if (literalType instanceof VarcharType) { + return new ClpExpression(format("%s: \"%s\"", variableName, literalString)); + } + else { + return new ClpExpression(format("%s: %s", variableName, literalString)); } - catch (NumberFormatException ignored) { } } - else if (expression instanceof CallExpression) { - CallExpression call = (CallExpression) expression; - FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(call.getFunctionHandle()); - Optional operatorTypeOptional = functionMetadata.getOperatorType(); - if (operatorTypeOptional.isPresent() && operatorTypeOptional.get().equals(OperatorType.NEGATION)) { - RowExpression arg0 = call.getArguments().get(0); - if (arg0 instanceof ConstantExpression) { - try { - return Optional.of(-Integer.parseInt(getLiteralString((ConstantExpression) arg0))); - } - catch (NumberFormatException ignored) { } - } + else if (operator.equals(NOT_EQUAL)) { + if (literalType instanceof VarcharType) { + return new ClpExpression(format("NOT %s: \"%s\"", variableName, literalString)); + } + else { + return new ClpExpression(format("NOT %s: %s", variableName, literalString)); } } - return Optional.empty(); + else if (LOGICAL_BINARY_OPS_FILTER.contains(operator) && !(literalType instanceof VarcharType)) { + return new ClpExpression(format("%s %s %s", variableName, operator.getOperator(), literalString)); + } + return new ClpExpression(originalNode); } /** - * If lengthExpression is a constant integer that matches targetString.length(), - * return that length. Otherwise empty. + * Checks whether the given expression matches the pattern SUBSTR(x, ...) = 'someString', + * and if so, attempts to convert it into a KQL query using wildcards and construct a CLP expression. + * + * @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 the translated KQL filter or an empty one if conversion fails */ - private Optional parseLengthLiteralOrFunction(RowExpression lengthExpression, String targetString) + private ClpExpression tryInterpretSubstringEquality( + OperatorType operator, + RowExpression possibleSubstring, + RowExpression possibleLiteral) { - if (lengthExpression instanceof ConstantExpression) { - String val = getLiteralString((ConstantExpression) lengthExpression); - try { - int parsed = Integer.parseInt(val); - if (parsed == targetString.length()) { - return Optional.of(parsed); - } - } - catch (NumberFormatException ignored) { } + if (!operator.equals(EQUAL)) { + return new ClpExpression(); } - return Optional.empty(); + + if (!(possibleSubstring instanceof CallExpression) || + !(possibleLiteral instanceof ConstantExpression)) { + return new ClpExpression(); + } + + Optional maybeSubstringCall = parseSubstringCall((CallExpression) possibleSubstring); + if (!maybeSubstringCall.isPresent()) { + return new ClpExpression(); + } + + String targetString = getLiteralString((ConstantExpression) possibleLiteral); + return interpretSubstringEquality(maybeSubstringCall.get(), targetString); + } + + /** + * Parses a SUBSTR(x, start [, length]) call into a SubstrInfo object if valid. + * + * @param callExpression the call expression to inspect + * @return an Optional containing SubstrInfo if the expression is a valid SUBSTR call, otherwise empty + */ + private Optional parseSubstringCall(CallExpression callExpression) + { + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(callExpression.getFunctionHandle()); + String functionName = functionMetadata.getName().getObjectName(); + if (!functionName.equals("substr")) { + return Optional.empty(); + } + + int argCount = callExpression.getArguments().size(); + if (argCount < 2 || argCount > 3) { + return Optional.empty(); + } + + ClpExpression variable = callExpression.getArguments().get(0).accept(this, null); + if (!variable.getDefinition().isPresent()) { + return Optional.empty(); + } + + String varName = variable.getDefinition().get(); + RowExpression startExpression = callExpression.getArguments().get(1); + RowExpression lengthExpression = null; + if (argCount == 3) { + lengthExpression = callExpression.getArguments().get(2); + } + + return Optional.of(new SubstrInfo(varName, startExpression, lengthExpression)); } /** - * Translate SUBSTR(x, start) or SUBSTR(x, start, length) = 'someString' to KQL. + * Converts a SUBSTR(x, start [, length]) = 'someString' into a KQL-style wildcard query. * Examples: - * SUBSTR(message, 1, 3) = 'abc' - * → message: "abc*" - * SUBSTR(message, 4, 3) = 'abc' - * → message: "???abc*" - * SUBSTR(message, 2) = 'hello' - * → message: "?hello" - * SUBSTR(message, -5) = 'hello' - * → message: "*hello" + * - SUBSTR(message, 1, 3) = 'abc' → message: "abc*" + * - SUBSTR(message, 4, 3) = 'abc' → message: "???abc*" + * - SUBSTR(message, 2) = 'hello' → message: "?hello" + * - SUBSTR(message, -5) = 'hello' → message: "*hello" + * + * @param info parsed SUBSTR call info + * @param targetString the literal string being compared to + * @return a ClpExpression containing the translated KQL query if successful; otherwise, an empty ClpExpression */ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetString) { if (info.lengthExpression != null) { Optional maybeStart = parseIntValue(info.startExpression); - Optional maybeLen = parseLengthLiteralOrFunction(info.lengthExpression, targetString); + Optional maybeLen = parseLengthLiteral(info.lengthExpression, targetString); if (maybeStart.isPresent() && maybeLen.isPresent()) { int start = maybeStart.get(); @@ -545,7 +469,7 @@ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetS return new ClpExpression(result.toString()); } if (start == -targetString.length()) { - return new ClpExpression(String.format("%s: \"*%s\"", info.variableName, targetString)); + return new ClpExpression(format("%s: \"*%s\"", info.variableName, targetString)); } } } @@ -554,126 +478,261 @@ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetS } /** - * Checks whether the given expression matches the pattern SUBSTR(x, ...) = 'someString', - * and if so, attempts to convert it into a KQL query using wildcards and construct a CLP expression. + * Attempts to parse a RowExpression as an integer constant. + * + * @param expression the row expression to parse + * @return an Optional containing the parsed integer value, if successful */ - private ClpExpression tryInterpretSubstringEquality( - OperatorType operator, - RowExpression possibleSubstring, - RowExpression possibleLiteral) + private Optional parseIntValue(RowExpression expression) { - if (!operator.equals(OperatorType.EQUAL)) { - return new ClpExpression(); + if (expression instanceof ConstantExpression) { + try { + return Optional.of(parseInt(getLiteralString((ConstantExpression) expression))); + } + catch (NumberFormatException ignored) { + } } - - if (!(possibleSubstring instanceof CallExpression) || - !(possibleLiteral instanceof ConstantExpression)) { - return new ClpExpression(); + else if (expression instanceof CallExpression) { + CallExpression call = (CallExpression) expression; + FunctionMetadata functionMetadata = functionMetadataManager.getFunctionMetadata(call.getFunctionHandle()); + Optional operatorTypeOptional = functionMetadata.getOperatorType(); + if (operatorTypeOptional.isPresent() && operatorTypeOptional.get().equals(NEGATION)) { + RowExpression arg0 = call.getArguments().get(0); + if (arg0 instanceof ConstantExpression) { + try { + return Optional.of(-parseInt(getLiteralString((ConstantExpression) arg0))); + } + catch (NumberFormatException ignored) { + } + } + } } + return Optional.empty(); + } - Optional maybeSubstringCall = parseSubstringCall((CallExpression) possibleSubstring); - if (!maybeSubstringCall.isPresent()) { - return new ClpExpression(); + /** + * Attempts to parse the length expression and match it against the target string's length. + * + * @param lengthExpression the expression representing the length parameter + * @param targetString the target string to compare length against + * @return an Optional containing the length if it matches targetString.length(), otherwise empty + */ + private Optional parseLengthLiteral(RowExpression lengthExpression, String targetString) + { + if (lengthExpression instanceof ConstantExpression) { + String val = getLiteralString((ConstantExpression) lengthExpression); + try { + int parsed = parseInt(val); + if (parsed == targetString.length()) { + return Optional.of(parsed); + } + } + catch (NumberFormatException ignored) { + } } - - String targetString = getLiteralString((ConstantExpression) possibleLiteral); - return interpretSubstringEquality(maybeSubstringCall.get(), targetString); + return Optional.empty(); } /** - * Builds a CLP expression from a basic comparison between a variable and a literal. - * Handles different operator types (EQUAL, NOT_EQUAL, and logical binary ops like <, >, etc.) - * and formats them appropriately based on whether the literal is a string or a non-string type. - * Examples: - * col = 'abc' → col: "abc" - * col != 42 → NOT col: 42 - * 5 < col → col > 5 + * Handles the logical AND expression. + * Combines all definable child expressions into a single KQL query joined by AND. + * Any unsupported children are collected into a remaining expression. + * Example: + * - col1 = 5 AND col2 = 'abc' → (col1: 5 AND col2: "abc") + * + * @param node the AND special form expression + * @return a ClpExpression containing the KQL query and any remaining sub-expressions */ - private ClpExpression buildClpExpression( - String variableName, - String literalString, - OperatorType operator, - Type literalType, - RowExpression originalNode) + private ClpExpression handleAnd(SpecialFormExpression node) { - if (operator.equals(OperatorType.EQUAL)) { - if (literalType instanceof VarcharType) { - return new ClpExpression(String.format("%s: \"%s\"", variableName, literalString)); + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append("("); + List remainingExpressions = new ArrayList<>(); + boolean hasDefinition = false; + for (RowExpression argument : node.getArguments()) { + ClpExpression expression = argument.accept(this, null); + if (expression.getDefinition().isPresent()) { + hasDefinition = true; + queryBuilder.append(expression.getDefinition().get()); + queryBuilder.append(" AND "); } - else { - return new ClpExpression(String.format("%s: %s", variableName, literalString)); + if (expression.getRemainingExpression().isPresent()) { + remainingExpressions.add(expression.getRemainingExpression().get()); } } - else if (operator.equals(OperatorType.NOT_EQUAL)) { - if (literalType instanceof VarcharType) { - return new ClpExpression(String.format("NOT %s: \"%s\"", variableName, literalString)); + if (!hasDefinition) { + return new ClpExpression(node); + } + else if (!remainingExpressions.isEmpty()) { + if (remainingExpressions.size() == 1) { + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 5) + ")", remainingExpressions.get(0)); } else { - return new ClpExpression(String.format("NOT %s: %s", variableName, literalString)); + return new ClpExpression( + queryBuilder.substring(0, queryBuilder.length() - 5) + ")", + new SpecialFormExpression(node.getSourceLocation(), AND, BOOLEAN, remainingExpressions)); } } - else if (LOGICAL_BINARY_OPS_FILTER.contains(operator) && !(literalType instanceof VarcharType)) { - return new ClpExpression(String.format("%s %s %s", variableName, operator.getOperator(), literalString)); + // Remove the last " AND " from the query + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 5) + ")"); + } + + /** + * Handles the logical OR expression. + * Combines all fully convertible child expressions into a single KQL query joined by OR. + * Falls back to the original node if any child cannot be converted. + * Example: + * - col1 = 5 OR col1 = 10 → (col1: 5 OR col1: 10) + * + * @param node the OR special form expression + * @return a ClpExpression containing the OR-based KQL string, or the original expression if not fully convertible + */ + private ClpExpression handleOr(SpecialFormExpression node) + { + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append("("); + for (RowExpression argument : node.getArguments()) { + ClpExpression expression = argument.accept(this, null); + if (expression.getRemainingExpression().isPresent() || !expression.getDefinition().isPresent()) { + return new ClpExpression(node); + } + queryBuilder.append(expression.getDefinition().get()); + queryBuilder.append(" OR "); } - return new ClpExpression(originalNode); + // Remove the last " OR " from the query + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); } /** - * Handles logical binary operators (e.g., =, !=, <, >) between two expressions. - * Supports constant on either side by flipping the operator when needed. - * Also checks for SUBSTR(x, ...) = 'value' patterns and delegates to substring handler. + * Handles the IN predicate. + * Example: + * - col1 IN (1, 2, 3) → (col1: 1 OR col1: 2 OR col1: 3) + * + * @param node the IN special form expression + * @return a ClpExpression with the generated KQL query, or the original expression if unsupported */ - private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression node) + private ClpExpression handleIn(SpecialFormExpression node) { - if (node.getArguments().size() != 2) { + ClpExpression variable = node.getArguments().get(0).accept(this, null); + if (!variable.getDefinition().isPresent()) { + return new ClpExpression(node); + } + String variableName = variable.getDefinition().get(); + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append("("); + for (RowExpression argument : node.getArguments().subList(1, node.getArguments().size())) { + if (!(argument instanceof ConstantExpression)) { + return new ClpExpression(node); + } + ConstantExpression literal = (ConstantExpression) argument; + String literalString = getLiteralString(literal); + queryBuilder.append(variableName).append(": "); + if (literal.getType() instanceof VarcharType) { + queryBuilder.append("\"").append(literalString).append("\""); + } + else { + queryBuilder.append(literalString); + } + queryBuilder.append(" OR "); + } + // Remove the last " OR " from the query + return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); + } + + /** + * Handles the IS NULL predicate. + * Example: + * - col1 IS NULL → NOT col1: * + * + * @param node the IS_NULL special form expression + * @return a ClpExpression with the KQL query for null checking, or the original expression if unsupported + */ + private ClpExpression handleIsNull(SpecialFormExpression node) + { + if (node.getArguments().size() != 1) { throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, - "Logical binary operator must have exactly two arguments. Received: " + node); + "IS NULL operator must have exactly one argument. Received: " + node); } - RowExpression left = node.getArguments().get(0); - RowExpression right = node.getArguments().get(1); - ClpExpression maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right); - if (maybeLeftSubstring.getDefinition().isPresent()) { - return maybeLeftSubstring; + ClpExpression expression = node.getArguments().get(0).accept(this, null); + if (!expression.getDefinition().isPresent()) { + return new ClpExpression(node); } - ClpExpression maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left); - if (maybeRightSubstring.getDefinition().isPresent()) { - return maybeRightSubstring; + String variableName = expression.getDefinition().get(); + return new ClpExpression(format("NOT %s: *", variableName)); + } + + /** + * Handles dereference expressions on RowTypes (e.g., col.row_field). + * Converts nested row field access into dot-separated KQL-compatible field names. + * Example: + * - address.city (from a RowType 'address') → address.city + * + * @param expression the dereference expression (SpecialFormExpression or VariableReferenceExpression) + * @return a ClpExpression containing the dot-separated field name, or the original expression if unsupported + */ + private ClpExpression handleDereference(RowExpression expression) + { + if (expression instanceof VariableReferenceExpression) { + return expression.accept(this, null); } - ClpExpression leftExpression = left.accept(this, null); - ClpExpression rightExpression = right.accept(this, null); - Optional leftDefinition = leftExpression.getDefinition(); - Optional rightDefinition = rightExpression.getDefinition(); - if (!leftDefinition.isPresent() || !rightDefinition.isPresent()) { - return new ClpExpression(node); + if (!(expression instanceof SpecialFormExpression)) { + return new ClpExpression(expression); } - boolean leftIsConstant = (left instanceof ConstantExpression); - boolean rightIsConstant = (right instanceof ConstantExpression); + SpecialFormExpression specialForm = (SpecialFormExpression) expression; + List arguments = specialForm.getArguments(); + if (arguments.size() != 2) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE expects 2 arguments"); + } - Type leftType = left.getType(); - Type rightType = right.getType(); + RowExpression base = arguments.get(0); + RowExpression index = arguments.get(1); + if (!(index instanceof ConstantExpression)) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE index must be a constant"); + } - if (rightIsConstant) { - return buildClpExpression( - leftDefinition.get(), // variable - rightDefinition.get(), // literal - operator, - rightType, - node); + ConstantExpression constExpr = (ConstantExpression) index; + Object value = constExpr.getValue(); + if (!(value instanceof Long)) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE index constant is not a long"); } - else if (leftIsConstant) { - OperatorType newOperator = OperatorType.flip(operator); - return buildClpExpression( - rightDefinition.get(), // variable - leftDefinition.get(), // literal - newOperator, - leftType, - node); + + int fieldIndex = ((Long) value).intValue(); + + Type baseType = base.getType(); + if (!(baseType instanceof RowType)) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "DEREFERENCE base is not a RowType: " + baseType); + } + + RowType rowType = (RowType) baseType; + if (fieldIndex < 0 || fieldIndex >= rowType.getFields().size()) { + throw new PrestoException(CLP_PUSHDOWN_UNSUPPORTED_EXPRESSION, "Invalid field index " + fieldIndex + " for RowType: " + rowType); + } + + RowType.Field field = rowType.getFields().get(fieldIndex); + String fieldName = field.getName().orElse("field" + fieldIndex); + + ClpExpression baseString = handleDereference(base); + if (!baseString.getDefinition().isPresent()) { + return new ClpExpression(expression); + } + return new ClpExpression(baseString.getDefinition().get() + "." + fieldName); + } + + private static class SubstrInfo + { + String variableName; + RowExpression startExpression; + RowExpression lengthExpression; + + SubstrInfo(String variableName, RowExpression start, RowExpression length) + { + this.variableName = variableName; + this.startExpression = start; + this.lengthExpression = length; } - // fallback - return new ClpExpression(node); } } 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 51eb670fd9d39..1ad12b054cc99 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 @@ -26,27 +26,6 @@ public class TestClpFilterToKql extends TestClpQueryBase { - private void testFilter(String sqlExpression, Optional expectedKqlExpression, - Optional expectedRemainingExpression, SessionHolder sessionHolder) - { - RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); - ClpExpression clpExpression = pushDownExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); - Optional kqlExpression = clpExpression.getDefinition(); - Optional remainingExpression = clpExpression.getRemainingExpression(); - if (expectedKqlExpression.isPresent()) { - assertTrue(kqlExpression.isPresent()); - assertEquals(kqlExpression.get(), expectedKqlExpression.get()); - } - - if (expectedRemainingExpression.isPresent()) { - assertTrue(remainingExpression.isPresent()); - assertEquals(remainingExpression.get(), getRowExpression(expectedRemainingExpression.get(), sessionHolder)); - } - else { - assertFalse(remainingExpression.isPresent()); - } - } - @Test public void testStringMatchPushdown() { @@ -212,4 +191,24 @@ public void testComplexPushdown() Optional.of("lower(city.Region.Name) = 'hello world' OR city.Name IS NULL"), sessionHolder); } + + private void testFilter(String sqlExpression, Optional expectedKqlExpression, Optional expectedRemainingExpression, SessionHolder sessionHolder) + { + RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); + ClpExpression clpExpression = pushDownExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); + Optional kqlExpression = clpExpression.getDefinition(); + Optional remainingExpression = clpExpression.getRemainingExpression(); + if (expectedKqlExpression.isPresent()) { + assertTrue(kqlExpression.isPresent()); + assertEquals(kqlExpression.get(), expectedKqlExpression.get()); + } + + if (expectedRemainingExpression.isPresent()) { + assertTrue(remainingExpression.isPresent()); + assertEquals(remainingExpression.get(), getRowExpression(expectedRemainingExpression.get(), sessionHolder)); + } + else { + assertFalse(remainingExpression.isPresent()); + } + } } diff --git a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java index 7a5eebd751d4b..1b759ab3e1282 100644 --- a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java @@ -29,11 +29,9 @@ import com.facebook.presto.spi.ColumnHandle; import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.SchemaTableName; -import com.facebook.presto.spi.WarningCollector; import com.facebook.presto.spi.function.StandardFunctionResolution; import com.facebook.presto.spi.relation.RowExpression; import com.facebook.presto.spi.relation.VariableReferenceExpression; -import com.facebook.presto.sql.ExpressionUtils; import com.facebook.presto.sql.parser.ParsingOptions; import com.facebook.presto.sql.parser.SqlParser; import com.facebook.presto.sql.planner.TypeProvider; @@ -55,7 +53,10 @@ import static com.facebook.presto.common.type.VarcharType.VARCHAR; import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager; import static com.facebook.presto.metadata.SessionPropertyManager.createTestingSessionPropertyManager; +import static com.facebook.presto.spi.WarningCollector.NOOP; +import static com.facebook.presto.sql.ExpressionUtils.rewriteIdentifiersToSymbolReferences; import static com.facebook.presto.sql.analyzer.ExpressionAnalyzer.getExpressionTypes; +import static com.facebook.presto.sql.parser.ParsingOptions.DecimalLiteralTreatment.AS_DECIMAL; import static com.facebook.presto.testing.TestingConnectorSession.SESSION; import static com.facebook.presto.transaction.InMemoryTransactionManager.createTestTransactionManager; import static java.util.stream.Collectors.toMap; @@ -94,8 +95,8 @@ public class TestClpQueryBase public static Expression expression(String sql) { - return ExpressionUtils.rewriteIdentifiersToSymbolReferences( - new SqlParser().createExpression(sql, new ParsingOptions(ParsingOptions.DecimalLiteralTreatment.AS_DECIMAL))); + return rewriteIdentifiersToSymbolReferences( + new SqlParser().createExpression(sql, ParsingOptions.builder().setDecimalLiteralTreatment(AS_DECIMAL).build())); } protected RowExpression toRowExpression(Expression expression, TypeProvider typeProvider, Session session) @@ -107,7 +108,7 @@ protected RowExpression toRowExpression(Expression expression, TypeProvider type typeProvider, expression, ImmutableMap.of(), - WarningCollector.NOOP); + NOOP); return SqlToRowExpressionTranslator.translate(expression, expressionTypes, ImmutableMap.of(), functionAndTypeManager, session); } From b6ee2ea0eb9e720dd9bf2f80f8e0c31d2f3a215d Mon Sep 17 00:00:00 2001 From: wraymo Date: Fri, 20 Jun 2025 15:05:50 -0400 Subject: [PATCH 3/7] fix inconsistent variable name and remove Optional from test cases --- .../clp/split/ClpMySqlSplitProvider.java | 4 +- .../presto/plugin/clp/ClpMetadataDbSetUp.java | 8 +- .../presto/plugin/clp/TestClpFilterToKql.java | 138 +++++++++--------- 3 files changed, 75 insertions(+), 75 deletions(-) diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/split/ClpMySqlSplitProvider.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/split/ClpMySqlSplitProvider.java index ac646a061ba4b..964a992230e36 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/split/ClpMySqlSplitProvider.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/split/ClpMySqlSplitProvider.java @@ -38,10 +38,10 @@ public class ClpMySqlSplitProvider public static final String ARCHIVES_TABLE_COLUMN_ID = "id"; // Table suffixes - public static final String ARCHIVE_TABLE_SUFFIX = "_archives"; + public static final String ARCHIVES_TABLE_SUFFIX = "_archives"; // SQL templates - private static final String SQL_SELECT_ARCHIVES_TEMPLATE = format("SELECT `%s` FROM `%%s%%s%s`", ARCHIVES_TABLE_COLUMN_ID, ARCHIVE_TABLE_SUFFIX); + private static final String SQL_SELECT_ARCHIVES_TEMPLATE = format("SELECT `%s` FROM `%%s%%s%s`", ARCHIVES_TABLE_COLUMN_ID, ARCHIVES_TABLE_SUFFIX); private static final Logger log = Logger.get(ClpMySqlSplitProvider.class); diff --git a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/ClpMetadataDbSetUp.java b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/ClpMetadataDbSetUp.java index 6b7220cc1dd43..cebe4afcfa0a7 100644 --- a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/ClpMetadataDbSetUp.java +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/ClpMetadataDbSetUp.java @@ -38,7 +38,7 @@ import static com.facebook.presto.plugin.clp.metadata.ClpMySqlMetadataProvider.DATASETS_TABLE_COLUMN_NAME; import static com.facebook.presto.plugin.clp.metadata.ClpMySqlMetadataProvider.DATASETS_TABLE_SUFFIX; import static com.facebook.presto.plugin.clp.split.ClpMySqlSplitProvider.ARCHIVES_TABLE_COLUMN_ID; -import static com.facebook.presto.plugin.clp.split.ClpMySqlSplitProvider.ARCHIVE_TABLE_SUFFIX; +import static com.facebook.presto.plugin.clp.split.ClpMySqlSplitProvider.ARCHIVES_TABLE_SUFFIX; import static java.lang.String.format; import static java.util.UUID.randomUUID; import static org.testng.Assert.fail; @@ -53,7 +53,7 @@ public final class ClpMetadataDbSetUp private static final Logger log = Logger.get(ClpMetadataDbSetUp.class); private static final String DATASETS_TABLE_NAME = METADATA_DB_TABLE_PREFIX + DATASETS_TABLE_SUFFIX; - private static final String ARCHIVE_TABLE_COLUMN_PAGINATION_ID = "pagination_id"; + private static final String ARCHIVES_TABLE_COLUMN_PAGINATION_ID = "pagination_id"; private ClpMetadataDbSetUp() { @@ -122,7 +122,7 @@ public static ClpMetadata setupMetadata(DbHandle dbHandle, Map> splits) { final String metadataDbUrl = format(METADATA_DB_URL_TEMPLATE, dbHandle.dbPath); - final String archiveTableFormat = METADATA_DB_TABLE_PREFIX + "%s" + ARCHIVE_TABLE_SUFFIX; + final String archiveTableFormat = METADATA_DB_TABLE_PREFIX + "%s" + ARCHIVES_TABLE_SUFFIX; try (Connection conn = DriverManager.getConnection(metadataDbUrl, METADATA_DB_USER, METADATA_DB_PASSWORD); Statement stmt = conn.createStatement()) { createDatasetsTable(stmt); @@ -138,7 +138,7 @@ public static ClpMySqlSplitProvider setupSplit(DbHandle dbHandle, Map 0", Optional.of("fare > 0"), Optional.empty(), sessionHolder); - testFilter("fare >= 0", Optional.of("fare >= 0"), Optional.empty(), sessionHolder); - testFilter("fare < 0", Optional.of("fare < 0"), Optional.empty(), sessionHolder); - testFilter("fare <= 0", Optional.of("fare <= 0"), Optional.empty(), sessionHolder); - testFilter("fare = 0", Optional.of("fare: 0"), Optional.empty(), sessionHolder); - testFilter("fare != 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); - testFilter("fare <> 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); - testFilter("0 < fare", Optional.of("fare > 0"), Optional.empty(), sessionHolder); - testFilter("0 <= fare", Optional.of("fare >= 0"), Optional.empty(), sessionHolder); - testFilter("0 > fare", Optional.of("fare < 0"), Optional.empty(), sessionHolder); - testFilter("0 >= fare", Optional.of("fare <= 0"), Optional.empty(), sessionHolder); - testFilter("0 = fare", Optional.of("fare: 0"), Optional.empty(), sessionHolder); - testFilter("0 != fare", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); - testFilter("0 <> fare", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("fare > 0", "fare > 0", null, sessionHolder); + testFilter("fare >= 0", "fare >= 0", null, sessionHolder); + testFilter("fare < 0", "fare < 0", null, sessionHolder); + testFilter("fare <= 0", "fare <= 0", null, sessionHolder); + testFilter("fare = 0", "fare: 0", null, sessionHolder); + testFilter("fare != 0", "NOT fare: 0", null, sessionHolder); + testFilter("fare <> 0", "NOT fare: 0", null, sessionHolder); + testFilter("0 < fare", "fare > 0", null, sessionHolder); + testFilter("0 <= fare", "fare >= 0", null, sessionHolder); + testFilter("0 > fare", "fare < 0", null, sessionHolder); + testFilter("0 >= fare", "fare <= 0", null, sessionHolder); + testFilter("0 = fare", "fare: 0", null, sessionHolder); + testFilter("0 != fare", "NOT fare: 0", null, sessionHolder); + testFilter("0 <> fare", "NOT fare: 0", null, sessionHolder); } @Test @@ -92,23 +92,23 @@ public void testOrPushdown() { SessionHolder sessionHolder = new SessionHolder(); - testFilter("fare > 0 OR city.Name like 'b%'", Optional.of("(fare > 0 OR city.Name: \"b*\")"), Optional.empty(), sessionHolder); + testFilter("fare > 0 OR city.Name like 'b%'", "(fare > 0 OR city.Name: \"b*\")", null, sessionHolder); testFilter( "lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1", - Optional.empty(), - Optional.of("(lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1)"), + null, + "(lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1)", sessionHolder); // Multiple ORs testFilter( "fare > 0 OR city.Name like 'b%' OR lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1", - Optional.empty(), - Optional.of("fare > 0 OR city.Name like 'b%' OR lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1"), + null, + "fare > 0 OR city.Name like 'b%' OR lower(city.Region.Name) = 'hello world' OR city.Region.Id != 1", sessionHolder); testFilter( "fare > 0 OR city.Name like 'b%' OR city.Region.Id != 1", - Optional.of("((fare > 0 OR city.Name: \"b*\") OR NOT city.Region.Id: 1)"), - Optional.empty(), + "((fare > 0 OR city.Name: \"b*\") OR NOT city.Region.Id: 1)", + null, sessionHolder); } @@ -117,23 +117,23 @@ public void testAndPushdown() { SessionHolder sessionHolder = new SessionHolder(); - testFilter("fare > 0 AND city.Name like 'b%'", Optional.of("(fare > 0 AND city.Name: \"b*\")"), Optional.empty(), sessionHolder); + testFilter("fare > 0 AND city.Name like 'b%'", "(fare > 0 AND city.Name: \"b*\")", null, sessionHolder); testFilter( "lower(city.Region.Name) = 'hello world' AND city.Region.Id != 1", - Optional.of("(NOT city.Region.Id: 1)"), - Optional.of("lower(city.Region.Name) = 'hello world'"), + "(NOT city.Region.Id: 1)", + "lower(city.Region.Name) = 'hello world'", sessionHolder); // Multiple ANDs testFilter( "fare > 0 AND city.Name like 'b%' AND lower(city.Region.Name) = 'hello world' AND city.Region.Id != 1", - Optional.of("(((fare > 0 AND city.Name: \"b*\")) AND NOT city.Region.Id: 1)"), - Optional.of("(lower(city.Region.Name) = 'hello world')"), + "(((fare > 0 AND city.Name: \"b*\")) AND NOT city.Region.Id: 1)", + "(lower(city.Region.Name) = 'hello world')", sessionHolder); testFilter( "fare > 0 AND city.Name like '%b%' AND lower(city.Region.Name) = 'hello world' AND city.Region.Id != 1", - Optional.of("(((fare > 0)) AND NOT city.Region.Id: 1)"), - Optional.of("city.Name like '%b%' AND lower(city.Region.Name) = 'hello world'"), + "(((fare > 0)) AND NOT city.Region.Id: 1)", + "city.Name like '%b%' AND lower(city.Region.Name) = 'hello world'", sessionHolder); } @@ -142,19 +142,19 @@ public void testNotPushdown() { SessionHolder sessionHolder = new SessionHolder(); - testFilter("city.Region.Name NOT LIKE 'hello%'", Optional.of("NOT city.Region.Name: \"hello*\""), Optional.empty(), sessionHolder); - testFilter("NOT (city.Region.Name LIKE 'hello%')", Optional.of("NOT city.Region.Name: \"hello*\""), Optional.empty(), sessionHolder); - testFilter("city.Name != 'hello world'", Optional.of("NOT city.Name: \"hello world\""), Optional.empty(), sessionHolder); - testFilter("city.Name <> 'hello world'", Optional.of("NOT city.Name: \"hello world\""), Optional.empty(), sessionHolder); - testFilter("NOT (city.Name = 'hello world')", Optional.of("NOT city.Name: \"hello world\""), Optional.empty(), sessionHolder); - testFilter("fare != 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); - testFilter("fare <> 0", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); - testFilter("NOT (fare = 0)", Optional.of("NOT fare: 0"), Optional.empty(), sessionHolder); + testFilter("city.Region.Name NOT LIKE 'hello%'", "NOT city.Region.Name: \"hello*\"", null, sessionHolder); + testFilter("NOT (city.Region.Name LIKE 'hello%')", "NOT city.Region.Name: \"hello*\"", null, sessionHolder); + testFilter("city.Name != 'hello world'", "NOT city.Name: \"hello world\"", null, sessionHolder); + testFilter("city.Name <> 'hello world'", "NOT city.Name: \"hello world\"", null, sessionHolder); + testFilter("NOT (city.Name = 'hello world')", "NOT city.Name: \"hello world\"", null, sessionHolder); + testFilter("fare != 0", "NOT fare: 0", null, sessionHolder); + testFilter("fare <> 0", "NOT fare: 0", null, sessionHolder); + testFilter("NOT (fare = 0)", "NOT fare: 0", null, sessionHolder); // Multiple NOTs - testFilter("NOT (NOT fare = 0)", Optional.of("NOT NOT fare: 0"), Optional.empty(), sessionHolder); - testFilter("NOT (fare = 0 AND city.Name = 'hello world')", Optional.of("NOT (fare: 0 AND city.Name: \"hello world\")"), Optional.empty(), sessionHolder); - testFilter("NOT (fare = 0 OR city.Name = 'hello world')", Optional.of("NOT (fare: 0 OR city.Name: \"hello world\")"), Optional.empty(), sessionHolder); + testFilter("NOT (NOT fare = 0)", "NOT NOT fare: 0", null, sessionHolder); + testFilter("NOT (fare = 0 AND city.Name = 'hello world')", "NOT (fare: 0 AND city.Name: \"hello world\")", null, sessionHolder); + testFilter("NOT (fare = 0 OR city.Name = 'hello world')", "NOT (fare: 0 OR city.Name: \"hello world\")", null, sessionHolder); } @Test @@ -162,7 +162,7 @@ public void testInPushdown() { SessionHolder sessionHolder = new SessionHolder(); - testFilter("city.Name IN ('hello world', 'hello world 2')", Optional.of("(city.Name: \"hello world\" OR city.Name: \"hello world 2\")"), Optional.empty(), sessionHolder); + testFilter("city.Name IN ('hello world', 'hello world 2')", "(city.Name: \"hello world\" OR city.Name: \"hello world 2\")", null, sessionHolder); } @Test @@ -170,9 +170,9 @@ public void testIsNullPushdown() { SessionHolder sessionHolder = new SessionHolder(); - testFilter("city.Name IS NULL", Optional.of("NOT city.Name: *"), Optional.empty(), sessionHolder); - testFilter("city.Name IS NOT NULL", Optional.of("NOT NOT city.Name: *"), Optional.empty(), sessionHolder); - testFilter("NOT (city.Name IS NULL)", Optional.of("NOT NOT city.Name: *"), Optional.empty(), sessionHolder); + testFilter("city.Name IS NULL", "NOT city.Name: *", null, sessionHolder); + testFilter("city.Name IS NOT NULL", "NOT NOT city.Name: *", null, sessionHolder); + testFilter("NOT (city.Name IS NULL)", "NOT NOT city.Name: *", null, sessionHolder); } @Test @@ -182,30 +182,30 @@ public void testComplexPushdown() testFilter( "(fare > 0 OR city.Name like 'b%') AND (lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)", - Optional.of("((fare > 0 OR city.Name: \"b*\"))"), - Optional.of("(lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)"), + "((fare > 0 OR city.Name: \"b*\"))", + "(lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)", sessionHolder); testFilter( "city.Region.Id = 1 AND (fare > 0 OR city.Name NOT like 'b%') AND (lower(city.Region.Name) = 'hello world' OR city.Name IS NULL)", - Optional.of("((city.Region.Id: 1 AND (fare > 0 OR NOT city.Name: \"b*\")))"), - Optional.of("lower(city.Region.Name) = 'hello world' OR city.Name IS NULL"), + "((city.Region.Id: 1 AND (fare > 0 OR NOT city.Name: \"b*\")))", + "lower(city.Region.Name) = 'hello world' OR city.Name IS NULL", sessionHolder); } - private void testFilter(String sqlExpression, Optional expectedKqlExpression, Optional expectedRemainingExpression, SessionHolder sessionHolder) + private void testFilter(String sqlExpression, String expectedKqlExpression, String expectedRemainingExpression, SessionHolder sessionHolder) { RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); ClpExpression clpExpression = pushDownExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); Optional kqlExpression = clpExpression.getDefinition(); Optional remainingExpression = clpExpression.getRemainingExpression(); - if (expectedKqlExpression.isPresent()) { + if (expectedKqlExpression != null) { assertTrue(kqlExpression.isPresent()); - assertEquals(kqlExpression.get(), expectedKqlExpression.get()); + assertEquals(kqlExpression.get(), expectedKqlExpression); } - if (expectedRemainingExpression.isPresent()) { + if (expectedRemainingExpression != null) { assertTrue(remainingExpression.isPresent()); - assertEquals(remainingExpression.get(), getRowExpression(expectedRemainingExpression.get(), sessionHolder)); + assertEquals(remainingExpression.get(), getRowExpression(expectedRemainingExpression, sessionHolder)); } else { assertFalse(remainingExpression.isPresent()); From 544ab5264388e53d158eed005fb6910aad1a6b5c Mon Sep 17 00:00:00 2001 From: wraymo Date: Mon, 23 Jun 2025 09:50:04 -0400 Subject: [PATCH 4/7] apply review suggestions --- .../presto/plugin/clp/ClpExpression.java | 28 ++- .../plugin/clp/ClpFilterToKqlConverter.java | 175 ++++++++++-------- .../presto/plugin/clp/ClpPlanOptimizer.java | 10 +- .../plugin/clp/ClpPlanOptimizerProvider.java | 3 +- .../presto/plugin/clp/TestClpFilterToKql.java | 2 +- 5 files changed, 117 insertions(+), 101 deletions(-) diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java index ee60ff1f38555..f90e4aa0e1cb1 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java @@ -18,25 +18,21 @@ import java.util.Optional; /** - * Represents the result of converting a Presto RowExpression into a CLP-compatible KQL query. - * There are three possible cases: - * 1. The entire RowExpression is convertible to KQL: `definition` is set, `remainingExpression` is empty. - * 2. Part of the RowExpression is convertible: the KQL part is stored in `definition`, - * and the remaining untranslatable part is stored in `remainingExpression`. - * 3. None of the expression is convertible: the full RowExpression is stored in `remainingExpression`, - * and `definition` is empty. + * Represents the result of converting a Presto RowExpression into a CLP-compatible KQL query. In + * every case, `kqlQuery` represents the part of the RowExpression that could be converted to a + * KQL expression, and `remainingExpression` represents the part that could not be converted. */ public class ClpExpression { // Optional KQL query string representing the fully or partially translatable part of the expression. - private final Optional definition; + private final Optional kqlQuery; // The remaining (non-translatable) portion of the RowExpression, if any. private final Optional remainingExpression; - public ClpExpression(String definition, RowExpression remainingExpression) + public ClpExpression(String kqlQuery, RowExpression remainingExpression) { - this.definition = Optional.ofNullable(definition); + this.kqlQuery = Optional.ofNullable(kqlQuery); this.remainingExpression = Optional.ofNullable(remainingExpression); } @@ -50,15 +46,17 @@ public ClpExpression() /** * Creates a ClpExpression from a fully translatable KQL string. - * @param definition + * + * @param kqlQuery */ - public ClpExpression(String definition) + public ClpExpression(String kqlQuery) { - this(definition, null); + this(kqlQuery, null); } /** * Creates a ClpExpression from a non-translatable RowExpression. + * * @param remainingExpression */ public ClpExpression(RowExpression remainingExpression) @@ -66,9 +64,9 @@ public ClpExpression(RowExpression remainingExpression) this(null, remainingExpression); } - public Optional getDefinition() + public Optional getKqlQuery() { - return definition; + return kqlQuery; } public Optional getRemainingExpression() 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 49ee7ce0ba0d5..20570632af17f 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 @@ -55,20 +55,25 @@ import static java.util.Objects.requireNonNull; /** - * ClpFilterToKqlConverter translates Presto RowExpressions into KQL (Kibana Query Language) filters - * used as CLP queries. This is used primarily for pushing down supported filters to the CLP engine. - * This class implements the RowExpressionVisitor interface and recursively walks Presto filter expressions, - * attempting to convert supported expressions (e.g., comparisons, logical AND/OR, LIKE, IN, IS NULL, - * and SUBSTR-based expressions) into corresponding KQL filter strings. Any part of the expression that - * cannot be translated is preserved as a "remaining expression" for potential fallback processing. + * A translator to translate Presto RowExpressions into KQL (Kibana Query Language) filters used as + * CLP queries. This is used primarily for pushing down supported filters to the CLP engine. This + * class implements the RowExpressionVisitor interface and recursively walks Presto filter + * expressions, attempting to convert supported expressions into corresponding KQL filter strings. + * Any part of the expression that cannot be translated is preserved as a "remaining expression" for + * potential fallback processing. + *

* Supported translations include: - * - Variable-to-literal comparisons (e.g., =, !=, <, >, <=, >=) - * - String pattern matches using LIKE - * - Membership checks using IN - * - NULL checks via IS NULL - * - Substring comparisons (e.g., SUBSTR(x, start, len) = "val") mapped to wildcard KQL queries - * - Dereferencing fields from row-typed variables - * - Logical operators AND, OR, and NOT + *
    + *
  • Comparisons between variables and constants (e.g., =, !=, <, >, <=, >=).
    • + *
  • String pattern matches using LIKE with constant patterns only. "^%[^%_]*%$" + * is not supported.
    • + *
  • Membership checks using IN with a list of constants only
    • + *
  • NULL checks via IS NULL
    • + *
  • Substring comparisons (e.g., SUBSTR(x, start, len) = "val") are supported + * only when compared against a constant.
    • + *
  • Dereferencing fields from row-typed variables
    • + *
  • Logical operators AND, OR, and NOT
    • + *
*/ public class ClpFilterToKqlConverter implements RowExpressionVisitor @@ -179,11 +184,12 @@ private String getVariableName(VariableReferenceExpression variable) /** * Handles the logical NOT expression. - * Example: - * - NOT (col1 = 5) → NOT col1: 5 + *

+ * Example: NOT (col1 = 5) → NOT col1: 5 * * @param node the NOT call expression - * @return a ClpExpression with the translated KQL query, or the original expression if unsupported + * @return a ClpExpression containing the equivalent KQL query or the original expression if it + * couldn't be translated */ private ClpExpression handleNot(CallExpression node) { @@ -194,21 +200,23 @@ private ClpExpression handleNot(CallExpression node) RowExpression input = node.getArguments().get(0); ClpExpression expression = input.accept(this, null); - if (expression.getRemainingExpression().isPresent() || !expression.getDefinition().isPresent()) { + if (expression.getRemainingExpression().isPresent() || !expression.getKqlQuery().isPresent()) { return new ClpExpression(node); } - return new ClpExpression("NOT " + expression.getDefinition().get()); + return new ClpExpression("NOT " + expression.getKqlQuery().get()); } /** * Handles LIKE expressions. - * Converts SQL LIKE patterns into equivalent KQL queries using * (for %) and ? (for _). - * Only supports constant patterns or constant cast patterns. - * Example: - * - col1 LIKE 'a_bc%' → col1: "a?bc*" + *

+ * Converts SQL LIKE patterns into equivalent KQL queries using * (for %) + * and ? (for _). Only supports constant or casted constant patterns. + *

+ * Example: col1 LIKE 'a_bc%' → col1: "a?bc*" * * @param node the LIKE call expression - * @return a ClpExpression with the KQL equivalent of the LIKE expression, or the original node if unsupported + * @return a ClpExpression containing the equivalent KQL query, or the original expression if it + * couldn't be translated */ private ClpExpression handleLike(CallExpression node) { @@ -216,11 +224,11 @@ private ClpExpression handleLike(CallExpression node) 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); - if (!variable.getDefinition().isPresent()) { + if (!variable.getKqlQuery().isPresent()) { return new ClpExpression(node); } - String variableName = variable.getDefinition().get(); + String variableName = variable.getKqlQuery().get(); RowExpression argument = node.getArguments().get(1); String pattern; @@ -249,14 +257,15 @@ else if (argument instanceof CallExpression) { } /** - * Handles logical binary operators (e.g., =, !=, <, >) between two expressions. - * Supports constant on either side by flipping the operator when needed. - * Also checks for SUBSTR(x, ...) = 'value' patterns and delegates to substring handler. - * If the expression cannot be translated, it returns a fallback expression that preserves the original node. + * Handles logical binary operators (e.g., =, !=, <, >) between two expressions. + *

+ * Supports constant values on either side and flips the operator if necessary. Also delegates to a + * substring handler for SUBSTR(x, ...) = 'value' patterns. * - * @param operator the logical binary operator (e.g., EQUAL, NOT_EQUAL, LESS_THAN) + * @param operator the binary operator (e.g., EQUAL, NOT_EQUAL) * @param node the call expression representing the binary operation - * @return an expression with a KQL query if possible, otherwise a fallback expression + * @return a ClpExpression containing the equivalent KQL query or the original expression if it + * couldn't be translated */ private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression node) { @@ -268,19 +277,19 @@ private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression RowExpression right = node.getArguments().get(1); ClpExpression maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right); - if (maybeLeftSubstring.getDefinition().isPresent()) { + if (maybeLeftSubstring.getKqlQuery().isPresent()) { return maybeLeftSubstring; } ClpExpression maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left); - if (maybeRightSubstring.getDefinition().isPresent()) { + if (maybeRightSubstring.getKqlQuery().isPresent()) { return maybeRightSubstring; } ClpExpression leftExpression = left.accept(this, null); ClpExpression rightExpression = right.accept(this, null); - Optional leftDefinition = leftExpression.getDefinition(); - Optional rightDefinition = rightExpression.getDefinition(); + Optional leftDefinition = leftExpression.getKqlQuery(); + Optional rightDefinition = rightExpression.getKqlQuery(); if (!leftDefinition.isPresent() || !rightDefinition.isPresent()) { return new ClpExpression(node); } @@ -313,20 +322,25 @@ else if (leftIsConstant) { } /** - * Builds a CLP expression from a basic comparison between a variable and a literal. - * Handles different operator types (EQUAL, NOT_EQUAL, and logical binary ops like <, >, etc.) - * and formats them appropriately based on whether the literal is a string or a non-string type. + * Builds a CLP expression from a basic comparison between a variable and a constant. + *

+ * Handles different operator types and formats them appropriately based on whether the literal + * is a string or a non-string type. + *

* Examples: - * - col = 'abc' → col: "abc" - * - col != 42 → NOT col: 42 - * - 5 < col → col > 5 + *
    + *
  • col = 'abc' → col: "abc"
    • + *
  • col != 42 → NOT col: 42
    • + *
  • 5 < col → col > 5
    • + *
* * @param variableName name of the variable * @param literalString string representation of the literal - * @param operator operator used in the comparison - * @param literalType type of the literal + * @param operator the comparison operator + * @param literalType the type of the literal * @param originalNode the original RowExpression node - * @return a ClpExpression containing the KQL filter or fallback to the original node if unsupported + * @return a ClpExpression containing the equivalent KQL query or the original expression if it + * couldn't be translated */ private ClpExpression buildClpExpression( String variableName, @@ -390,7 +404,7 @@ private ClpExpression tryInterpretSubstringEquality( } /** - * Parses a SUBSTR(x, start [, length]) call into a SubstrInfo object if valid. + * Parses a SUBSTR(x, start [, length]) call into a SubstrInfo object if valid. * * @param callExpression the call expression to inspect * @return an Optional containing SubstrInfo if the expression is a valid SUBSTR call, otherwise empty @@ -409,11 +423,11 @@ private Optional parseSubstringCall(CallExpression callExpression) } ClpExpression variable = callExpression.getArguments().get(0).accept(this, null); - if (!variable.getDefinition().isPresent()) { + if (!variable.getKqlQuery().isPresent()) { return Optional.empty(); } - String varName = variable.getDefinition().get(); + String varName = variable.getKqlQuery().get(); RowExpression startExpression = callExpression.getArguments().get(1); RowExpression lengthExpression = null; if (argCount == 3) { @@ -424,12 +438,15 @@ private Optional parseSubstringCall(CallExpression callExpression) } /** - * Converts a SUBSTR(x, start [, length]) = 'someString' into a KQL-style wildcard query. + * Converts a SUBSTR(x, start [, length]) = 'someString' into a KQL-style wildcard query. + *

* Examples: - * - SUBSTR(message, 1, 3) = 'abc' → message: "abc*" - * - SUBSTR(message, 4, 3) = 'abc' → message: "???abc*" - * - SUBSTR(message, 2) = 'hello' → message: "?hello" - * - SUBSTR(message, -5) = 'hello' → message: "*hello" + *
    + *
  • SUBSTR(message, 1, 3) = 'abc' → message: "abc*"
    • + *
  • SUBSTR(message, 4, 3) = 'abc' → message: "???abc*"
    • + *
  • SUBSTR(message, 2) = 'hello' → message: "?hello"
    • + *
  • SUBSTR(message, -5) = 'hello' → message: "*hello"
    • + *
* * @param info parsed SUBSTR call info * @param targetString the literal string being compared to @@ -535,10 +552,11 @@ private Optional parseLengthLiteral(RowExpression lengthExpression, Str /** * Handles the logical AND expression. + *

* Combines all definable child expressions into a single KQL query joined by AND. * Any unsupported children are collected into a remaining expression. - * Example: - * - col1 = 5 AND col2 = 'abc' → (col1: 5 AND col2: "abc") + *

+ * Example: col1 = 5 AND col2 = 'abc' → (col1: 5 AND col2: "abc") * * @param node the AND special form expression * @return a ClpExpression containing the KQL query and any remaining sub-expressions @@ -551,9 +569,9 @@ private ClpExpression handleAnd(SpecialFormExpression node) boolean hasDefinition = false; for (RowExpression argument : node.getArguments()) { ClpExpression expression = argument.accept(this, null); - if (expression.getDefinition().isPresent()) { + if (expression.getKqlQuery().isPresent()) { hasDefinition = true; - queryBuilder.append(expression.getDefinition().get()); + queryBuilder.append(expression.getKqlQuery().get()); queryBuilder.append(" AND "); } if (expression.getRemainingExpression().isPresent()) { @@ -579,10 +597,11 @@ else if (!remainingExpressions.isEmpty()) { /** * Handles the logical OR expression. + *

* Combines all fully convertible child expressions into a single KQL query joined by OR. * Falls back to the original node if any child cannot be converted. - * Example: - * - col1 = 5 OR col1 = 10 → (col1: 5 OR col1: 10) + *

+ * Example: col1 = 5 OR col1 = 10 → (col1: 5 OR col1: 10) * * @param node the OR special form expression * @return a ClpExpression containing the OR-based KQL string, or the original expression if not fully convertible @@ -593,10 +612,10 @@ private ClpExpression handleOr(SpecialFormExpression node) queryBuilder.append("("); for (RowExpression argument : node.getArguments()) { ClpExpression expression = argument.accept(this, null); - if (expression.getRemainingExpression().isPresent() || !expression.getDefinition().isPresent()) { + if (expression.getRemainingExpression().isPresent() || !expression.getKqlQuery().isPresent()) { return new ClpExpression(node); } - queryBuilder.append(expression.getDefinition().get()); + queryBuilder.append(expression.getKqlQuery().get()); queryBuilder.append(" OR "); } // Remove the last " OR " from the query @@ -605,19 +624,20 @@ private ClpExpression handleOr(SpecialFormExpression node) /** * Handles the IN predicate. - * Example: - * - col1 IN (1, 2, 3) → (col1: 1 OR col1: 2 OR col1: 3) + *

+ * Example: col1 IN (1, 2, 3) → (col1: 1 OR col1: 2 OR col1: 3) * * @param node the IN special form expression - * @return a ClpExpression with the generated KQL query, or the original expression if unsupported + * @return a ClpExpression containing the equivalent KQL query or the original expression if it + * couldn't be translated */ private ClpExpression handleIn(SpecialFormExpression node) { ClpExpression variable = node.getArguments().get(0).accept(this, null); - if (!variable.getDefinition().isPresent()) { + if (!variable.getKqlQuery().isPresent()) { return new ClpExpression(node); } - String variableName = variable.getDefinition().get(); + String variableName = variable.getKqlQuery().get(); StringBuilder queryBuilder = new StringBuilder(); queryBuilder.append("("); for (RowExpression argument : node.getArguments().subList(1, node.getArguments().size())) { @@ -641,11 +661,12 @@ private ClpExpression handleIn(SpecialFormExpression node) /** * Handles the IS NULL predicate. - * Example: - * - col1 IS NULL → NOT col1: * + *

+ * Example: col1 IS NULL → NOT col1: * * * @param node the IS_NULL special form expression - * @return a ClpExpression with the KQL query for null checking, or the original expression if unsupported + * @return a ClpExpression containing the equivalent KQL query or the original expression if it + * couldn't be translated */ private ClpExpression handleIsNull(SpecialFormExpression node) { @@ -655,22 +676,24 @@ private ClpExpression handleIsNull(SpecialFormExpression node) } ClpExpression expression = node.getArguments().get(0).accept(this, null); - if (!expression.getDefinition().isPresent()) { + if (!expression.getKqlQuery().isPresent()) { return new ClpExpression(node); } - String variableName = expression.getDefinition().get(); + String variableName = expression.getKqlQuery().get(); return new ClpExpression(format("NOT %s: *", variableName)); } /** - * Handles dereference expressions on RowTypes (e.g., col.row_field). + * Handles dereference expressions on RowTypes (e.g., col.row_field). + *

* Converts nested row field access into dot-separated KQL-compatible field names. - * Example: - * - address.city (from a RowType 'address') → address.city + *

+ * Example: address.city (from a RowType 'address') → address.city * * @param expression the dereference expression (SpecialFormExpression or VariableReferenceExpression) - * @return a ClpExpression containing the dot-separated field name, or the original expression if unsupported + * @return a ClpExpression containing the dot-separated field name or the original expression if it + * couldn't be translated */ private ClpExpression handleDereference(RowExpression expression) { @@ -716,10 +739,10 @@ private ClpExpression handleDereference(RowExpression expression) String fieldName = field.getName().orElse("field" + fieldIndex); ClpExpression baseString = handleDereference(base); - if (!baseString.getDefinition().isPresent()) { + if (!baseString.getKqlQuery().isPresent()) { return new ClpExpression(expression); } - return new ClpExpression(baseString.getDefinition().get() + "." + fieldName); + return new ClpExpression(baseString.getKqlQuery().get() + "." + fieldName); } private static class SubstrInfo 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 bfa76edce63b5..8916b73d74cbe 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 @@ -42,18 +42,14 @@ public class ClpPlanOptimizer private final FunctionMetadataManager functionManager; private final StandardFunctionResolution functionResolution; - public ClpPlanOptimizer(FunctionMetadataManager functionManager, - StandardFunctionResolution functionResolution) + public ClpPlanOptimizer(FunctionMetadataManager functionManager, StandardFunctionResolution functionResolution) { this.functionManager = requireNonNull(functionManager, "functionManager is null"); this.functionResolution = requireNonNull(functionResolution, "functionResolution is null"); } @Override - public PlanNode optimize(PlanNode maxSubplan, - ConnectorSession session, - VariableAllocator variableAllocator, - PlanNodeIdAllocator idAllocator) + public PlanNode optimize(PlanNode maxSubplan, ConnectorSession session, VariableAllocator variableAllocator, PlanNodeIdAllocator idAllocator) { return rewriteWith(new Rewriter(idAllocator), maxSubplan); } @@ -81,7 +77,7 @@ public PlanNode visitFilter(FilterNode node, RewriteContext context) ClpTableHandle clpTableHandle = (ClpTableHandle) tableHandle.getConnectorHandle(); ClpExpression clpExpression = node.getPredicate() .accept(new ClpFilterToKqlConverter(functionResolution, functionManager, assignments), null); - Optional kqlQuery = clpExpression.getDefinition(); + Optional kqlQuery = clpExpression.getKqlQuery(); Optional remainingPredicate = clpExpression.getRemainingExpression(); if (!kqlQuery.isPresent()) { return node; diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java index 8dc884acc2682..f6f166eb7f657 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizerProvider.java @@ -30,8 +30,7 @@ public class ClpPlanOptimizerProvider private final StandardFunctionResolution functionResolution; @Inject - public ClpPlanOptimizerProvider(FunctionMetadataManager functionManager, - StandardFunctionResolution functionResolution) + public ClpPlanOptimizerProvider(FunctionMetadataManager functionManager, StandardFunctionResolution functionResolution) { this.functionManager = functionManager; this.functionResolution = functionResolution; 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 d40ef77c6eba5..04ca590451f53 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 @@ -196,7 +196,7 @@ private void testFilter(String sqlExpression, String expectedKqlExpression, Stri { RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); ClpExpression clpExpression = pushDownExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); - Optional kqlExpression = clpExpression.getDefinition(); + Optional kqlExpression = clpExpression.getKqlQuery(); Optional remainingExpression = clpExpression.getRemainingExpression(); if (expectedKqlExpression != null) { assertTrue(kqlExpression.isPresent()); From 7dcc7fc7b3e0f788b7509139ff519008d2fe1e1b Mon Sep 17 00:00:00 2001 From: wraymo Date: Mon, 23 Jun 2025 14:43:50 -0400 Subject: [PATCH 5/7] rename definition to pushDownExpression in ClpExpression --- .../presto/plugin/clp/ClpExpression.java | 22 ++++----- .../plugin/clp/ClpFilterToKqlConverter.java | 46 +++++++++---------- .../presto/plugin/clp/ClpPlanOptimizer.java | 2 +- .../presto/plugin/clp/TestClpFilterToKql.java | 6 +-- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java index f90e4aa0e1cb1..e22ccb3b8a6a9 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java @@ -24,20 +24,20 @@ */ public class ClpExpression { - // Optional KQL query string representing the fully or partially translatable part of the expression. - private final Optional kqlQuery; + // Optional KQL query or column name representing the fully or partially translatable part of the expression. + private final Optional pushDownExpression; // The remaining (non-translatable) portion of the RowExpression, if any. private final Optional remainingExpression; - public ClpExpression(String kqlQuery, RowExpression remainingExpression) + public ClpExpression(String pushDownExpression, RowExpression remainingExpression) { - this.kqlQuery = Optional.ofNullable(kqlQuery); + this.pushDownExpression = Optional.ofNullable(pushDownExpression); this.remainingExpression = Optional.ofNullable(remainingExpression); } /** - * Creates an empty ClpExpression (no KQL definition, no remaining expression). + * Creates an empty ClpExpression with neither pushdown nor remaining expressions. */ public ClpExpression() { @@ -45,13 +45,13 @@ public ClpExpression() } /** - * Creates a ClpExpression from a fully translatable KQL string. + * Creates a ClpExpression from a fully translatable KQL query or column name. * - * @param kqlQuery + * @param pushDownExpression */ - public ClpExpression(String kqlQuery) + public ClpExpression(String pushDownExpression) { - this(kqlQuery, null); + this(pushDownExpression, null); } /** @@ -64,9 +64,9 @@ public ClpExpression(RowExpression remainingExpression) this(null, remainingExpression); } - public Optional getKqlQuery() + public Optional getPushDownExpression() { - return kqlQuery; + return pushDownExpression; } public Optional getRemainingExpression() 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 20570632af17f..33af3f60a1731 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 @@ -200,10 +200,10 @@ private ClpExpression handleNot(CallExpression node) RowExpression input = node.getArguments().get(0); ClpExpression expression = input.accept(this, null); - if (expression.getRemainingExpression().isPresent() || !expression.getKqlQuery().isPresent()) { + if (expression.getRemainingExpression().isPresent() || !expression.getPushDownExpression().isPresent()) { return new ClpExpression(node); } - return new ClpExpression("NOT " + expression.getKqlQuery().get()); + return new ClpExpression("NOT " + expression.getPushDownExpression().get()); } /** @@ -224,11 +224,11 @@ private ClpExpression handleLike(CallExpression node) 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); - if (!variable.getKqlQuery().isPresent()) { + if (!variable.getPushDownExpression().isPresent()) { return new ClpExpression(node); } - String variableName = variable.getKqlQuery().get(); + String variableName = variable.getPushDownExpression().get(); RowExpression argument = node.getArguments().get(1); String pattern; @@ -277,19 +277,19 @@ private ClpExpression handleLogicalBinary(OperatorType operator, CallExpression RowExpression right = node.getArguments().get(1); ClpExpression maybeLeftSubstring = tryInterpretSubstringEquality(operator, left, right); - if (maybeLeftSubstring.getKqlQuery().isPresent()) { + if (maybeLeftSubstring.getPushDownExpression().isPresent()) { return maybeLeftSubstring; } ClpExpression maybeRightSubstring = tryInterpretSubstringEquality(operator, right, left); - if (maybeRightSubstring.getKqlQuery().isPresent()) { + if (maybeRightSubstring.getPushDownExpression().isPresent()) { return maybeRightSubstring; } ClpExpression leftExpression = left.accept(this, null); ClpExpression rightExpression = right.accept(this, null); - Optional leftDefinition = leftExpression.getKqlQuery(); - Optional rightDefinition = rightExpression.getKqlQuery(); + Optional leftDefinition = leftExpression.getPushDownExpression(); + Optional rightDefinition = rightExpression.getPushDownExpression(); if (!leftDefinition.isPresent() || !rightDefinition.isPresent()) { return new ClpExpression(node); } @@ -423,11 +423,11 @@ private Optional parseSubstringCall(CallExpression callExpression) } ClpExpression variable = callExpression.getArguments().get(0).accept(this, null); - if (!variable.getKqlQuery().isPresent()) { + if (!variable.getPushDownExpression().isPresent()) { return Optional.empty(); } - String varName = variable.getKqlQuery().get(); + String varName = variable.getPushDownExpression().get(); RowExpression startExpression = callExpression.getArguments().get(1); RowExpression lengthExpression = null; if (argCount == 3) { @@ -566,19 +566,19 @@ private ClpExpression handleAnd(SpecialFormExpression node) StringBuilder queryBuilder = new StringBuilder(); queryBuilder.append("("); List remainingExpressions = new ArrayList<>(); - boolean hasDefinition = false; + boolean hasPushDownExpression = false; for (RowExpression argument : node.getArguments()) { ClpExpression expression = argument.accept(this, null); - if (expression.getKqlQuery().isPresent()) { - hasDefinition = true; - queryBuilder.append(expression.getKqlQuery().get()); + if (expression.getPushDownExpression().isPresent()) { + hasPushDownExpression = true; + queryBuilder.append(expression.getPushDownExpression().get()); queryBuilder.append(" AND "); } if (expression.getRemainingExpression().isPresent()) { remainingExpressions.add(expression.getRemainingExpression().get()); } } - if (!hasDefinition) { + if (!hasPushDownExpression) { return new ClpExpression(node); } else if (!remainingExpressions.isEmpty()) { @@ -612,10 +612,10 @@ private ClpExpression handleOr(SpecialFormExpression node) queryBuilder.append("("); for (RowExpression argument : node.getArguments()) { ClpExpression expression = argument.accept(this, null); - if (expression.getRemainingExpression().isPresent() || !expression.getKqlQuery().isPresent()) { + if (expression.getRemainingExpression().isPresent() || !expression.getPushDownExpression().isPresent()) { return new ClpExpression(node); } - queryBuilder.append(expression.getKqlQuery().get()); + queryBuilder.append(expression.getPushDownExpression().get()); queryBuilder.append(" OR "); } // Remove the last " OR " from the query @@ -634,10 +634,10 @@ private ClpExpression handleOr(SpecialFormExpression node) private ClpExpression handleIn(SpecialFormExpression node) { ClpExpression variable = node.getArguments().get(0).accept(this, null); - if (!variable.getKqlQuery().isPresent()) { + if (!variable.getPushDownExpression().isPresent()) { return new ClpExpression(node); } - String variableName = variable.getKqlQuery().get(); + String variableName = variable.getPushDownExpression().get(); StringBuilder queryBuilder = new StringBuilder(); queryBuilder.append("("); for (RowExpression argument : node.getArguments().subList(1, node.getArguments().size())) { @@ -676,11 +676,11 @@ private ClpExpression handleIsNull(SpecialFormExpression node) } ClpExpression expression = node.getArguments().get(0).accept(this, null); - if (!expression.getKqlQuery().isPresent()) { + if (!expression.getPushDownExpression().isPresent()) { return new ClpExpression(node); } - String variableName = expression.getKqlQuery().get(); + String variableName = expression.getPushDownExpression().get(); return new ClpExpression(format("NOT %s: *", variableName)); } @@ -739,10 +739,10 @@ private ClpExpression handleDereference(RowExpression expression) String fieldName = field.getName().orElse("field" + fieldIndex); ClpExpression baseString = handleDereference(base); - if (!baseString.getKqlQuery().isPresent()) { + if (!baseString.getPushDownExpression().isPresent()) { return new ClpExpression(expression); } - return new ClpExpression(baseString.getKqlQuery().get() + "." + fieldName); + return new ClpExpression(baseString.getPushDownExpression().get() + "." + fieldName); } private static class SubstrInfo 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 8916b73d74cbe..adab0bf71c9a8 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 @@ -77,7 +77,7 @@ public PlanNode visitFilter(FilterNode node, RewriteContext context) ClpTableHandle clpTableHandle = (ClpTableHandle) tableHandle.getConnectorHandle(); ClpExpression clpExpression = node.getPredicate() .accept(new ClpFilterToKqlConverter(functionResolution, functionManager, assignments), null); - Optional kqlQuery = clpExpression.getKqlQuery(); + Optional kqlQuery = clpExpression.getPushDownExpression(); Optional remainingPredicate = clpExpression.getRemainingExpression(); if (!kqlQuery.isPresent()) { return node; 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 04ca590451f53..805b241255cce 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 @@ -194,9 +194,9 @@ public void testComplexPushdown() private void testFilter(String sqlExpression, String expectedKqlExpression, String expectedRemainingExpression, SessionHolder sessionHolder) { - RowExpression pushDownExpression = getRowExpression(sqlExpression, sessionHolder); - ClpExpression clpExpression = pushDownExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); - Optional kqlExpression = clpExpression.getKqlQuery(); + RowExpression actualExpression = getRowExpression(sqlExpression, sessionHolder); + ClpExpression clpExpression = actualExpression.accept(new ClpFilterToKqlConverter(standardFunctionResolution, functionAndTypeManager, variableToColumnHandleMap), null); + Optional kqlExpression = clpExpression.getPushDownExpression(); Optional remainingExpression = clpExpression.getRemainingExpression(); if (expectedKqlExpression != null) { assertTrue(kqlExpression.isPresent()); From 636d35e217b4fd29d31d4c59777bdcb37911ff52 Mon Sep 17 00:00:00 2001 From: wraymo Date: Tue, 24 Jun 2025 10:01:24 -0400 Subject: [PATCH 6/7] apply review suggestion --- .../plugin/clp/ClpFilterToKqlConverter.java | 102 ++++++++++-------- 1 file changed, 55 insertions(+), 47 deletions(-) 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 33af3f60a1731..02fe7c5dc7f31 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 @@ -65,14 +65,14 @@ * Supported translations include: *
    *
  • Comparisons between variables and constants (e.g., =, !=, <, >, <=, >=).
    • - *
  • String pattern matches using LIKE with constant patterns only. "^%[^%_]*%$" - * is not supported.
    • - *
  • Membership checks using IN with a list of constants only
    • - *
  • NULL checks via IS NULL
    • - *
  • Substring comparisons (e.g., SUBSTR(x, start, len) = "val") are supported - * only when compared against a constant.
    • - *
  • Dereferencing fields from row-typed variables
    • - *
  • Logical operators AND, OR, and NOT
    • + *
  • String pattern matches using LIKE with constant patterns only. Patterns that begin and + * end with % (i.e., "^%[^%_]*%$") are not supported.
    • + *
  • Membership checks using IN with a list of constants only.
    • + *
  • NULL checks via IS NULL.
    • + *
  • Substring comparisons (e.g., SUBSTR(x, start, len) = "val") against a + * constant.
    • + *
  • Dereferencing fields from row-typed variables.
    • + *
  • Logical operators AND, OR, and NOT.
    • *
*/ public class ClpFilterToKqlConverter @@ -188,8 +188,8 @@ private String getVariableName(VariableReferenceExpression variable) * Example: NOT (col1 = 5) → NOT col1: 5 * * @param node the NOT call expression - * @return a ClpExpression containing the equivalent KQL query or the original expression if it - * couldn't be translated + * @return a ClpExpression containing either the equivalent KQL query, or the original + * expression if it couldn't be translated */ private ClpExpression handleNot(CallExpression node) { @@ -209,14 +209,15 @@ private ClpExpression handleNot(CallExpression node) /** * Handles LIKE expressions. *

- * Converts SQL LIKE patterns into equivalent KQL queries using * (for %) - * and ? (for _). Only supports constant or casted constant patterns. + * Converts SQL LIKE patterns into equivalent KQL queries using * (for + * %) and ? (for _). Only supports constant or casted + * constant patterns. *

* Example: col1 LIKE 'a_bc%' → col1: "a?bc*" * * @param node the LIKE call expression - * @return a ClpExpression containing the equivalent KQL query, or the original expression if it - * couldn't be translated + * @return a ClpExpression containing either the equivalent KQL query, or the original + * expression if it couldn't be translated */ private ClpExpression handleLike(CallExpression node) { @@ -259,13 +260,13 @@ else if (argument instanceof CallExpression) { /** * Handles logical binary operators (e.g., =, !=, <, >) between two expressions. *

- * Supports constant values on either side and flips the operator if necessary. Also delegates to a - * substring handler for SUBSTR(x, ...) = 'value' patterns. + * Supports constant values on either side and flips the operator if necessary. Also delegates + * to a substring handler for SUBSTR(x, ...) = 'value' patterns. * * @param operator the binary operator (e.g., EQUAL, NOT_EQUAL) * @param node the call expression representing the binary operation - * @return a ClpExpression containing the equivalent KQL query or the original expression if it - * couldn't be translated + * @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) { @@ -339,8 +340,8 @@ else if (leftIsConstant) { * @param operator the comparison operator * @param literalType the type of the literal * @param originalNode the original RowExpression node - * @return a ClpExpression containing the equivalent KQL query or the original expression if it - * couldn't be translated + * @return a ClpExpression containing either the equivalent KQL query, or the original + * expression if it couldn't be translated */ private ClpExpression buildClpExpression( String variableName, @@ -372,13 +373,15 @@ else if (LOGICAL_BINARY_OPS_FILTER.contains(operator) && !(literalType instanceo } /** - * Checks whether the given expression matches the pattern SUBSTR(x, ...) = 'someString', - * and if so, attempts to convert it into a KQL query using wildcards and construct a CLP expression. + * Checks whether the given expression matches the pattern + * SUBSTR(x, ...) = 'someString', and if so, attempts to convert it into a KQL + * query using wildcards and constructs a CLP expression. * * @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 the translated KQL filter or an empty one if conversion fails + * @return a ClpExpression containing either the equivalent KQL query, or nothing if it couldn't + * be translated */ private ClpExpression tryInterpretSubstringEquality( OperatorType operator, @@ -407,7 +410,7 @@ private ClpExpression tryInterpretSubstringEquality( * Parses a SUBSTR(x, start [, length]) call into a SubstrInfo object if valid. * * @param callExpression the call expression to inspect - * @return an Optional containing SubstrInfo if the expression is a valid SUBSTR call, otherwise empty + * @return an Optional containing SubstrInfo if the expression is a valid SUBSTR call */ private Optional parseSubstringCall(CallExpression callExpression) { @@ -438,7 +441,8 @@ private Optional parseSubstringCall(CallExpression callExpression) } /** - * Converts a SUBSTR(x, start [, length]) = 'someString' into a KQL-style wildcard query. + * Converts a SUBSTR(x, start [, length]) = 'someString' into a KQL-style wildcard + * query. *

* Examples: *
    @@ -450,7 +454,8 @@ private Optional parseSubstringCall(CallExpression callExpression) * * @param info parsed SUBSTR call info * @param targetString the literal string being compared to - * @return a ClpExpression containing the translated KQL query if successful; otherwise, an empty ClpExpression + * @return a ClpExpression containing either the equivalent KQL query, or nothing if it couldn't + * be translated */ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetString) { @@ -498,7 +503,7 @@ private ClpExpression interpretSubstringEquality(SubstrInfo info, String targetS * Attempts to parse a RowExpression as an integer constant. * * @param expression the row expression to parse - * @return an Optional containing the parsed integer value, if successful + * @return an Optional containing the integer value if it could be parsed */ private Optional parseIntValue(RowExpression expression) { @@ -532,7 +537,7 @@ else if (expression instanceof CallExpression) { * * @param lengthExpression the expression representing the length parameter * @param targetString the target string to compare length against - * @return an Optional containing the length if it matches targetString.length(), otherwise empty + * @return an Optional containing the length if it matches targetString.length() */ private Optional parseLengthLiteral(RowExpression lengthExpression, String targetString) { @@ -551,14 +556,14 @@ private Optional parseLengthLiteral(RowExpression lengthExpression, Str } /** - * Handles the logical AND expression. + * Handles the logical AND expression. *

    - * Combines all definable child expressions into a single KQL query joined by AND. - * Any unsupported children are collected into a remaining expression. + * Combines all definable child expressions into a single KQL query joined by AND. Any + * unsupported children are collected into the remaining expression. *

    * Example: col1 = 5 AND col2 = 'abc' → (col1: 5 AND col2: "abc") * - * @param node the AND special form expression + * @param node the AND special form expression * @return a ClpExpression containing the KQL query and any remaining sub-expressions */ private ClpExpression handleAnd(SpecialFormExpression node) @@ -596,15 +601,16 @@ else if (!remainingExpressions.isEmpty()) { } /** - * Handles the logical OR expression. + * Handles the logical OR expression. *

    * Combines all fully convertible child expressions into a single KQL query joined by OR. * Falls back to the original node if any child cannot be converted. *

    * Example: col1 = 5 OR col1 = 10 → (col1: 5 OR col1: 10) * - * @param node the OR special form expression - * @return a ClpExpression containing the OR-based KQL string, or the original expression if not fully convertible + * @param node the OR special form expression + * @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) { @@ -623,13 +629,13 @@ private ClpExpression handleOr(SpecialFormExpression node) } /** - * Handles the IN predicate. + * Handles the IN predicate. *

    * Example: col1 IN (1, 2, 3) → (col1: 1 OR col1: 2 OR col1: 3) * - * @param node the IN special form expression - * @return a ClpExpression containing the equivalent KQL query or the original expression if it - * couldn't be translated + * @param node the IN special form expression + * @return a ClpExpression containing either the equivalent KQL query, or the original + * expression if it couldn't be translated */ private ClpExpression handleIn(SpecialFormExpression node) { @@ -655,18 +661,19 @@ private ClpExpression handleIn(SpecialFormExpression node) } queryBuilder.append(" OR "); } + // Remove the last " OR " from the query return new ClpExpression(queryBuilder.substring(0, queryBuilder.length() - 4) + ")"); } /** - * Handles the IS NULL predicate. + * Handles the IS NULL predicate. *

    * Example: col1 IS NULL → NOT col1: * * - * @param node the IS_NULL special form expression - * @return a ClpExpression containing the equivalent KQL query or the original expression if it - * couldn't be translated + * @param node the IS_NULL special form expression + * @return a ClpExpression containing either the equivalent KQL query, or the original + * expression if it couldn't be translated */ private ClpExpression handleIsNull(SpecialFormExpression node) { @@ -687,13 +694,14 @@ private ClpExpression handleIsNull(SpecialFormExpression node) /** * Handles dereference expressions on RowTypes (e.g., col.row_field). *

    - * Converts nested row field access into dot-separated KQL-compatible field names. + * Converts nested row field accesses into dot-separated KQL-compatible field names. *

    * Example: address.city (from a RowType 'address') → address.city * - * @param expression the dereference expression (SpecialFormExpression or VariableReferenceExpression) - * @return a ClpExpression containing the dot-separated field name or the original expression if it - * couldn't be translated + * @param expression the dereference expression ({@link SpecialFormExpression} or + * {@link VariableReferenceExpression}) + * @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) { From 537c424e2b62fba41d7e53c61a463680323f875c Mon Sep 17 00:00:00 2001 From: wraymo Date: Tue, 24 Jun 2025 11:23:51 -0400 Subject: [PATCH 7/7] fix a name in comments --- .../java/com/facebook/presto/plugin/clp/ClpExpression.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java index e22ccb3b8a6a9..fd74933c709fe 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.java @@ -19,8 +19,9 @@ /** * Represents the result of converting a Presto RowExpression into a CLP-compatible KQL query. In - * every case, `kqlQuery` represents the part of the RowExpression that could be converted to a - * KQL expression, and `remainingExpression` represents the part that could not be converted. + * every case, `pushDownExpression` represents the part of the RowExpression that could be + * converted to a KQL expression, and `remainingExpression` represents the part that could not be + * converted. */ public class ClpExpression {