diff --git a/presto-main/src/main/java/com/facebook/presto/cost/FilterStatsCalculator.java b/presto-main/src/main/java/com/facebook/presto/cost/FilterStatsCalculator.java index 9366774176900..bd406ccc1d379 100644 --- a/presto-main/src/main/java/com/facebook/presto/cost/FilterStatsCalculator.java +++ b/presto-main/src/main/java/com/facebook/presto/cost/FilterStatsCalculator.java @@ -33,7 +33,7 @@ import com.facebook.presto.sql.planner.ExpressionInterpreter; import com.facebook.presto.sql.planner.LiteralEncoder; import com.facebook.presto.sql.planner.LiteralInterpreter; -import com.facebook.presto.sql.planner.NoOpSymbolResolver; +import com.facebook.presto.sql.planner.NoOpVariableResolver; import com.facebook.presto.sql.planner.RowExpressionInterpreter; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.relational.FunctionResolution; @@ -142,7 +142,7 @@ private Expression simplifyExpression(Session session, Expression predicate, Typ Map, Type> expressionTypes = getExpressionTypes(session, predicate, types); ExpressionInterpreter interpreter = ExpressionInterpreter.expressionOptimizer(predicate, metadata, session, expressionTypes); - Object value = interpreter.optimize(NoOpSymbolResolver.INSTANCE); + Object value = interpreter.optimize(NoOpVariableResolver.INSTANCE); if (value == null) { // Expression evaluates to SQL null, which in Filter is equivalent to false. This assumes the expression is a top-level expression (eg. not in NOT). diff --git a/presto-main/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java b/presto-main/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java index 8565035344042..868c0892159e7 100644 --- a/presto-main/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java +++ b/presto-main/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java @@ -31,7 +31,7 @@ import com.facebook.presto.sql.analyzer.ExpressionAnalyzer; import com.facebook.presto.sql.analyzer.Scope; import com.facebook.presto.sql.planner.ExpressionInterpreter; -import com.facebook.presto.sql.planner.NoOpSymbolResolver; +import com.facebook.presto.sql.planner.NoOpVariableResolver; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.relational.FunctionResolution; import com.facebook.presto.sql.relational.RowExpressionOptimizer; @@ -367,7 +367,7 @@ protected VariableStatsEstimate visitFunctionCall(FunctionCall node, Void contex { Map, Type> expressionTypes = getExpressionTypes(session, node, types); ExpressionInterpreter interpreter = ExpressionInterpreter.expressionOptimizer(node, metadata, session, expressionTypes); - Object value = interpreter.optimize(NoOpSymbolResolver.INSTANCE); + Object value = interpreter.optimize(NoOpVariableResolver.INSTANCE); if (value == null || value instanceof NullLiteral) { return nullStatsEstimate(); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionDomainTranslator.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionDomainTranslator.java index 65d853d055a55..8f261af9eb79b 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionDomainTranslator.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionDomainTranslator.java @@ -450,8 +450,8 @@ else if (symbolExpression instanceof Cast) { private Optional toNormalizedSimpleComparison(ComparisonExpression comparison) { Map, Type> expressionTypes = analyzeExpression(comparison); - Object left = ExpressionInterpreter.expressionOptimizer(comparison.getLeft(), metadata, session, expressionTypes).optimize(NoOpSymbolResolver.INSTANCE); - Object right = ExpressionInterpreter.expressionOptimizer(comparison.getRight(), metadata, session, expressionTypes).optimize(NoOpSymbolResolver.INSTANCE); + Object left = ExpressionInterpreter.expressionOptimizer(comparison.getLeft(), metadata, session, expressionTypes).optimize(NoOpVariableResolver.INSTANCE); + Object right = ExpressionInterpreter.expressionOptimizer(comparison.getRight(), metadata, session, expressionTypes).optimize(NoOpVariableResolver.INSTANCE); Type leftType = expressionTypes.get(NodeRef.of(comparison.getLeft())); Type rightType = expressionTypes.get(NodeRef.of(comparison.getRight())); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionInterpreter.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionInterpreter.java index 264d8fc477a81..48da9402878c2 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionInterpreter.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/ExpressionInterpreter.java @@ -41,7 +41,7 @@ import com.facebook.presto.sql.analyzer.Scope; import com.facebook.presto.sql.analyzer.SemanticErrorCode; import com.facebook.presto.sql.analyzer.SemanticException; -import com.facebook.presto.sql.planner.Interpreters.LambdaSymbolResolver; +import com.facebook.presto.sql.planner.Interpreters.LambdaVariableResolver; import com.facebook.presto.sql.planner.iterative.rule.DesugarCurrentUser; import com.facebook.presto.sql.tree.ArithmeticBinaryExpression; import com.facebook.presto.sql.tree.ArithmeticUnaryExpression; @@ -127,6 +127,7 @@ import static com.facebook.presto.sql.planner.LiteralEncoder.estimatedSizeInBytes; import static com.facebook.presto.sql.planner.LiteralEncoder.isSupportedLiteralType; import static com.facebook.presto.sql.planner.iterative.rule.CanonicalizeExpressionRewriter.canonicalizeExpression; +import static com.facebook.presto.sql.relational.Expressions.variable; import static com.facebook.presto.type.LikeFunctions.isLikePattern; import static com.facebook.presto.type.LikeFunctions.unescapeLiteralLikePattern; import static com.facebook.presto.util.LegacyRowFieldOrdinalAccessUtil.parseAnonymousRowFieldOrdinalAccess; @@ -260,13 +261,13 @@ public Object evaluate() return visitor.process(expression, null); } - public Object evaluate(SymbolResolver inputs) + public Object evaluate(VariableResolver inputs) { checkState(!optimize, "evaluate(SymbolResolver) not allowed for optimizer"); return visitor.process(expression, inputs); } - public Object optimize(SymbolResolver inputs) + public Object optimize(VariableResolver inputs) { checkState(optimize, "evaluate(SymbolResolver) not allowed for interpreter"); return visitor.process(expression, inputs); @@ -333,7 +334,7 @@ protected Object visitIdentifier(Identifier node, Object context) // ExpressionInterpreter should only be invoked after planning. // As a result, this method should be unreachable. // However, RelationPlanner.visitUnnest and visitValues invokes evaluateConstantExpression. - return ((SymbolResolver) context).getValue(new Symbol(node.getValue())); + return ((VariableResolver) context).getValue(variable(node.getValue(), type(node))); } @Override @@ -345,7 +346,7 @@ protected Object visitParameter(Parameter node, Object context) @Override protected Object visitSymbolReference(SymbolReference node, Object context) { - return ((SymbolResolver) context).getValue(Symbol.from(node)); + return ((VariableResolver) context).getValue(variable(node.getName(), type(node))); } @Override @@ -935,7 +936,7 @@ protected Object visitLambdaExpression(LambdaExpression node, Object context) .map(Primitives::wrap) .collect(toImmutableList()), argumentNames, - map -> process(body, new LambdaSymbolResolver(map))); + map -> process(body, new LambdaVariableResolver(map))); } @Override diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/Interpreters.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/Interpreters.java index 165bdc73fb958..c800be2918293 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/Interpreters.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/Interpreters.java @@ -14,6 +14,7 @@ package com.facebook.presto.sql.planner; import com.facebook.presto.spi.block.Block; +import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.spi.type.CharType; import com.facebook.presto.spi.type.Type; import com.facebook.presto.spi.type.VarcharType; @@ -67,21 +68,21 @@ static boolean interpretLikePredicate(Type valueType, Slice value, Regex regex) return LikeFunctions.likeChar((long) ((CharType) valueType).getLength(), value, regex); } - public static class LambdaSymbolResolver - implements SymbolResolver + public static class LambdaVariableResolver + implements VariableResolver { private final Map values; - public LambdaSymbolResolver(Map values) + public LambdaVariableResolver(Map values) { this.values = requireNonNull(values, "values is null"); } @Override - public Object getValue(Symbol symbol) + public Object getValue(VariableReferenceExpression variable) { - checkState(values.containsKey(symbol.getName()), "values does not contain %s", symbol); - return values.get(symbol.getName()); + checkState(values.containsKey(variable.getName()), "values does not contain %s", variable); + return values.get(variable.getName()); } } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/LookupSymbolResolver.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/LookupSymbolResolver.java deleted file mode 100644 index 3b5a0e976fbf9..0000000000000 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/LookupSymbolResolver.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * 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.sql.planner; - -import com.facebook.presto.spi.ColumnHandle; -import com.facebook.presto.spi.predicate.NullableValue; -import com.facebook.presto.spi.relation.VariableReferenceExpression; -import com.google.common.collect.ImmutableMap; - -import java.util.Map; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static java.util.Objects.requireNonNull; - -public class LookupSymbolResolver - implements SymbolResolver -{ - private final Map assignments; - private final Map bindings; - - public LookupSymbolResolver(Map assignments, Map bindings) - { - requireNonNull(assignments, "assignments is null"); - requireNonNull(bindings, "bindings is null"); - - this.assignments = assignments.entrySet().stream().collect(toImmutableMap(entry -> new Symbol(entry.getKey().getName()), Map.Entry::getValue)); - this.bindings = ImmutableMap.copyOf(bindings); - } - - @Override - public Object getValue(Symbol symbol) - { - ColumnHandle column = assignments.get(symbol); - checkArgument(column != null, "Missing column assignment for %s", symbol); - - if (!bindings.containsKey(column)) { - return symbol.toSymbolReference(); - } - - return bindings.get(column).getValue(); - } -} diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/NoOpSymbolResolver.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/NoOpVariableResolver.java similarity index 64% rename from presto-main/src/main/java/com/facebook/presto/sql/planner/NoOpSymbolResolver.java rename to presto-main/src/main/java/com/facebook/presto/sql/planner/NoOpVariableResolver.java index bbb9e7ceb1aea..83b38912665d8 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/NoOpSymbolResolver.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/NoOpVariableResolver.java @@ -13,14 +13,16 @@ */ package com.facebook.presto.sql.planner; -public class NoOpSymbolResolver - implements SymbolResolver +import com.facebook.presto.spi.relation.VariableReferenceExpression; + +public class NoOpVariableResolver + implements VariableResolver { - public static final NoOpSymbolResolver INSTANCE = new NoOpSymbolResolver(); + public static final NoOpVariableResolver INSTANCE = new NoOpVariableResolver(); @Override - public Object getValue(Symbol symbol) + public Object getValue(VariableReferenceExpression variable) { - return symbol.toSymbolReference(); + return new Symbol(variable.getName()).toSymbolReference(); } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java index fc05f98200c99..4c7e5f1a5b981 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java @@ -455,6 +455,15 @@ public PlanOptimizers( .add(new InlineProjections(metadata.getFunctionManager())) .build())); + // TODO: move this before optimization if possible!! + // Replace all expressions with row expressions + builder.add(new IterativeOptimizer( + ruleStats, + statsCalculator, + costCalculator, + new TranslateExpressions(metadata, sqlParser).rules())); + // After this point, all planNodes should not contain OriginalExpression + if (!forceSingleNode) { builder.add(new ReplicateSemiJoinInDelete()); // Must run before AddExchanges builder.add((new IterativeOptimizer( @@ -475,15 +484,6 @@ public PlanOptimizers( builder.add(new StatsRecordingPlanOptimizer(optimizerStats, new AddExchanges(metadata, sqlParser))); } - // TODO: move this before optimization if possible!! - // Replace all expressions with row expressions - builder.add(new IterativeOptimizer( - ruleStats, - statsCalculator, - costCalculator, - new TranslateExpressions(metadata, sqlParser).rules())); - // After this point, all planNodes should not contain OriginalExpression - //noinspection UnusedAssignment estimatedExchangesCostCalculator = null; // Prevent accidental use after AddExchanges diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/RowExpressionInterpreter.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/RowExpressionInterpreter.java index 8d955f292c0ca..3b2f5a2a6fd25 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/RowExpressionInterpreter.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/RowExpressionInterpreter.java @@ -39,11 +39,10 @@ import com.facebook.presto.spi.type.TypeManager; import com.facebook.presto.spi.type.TypeSignature; import com.facebook.presto.sql.InterpretedFunctionInvoker; -import com.facebook.presto.sql.planner.Interpreters.LambdaSymbolResolver; +import com.facebook.presto.sql.planner.Interpreters.LambdaVariableResolver; import com.facebook.presto.sql.relational.FunctionResolution; import com.facebook.presto.sql.relational.RowExpressionDeterminismEvaluator; import com.facebook.presto.util.Failures; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.primitives.Primitives; import io.airlift.joni.Regex; @@ -171,10 +170,9 @@ public Object optimize() } /** - * For test only; convenient to replace symbol with constants. Production code should not replace any symbols; use the interface above + * Replace symbol with constants */ - @VisibleForTesting - public Object optimize(SymbolResolver inputs) + public Object optimize(VariableResolver inputs) { checkState(optimizationLevel.ordinal() <= EVALUATED.ordinal(), "optimize(SymbolResolver) not allowed for interpreter"); return expression.accept(visitor, inputs); @@ -198,8 +196,8 @@ public Object visitConstant(ConstantExpression node, Object context) @Override public Object visitVariableReference(VariableReferenceExpression node, Object context) { - if (context instanceof SymbolResolver) { - return ((SymbolResolver) context).getValue(new Symbol(node.getName())); + if (context instanceof VariableResolver) { + return ((VariableResolver) context).getValue(node); } return node; } @@ -281,7 +279,7 @@ public Object visitLambda(LambdaDefinitionExpression node, Object context) .map(Primitives::wrap) .collect(toImmutableList()), node.getArguments(), - map -> body.accept(this, new LambdaSymbolResolver(map))); + map -> body.accept(this, new LambdaVariableResolver(map))); } @Override diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/SymbolResolver.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/VariableResolver.java similarity index 78% rename from presto-main/src/main/java/com/facebook/presto/sql/planner/SymbolResolver.java rename to presto-main/src/main/java/com/facebook/presto/sql/planner/VariableResolver.java index 9b6b8b51fc463..e08fb43ce78a5 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/SymbolResolver.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/VariableResolver.java @@ -13,7 +13,9 @@ */ package com.facebook.presto.sql.planner; -public interface SymbolResolver +import com.facebook.presto.spi.relation.VariableReferenceExpression; + +public interface VariableResolver { - Object getValue(Symbol symbol); + Object getValue(VariableReferenceExpression variable); } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PickTableLayout.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PickTableLayout.java index ae377a192ba52..73494414a6c0f 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PickTableLayout.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PickTableLayout.java @@ -15,6 +15,7 @@ import com.facebook.presto.Session; import com.facebook.presto.execution.warnings.WarningCollector; +import com.facebook.presto.expressions.LogicalRowExpressions; import com.facebook.presto.matching.Capture; import com.facebook.presto.matching.Captures; import com.facebook.presto.matching.Pattern; @@ -33,6 +34,8 @@ import com.facebook.presto.spi.plan.ValuesNode; import com.facebook.presto.spi.predicate.NullableValue; import com.facebook.presto.spi.predicate.TupleDomain; +import com.facebook.presto.spi.relation.ConstantExpression; +import com.facebook.presto.spi.relation.DomainTranslator; import com.facebook.presto.spi.relation.RowExpression; import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.spi.type.Type; @@ -40,10 +43,15 @@ import com.facebook.presto.sql.planner.ExpressionDomainTranslator; import com.facebook.presto.sql.planner.ExpressionInterpreter; import com.facebook.presto.sql.planner.LiteralEncoder; -import com.facebook.presto.sql.planner.LookupSymbolResolver; +import com.facebook.presto.sql.planner.RowExpressionInterpreter; +import com.facebook.presto.sql.planner.Symbol; import com.facebook.presto.sql.planner.TypeProvider; +import com.facebook.presto.sql.planner.VariableResolver; import com.facebook.presto.sql.planner.VariablesExtractor; import com.facebook.presto.sql.planner.iterative.Rule; +import com.facebook.presto.sql.relational.FunctionResolution; +import com.facebook.presto.sql.relational.RowExpressionDeterminismEvaluator; +import com.facebook.presto.sql.relational.RowExpressionDomainTranslator; import com.facebook.presto.sql.relational.SqlToRowExpressionTranslator; import com.facebook.presto.sql.tree.Expression; import com.facebook.presto.sql.tree.NodeRef; @@ -58,12 +66,15 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import static com.facebook.presto.SystemSessionProperties.isNewOptimizerEnabled; import static com.facebook.presto.expressions.LogicalRowExpressions.TRUE_CONSTANT; import static com.facebook.presto.expressions.RowExpressionNodeInliner.replaceExpression; import static com.facebook.presto.matching.Capture.newCapture; import static com.facebook.presto.metadata.TableLayoutResult.computeEnforced; +import static com.facebook.presto.spi.relation.DomainTranslator.BASIC_COLUMN_EXTRACTOR; +import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.OPTIMIZED; import static com.facebook.presto.sql.ExpressionUtils.combineConjuncts; import static com.facebook.presto.sql.ExpressionUtils.filterDeterministicConjuncts; import static com.facebook.presto.sql.ExpressionUtils.filterNonDeterministicConjuncts; @@ -74,7 +85,9 @@ import static com.facebook.presto.sql.planner.plan.Patterns.tableScan; import static com.facebook.presto.sql.relational.OriginalExpressionUtils.castToExpression; import static com.facebook.presto.sql.relational.OriginalExpressionUtils.castToRowExpression; +import static com.facebook.presto.sql.relational.OriginalExpressionUtils.isExpression; import static com.facebook.presto.sql.tree.BooleanLiteral.TRUE_LITERAL; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; @@ -155,7 +168,7 @@ public Result apply(FilterNode filterNode, Captures captures, Context context) PlanNode rewritten = pushPredicateIntoTableScan( tableScan, - castToExpression(filterNode.getPredicate()), + filterNode.getPredicate(), false, context.getSession(), context.getVariableAllocator().getTypes(), @@ -267,9 +280,13 @@ public Result apply(TableScanNode tableScanNode, Captures captures, Context cont } } + /** + * @param predicate can be a RowExpression or an OriginalExpression. The method will handle both cases. + * Once Expression is migrated to RowExpression in PickTableLayout, the method should only support RowExpression. + */ public static PlanNode pushPredicateIntoTableScan( TableScanNode node, - Expression predicate, + RowExpression predicate, boolean pruneWithPredicateExpression, Session session, TypeProvider types, @@ -278,46 +295,74 @@ public static PlanNode pushPredicateIntoTableScan( SqlParser parser, ExpressionDomainTranslator domainTranslator) { - if (metadata.isPushdownFilterSupported(session, node.getTable())) { + if (!metadata.isPushdownFilterSupported(session, node.getTable())) { + if (isExpression(predicate)) { + return pushPredicateIntoTableScan(node, castToExpression(predicate), pruneWithPredicateExpression, session, types, idAllocator, metadata, parser, domainTranslator); + } + return pushPredicateIntoTableScan(node, predicate, pruneWithPredicateExpression, session, idAllocator, metadata, new RowExpressionDomainTranslator(metadata)); + } + + // filter pushdown; to be replaced by ConnectorPlanOptimizer + RowExpression expression; + if (isExpression(predicate)) { Map, Type> predicateTypes = getExpressionTypes( session, metadata, parser, types, - predicate, + castToExpression(predicate), emptyList(), WarningCollector.NOOP, false); - BiMap symbolToColumnMapping = node.getAssignments().entrySet().stream() - .collect(toImmutableBiMap( - Map.Entry::getKey, - entry -> new VariableReferenceExpression(getColumnName(session, metadata, node.getTable(), entry.getValue()), entry.getKey().getType()))); - RowExpression translatedPredicate = replaceExpression(SqlToRowExpressionTranslator.translate(predicate, predicateTypes, ImmutableMap.of(), metadata.getFunctionManager(), metadata.getTypeManager(), session), symbolToColumnMapping); - - PushdownFilterResult pushdownFilterResult = metadata.pushdownFilter(session, node.getTable(), translatedPredicate); + expression = SqlToRowExpressionTranslator.translate(castToExpression(predicate), predicateTypes, ImmutableMap.of(), metadata.getFunctionManager(), metadata.getTypeManager(), session); + } + else { + expression = predicate; + } - TableLayout layout = pushdownFilterResult.getLayout(); - if (layout.getPredicate().isNone()) { - return new ValuesNode(idAllocator.getNextId(), node.getOutputVariables(), ImmutableList.of()); - } + BiMap symbolToColumnMapping = node.getAssignments().entrySet().stream() + .collect(toImmutableBiMap( + Map.Entry::getKey, + entry -> new VariableReferenceExpression(getColumnName(session, metadata, node.getTable(), entry.getValue()), entry.getKey().getType()))); + PushdownFilterResult pushdownFilterResult = metadata.pushdownFilter(session, node.getTable(), replaceExpression(expression, symbolToColumnMapping)); - TableScanNode tableScan = new TableScanNode( - node.getId(), - layout.getNewTableHandle(), - node.getOutputVariables(), - node.getAssignments(), - layout.getPredicate(), - TupleDomain.all()); + TableLayout layout = pushdownFilterResult.getLayout(); + if (layout.getPredicate().isNone()) { + return new ValuesNode(idAllocator.getNextId(), node.getOutputVariables(), ImmutableList.of()); + } - RowExpression unenforcedFilter = pushdownFilterResult.getUnenforcedFilter(); - if (!TRUE_CONSTANT.equals(unenforcedFilter)) { - return new FilterNode(idAllocator.getNextId(), tableScan, replaceExpression(unenforcedFilter, symbolToColumnMapping.inverse())); - } + TableScanNode tableScan = new TableScanNode( + node.getId(), + layout.getNewTableHandle(), + node.getOutputVariables(), + node.getAssignments(), + layout.getPredicate(), + TupleDomain.all()); - return tableScan; + RowExpression unenforcedFilter = pushdownFilterResult.getUnenforcedFilter(); + if (!TRUE_CONSTANT.equals(unenforcedFilter)) { + return new FilterNode(idAllocator.getNextId(), tableScan, replaceExpression(unenforcedFilter, symbolToColumnMapping.inverse())); } + return tableScan; + } + + /** + * For Expression {@param predicate} + */ + @Deprecated + private static PlanNode pushPredicateIntoTableScan( + TableScanNode node, + Expression predicate, + boolean pruneWithPredicateExpression, + Session session, + TypeProvider types, + PlanNodeIdAllocator idAllocator, + Metadata metadata, + SqlParser parser, + ExpressionDomainTranslator domainTranslator) + { // don't include non-deterministic predicates Expression deterministicPredicate = filterDeterministicConjuncts(predicate); ExpressionDomainTranslator.ExtractionResult decomposedPredicate = ExpressionDomainTranslator.fromPredicate( @@ -334,7 +379,7 @@ public static PlanNode pushPredicateIntoTableScan( Constraint constraint; if (pruneWithPredicateExpression) { - LayoutConstraintEvaluator evaluator = new LayoutConstraintEvaluator( + LayoutConstraintEvaluatorForExpression evaluator = new LayoutConstraintEvaluatorForExpression( metadata, parser, session, @@ -396,18 +441,110 @@ public static PlanNode pushPredicateIntoTableScan( return tableScan; } + /** + * For RowExpression {@param predicate} + */ + private static PlanNode pushPredicateIntoTableScan( + TableScanNode node, + RowExpression predicate, + boolean pruneWithPredicateExpression, + Session session, + PlanNodeIdAllocator idAllocator, + Metadata metadata, + RowExpressionDomainTranslator domainTranslator) + { + // don't include non-deterministic predicates + LogicalRowExpressions logicalRowExpressions = new LogicalRowExpressions( + new RowExpressionDeterminismEvaluator(metadata.getFunctionManager()), + new FunctionResolution(metadata.getFunctionManager()), + metadata.getFunctionManager()); + RowExpression deterministicPredicate = logicalRowExpressions.filterDeterministicConjuncts(predicate); + DomainTranslator.ExtractionResult decomposedPredicate = domainTranslator.fromPredicate( + session.toConnectorSession(), + deterministicPredicate, + BASIC_COLUMN_EXTRACTOR); + + TupleDomain newDomain = decomposedPredicate.getTupleDomain() + .transform(variableName -> node.getAssignments().get(variableName)) + .intersect(node.getEnforcedConstraint()); + + Map assignments = ImmutableBiMap.copyOf(node.getAssignments()).inverse(); + + Constraint constraint; + if (pruneWithPredicateExpression) { + LayoutConstraintEvaluatorForRowExpression evaluator = new LayoutConstraintEvaluatorForRowExpression( + metadata, + session, + node.getAssignments(), + logicalRowExpressions.combineConjuncts( + deterministicPredicate, + // Simplify the tuple domain to avoid creating an expression with too many nodes, + // which would be expensive to evaluate in the call to isCandidate below. + domainTranslator.toPredicate(newDomain.simplify().transform(column -> assignments.getOrDefault(column, null))))); + constraint = new Constraint<>(newDomain, evaluator::isCandidate); + } + else { + // Currently, invoking the expression interpreter is very expensive. + // TODO invoke the interpreter unconditionally when the interpreter becomes cheap enough. + constraint = new Constraint<>(newDomain); + } + if (constraint.getSummary().isNone()) { + return new ValuesNode(idAllocator.getNextId(), node.getOutputVariables(), ImmutableList.of()); + } + + // Layouts will be returned in order of the connector's preference + TableLayoutResult layout = metadata.getLayout( + session, + node.getTable(), + constraint, + Optional.of(node.getOutputVariables().stream() + .map(variable -> node.getAssignments().get(variable)) + .collect(toImmutableSet()))); + + if (layout.getLayout().getPredicate().isNone()) { + return new ValuesNode(idAllocator.getNextId(), node.getOutputVariables(), ImmutableList.of()); + } + + TableScanNode tableScan = new TableScanNode( + node.getId(), + layout.getLayout().getNewTableHandle(), + node.getOutputVariables(), + node.getAssignments(), + layout.getLayout().getPredicate(), + computeEnforced(newDomain, layout.getUnenforcedConstraint())); + + // The order of the arguments to combineConjuncts matters: + // * Unenforced constraints go first because they can only be simple column references, + // which are not prone to logic errors such as out-of-bound access, div-by-zero, etc. + // * Conjuncts in non-deterministic expressions and non-TupleDomain-expressible expressions should + // retain their original (maybe intermixed) order from the input predicate. However, this is not implemented yet. + // * Short of implementing the previous bullet point, the current order of non-deterministic expressions + // and non-TupleDomain-expressible expressions should be retained. Changing the order can lead + // to failures of previously successful queries. + RowExpression resultingPredicate = logicalRowExpressions.combineConjuncts( + domainTranslator.toPredicate(layout.getUnenforcedConstraint().transform(assignments::get)), + logicalRowExpressions.filterNonDeterministicConjuncts(predicate), + decomposedPredicate.getRemainingExpression()); + + if (!TRUE_CONSTANT.equals(resultingPredicate)) { + return new FilterNode(idAllocator.getNextId(), tableScan, resultingPredicate); + } + return tableScan; + } + private static String getColumnName(Session session, Metadata metadata, TableHandle tableHandle, ColumnHandle columnHandle) { return metadata.getColumnMetadata(session, tableHandle, columnHandle).getName(); } - private static class LayoutConstraintEvaluator + @Deprecated + private static class LayoutConstraintEvaluatorForExpression { private final Map assignments; private final ExpressionInterpreter evaluator; private final Set arguments; - public LayoutConstraintEvaluator(Metadata metadata, SqlParser parser, Session session, TypeProvider types, Map assignments, Expression expression) + public LayoutConstraintEvaluatorForExpression(Metadata metadata, SqlParser parser, Session session, TypeProvider types, Map assignments, Expression expression) { this.assignments = assignments; @@ -424,18 +561,79 @@ private boolean isCandidate(Map bindings) if (intersection(bindings.keySet(), arguments).isEmpty()) { return true; } - LookupSymbolResolver inputs = new LookupSymbolResolver(assignments, bindings); + LookupVariableResolver inputs = new LookupVariableResolver(assignments, bindings, variable -> new Symbol(variable.getName()).toSymbolReference()); // Skip pruning if evaluation fails in a recoverable way. Failing here can cause // spurious query failures for partitions that would otherwise be filtered out. Object optimized = TryFunction.evaluate(() -> evaluator.optimize(inputs), true); // If any conjuncts evaluate to FALSE or null, then the whole predicate will never be true and so the partition should be pruned - if (Boolean.FALSE.equals(optimized) || optimized == null || optimized instanceof NullLiteral) { - return false; + return !Boolean.FALSE.equals(optimized) && optimized != null && !(optimized instanceof NullLiteral); + } + } + + private static class LayoutConstraintEvaluatorForRowExpression + { + private final Map assignments; + private final RowExpressionInterpreter evaluator; + private final Set arguments; + + public LayoutConstraintEvaluatorForRowExpression(Metadata metadata, Session session, Map assignments, RowExpression expression) + { + this.assignments = assignments; + + evaluator = new RowExpressionInterpreter(expression, metadata, session.toConnectorSession(), OPTIMIZED); + arguments = VariablesExtractor.extractUnique(expression).stream() + .map(assignments::get) + .collect(toImmutableSet()); + } + + private boolean isCandidate(Map bindings) + { + if (intersection(bindings.keySet(), arguments).isEmpty()) { + return true; + } + LookupVariableResolver inputs = new LookupVariableResolver(assignments, bindings, variable -> variable); + + // Skip pruning if evaluation fails in a recoverable way. Failing here can cause + // spurious query failures for partitions that would otherwise be filtered out. + Object optimized = TryFunction.evaluate(() -> evaluator.optimize(inputs), true); + + // If any conjuncts evaluate to FALSE or null, then the whole predicate will never be true and so the partition should be pruned + return !Boolean.FALSE.equals(optimized) && optimized != null && (!(optimized instanceof ConstantExpression) || !((ConstantExpression) optimized).isNull()); + } + } + + private static class LookupVariableResolver + implements VariableResolver + { + private final Map assignments; + private final Map bindings; + // Use Object type to let interpreters consume the result + // TODO: use RowExpression once the Expression-to-RowExpression is done + private final Function missingBindingSupplier; + + public LookupVariableResolver( + Map assignments, + Map bindings, + Function missingBindingSupplier) + { + this.assignments = requireNonNull(assignments, "assignments is null"); + this.bindings = ImmutableMap.copyOf(requireNonNull(bindings, "bindings is null")); + this.missingBindingSupplier = requireNonNull(missingBindingSupplier, "missingBindingSupplier is null"); + } + + @Override + public Object getValue(VariableReferenceExpression variable) + { + ColumnHandle column = assignments.get(variable); + checkArgument(column != null, "Missing column assignment for %s", variable); + + if (!bindings.containsKey(column)) { + return missingBindingSupplier.apply(variable); } - return true; + return bindings.get(column).getValue(); } } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/SimplifyExpressions.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/SimplifyExpressions.java index 11ccf1023bef5..050c21796baba 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/SimplifyExpressions.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/SimplifyExpressions.java @@ -20,7 +20,7 @@ import com.facebook.presto.sql.parser.SqlParser; import com.facebook.presto.sql.planner.ExpressionInterpreter; import com.facebook.presto.sql.planner.LiteralEncoder; -import com.facebook.presto.sql.planner.NoOpSymbolResolver; +import com.facebook.presto.sql.planner.NoOpVariableResolver; import com.facebook.presto.sql.planner.PlanVariableAllocator; import com.facebook.presto.sql.planner.iterative.Rule; import com.facebook.presto.sql.tree.Expression; @@ -53,7 +53,7 @@ static Expression rewrite(Expression expression, Session session, PlanVariableAl expression = extractCommonPredicates(expression); Map, Type> expressionTypes = getExpressionTypes(session, metadata, sqlParser, variableAllocator.getTypes(), expression, emptyList(), WarningCollector.NOOP); ExpressionInterpreter interpreter = ExpressionInterpreter.expressionOptimizer(expression, metadata, session, expressionTypes); - return literalEncoder.toExpression(interpreter.optimize(NoOpSymbolResolver.INSTANCE), expressionTypes.get(NodeRef.of(expression))); + return literalEncoder.toExpression(interpreter.optimize(NoOpVariableResolver.INSTANCE), expressionTypes.get(NodeRef.of(expression))); } public SimplifyExpressions(Metadata metadata, SqlParser sqlParser) diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java index ae8bc80ee6ceb..2208e741972bd 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java @@ -71,8 +71,6 @@ import com.facebook.presto.sql.planner.plan.TopNRowNumberNode; import com.facebook.presto.sql.planner.plan.UnnestNode; import com.facebook.presto.sql.planner.plan.WindowNode; -import com.facebook.presto.sql.tree.Expression; -import com.facebook.presto.sql.tree.SymbolReference; import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; @@ -106,12 +104,12 @@ import static com.facebook.presto.SystemSessionProperties.isRedistributeWrites; import static com.facebook.presto.SystemSessionProperties.isScaleWriters; import static com.facebook.presto.SystemSessionProperties.preferStreamingOperators; +import static com.facebook.presto.expressions.LogicalRowExpressions.TRUE_CONSTANT; import static com.facebook.presto.operator.aggregation.AggregationUtils.hasSingleNodeExecutionPreference; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.plan.LimitNode.Step.PARTIAL; import static com.facebook.presto.sql.planner.FragmentTableScanCounter.getNumberOfTableScans; import static com.facebook.presto.sql.planner.FragmentTableScanCounter.hasMultipleTableScans; -import static com.facebook.presto.sql.planner.PlannerUtils.toVariableReference; import static com.facebook.presto.sql.planner.SystemPartitioningHandle.FIXED_ARBITRARY_DISTRIBUTION; import static com.facebook.presto.sql.planner.SystemPartitioningHandle.FIXED_HASH_DISTRIBUTION; import static com.facebook.presto.sql.planner.SystemPartitioningHandle.SCALED_WRITER_DISTRIBUTION; @@ -131,8 +129,6 @@ import static com.facebook.presto.sql.planner.plan.ExchangeNode.partitionedExchange; import static com.facebook.presto.sql.planner.plan.ExchangeNode.replicatedExchange; import static com.facebook.presto.sql.planner.plan.ExchangeNode.roundRobinExchange; -import static com.facebook.presto.sql.relational.OriginalExpressionUtils.castToExpression; -import static com.facebook.presto.sql.tree.BooleanLiteral.TRUE_LITERAL; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; @@ -203,7 +199,7 @@ public PlanWithProperties visitPlan(PlanNode node, PreferredProperties preferred @Override public PlanWithProperties visitProject(ProjectNode node, PreferredProperties preferredProperties) { - Map identities = computeIdentityTranslations(node.getAssignments(), types); + Map identities = computeIdentityTranslations(node.getAssignments()); PreferredProperties translatedPreferred = preferredProperties.translate(symbol -> Optional.ofNullable(identities.get(symbol))); return rebaseAndDeriveProperties(node, planChild(node, translatedPreferred)); @@ -547,7 +543,7 @@ public PlanWithProperties visitDistinctLimit(DistinctLimitNode node, PreferredPr public PlanWithProperties visitFilter(FilterNode node, PreferredProperties preferredProperties) { if (node.getSource() instanceof TableScanNode) { - return planTableScan((TableScanNode) node.getSource(), castToExpression(node.getPredicate())); + return planTableScan((TableScanNode) node.getSource(), node.getPredicate()); } return rebaseAndDeriveProperties(node, planChild(node, preferredProperties)); @@ -556,7 +552,7 @@ public PlanWithProperties visitFilter(FilterNode node, PreferredProperties prefe @Override public PlanWithProperties visitTableScan(TableScanNode node, PreferredProperties preferredProperties) { - return planTableScan(node, TRUE_LITERAL); + return planTableScan(node, TRUE_CONSTANT); } @Override @@ -590,7 +586,7 @@ else if (redistributeWrites) { return rebaseAndDeriveProperties(node, source); } - private PlanWithProperties planTableScan(TableScanNode node, Expression predicate) + private PlanWithProperties planTableScan(TableScanNode node, RowExpression predicate) { PlanNode plan = pushPredicateIntoTableScan(node, predicate, true, session, types, idAllocator, metadata, parser, domainTranslator); // TODO: Support selecting layout with best local property once connector can participate in query optimization. @@ -1398,12 +1394,12 @@ private boolean canPushdownPartialMergeThroughLowMemoryOperators(PlanNode node) .allMatch(this::canPushdownPartialMergeThroughLowMemoryOperators); } - public static Map computeIdentityTranslations(Assignments assignments, TypeProvider types) + public static Map computeIdentityTranslations(Assignments assignments) { Map outputToInput = new HashMap<>(); for (Map.Entry assignment : assignments.getMap().entrySet()) { - if (castToExpression(assignment.getValue()) instanceof SymbolReference) { - outputToInput.put(assignment.getKey(), toVariableReference(castToExpression(assignment.getValue()), types)); + if (assignment.getValue() instanceof VariableReferenceExpression) { + outputToInput.put(assignment.getKey(), (VariableReferenceExpression) assignment.getValue()); } } return outputToInput; diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PredicatePushDown.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PredicatePushDown.java index 2f31aceb34d43..552dcb4715003 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PredicatePushDown.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PredicatePushDown.java @@ -34,8 +34,9 @@ import com.facebook.presto.sql.planner.ExpressionInterpreter; import com.facebook.presto.sql.planner.ExpressionVariableInliner; import com.facebook.presto.sql.planner.LiteralEncoder; -import com.facebook.presto.sql.planner.NoOpSymbolResolver; +import com.facebook.presto.sql.planner.NoOpVariableResolver; import com.facebook.presto.sql.planner.PlanVariableAllocator; +import com.facebook.presto.sql.planner.Symbol; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.planner.VariablesExtractor; import com.facebook.presto.sql.planner.plan.AssignUniqueId; @@ -975,7 +976,7 @@ private Expression simplifyExpression(Expression expression) emptyList(), /* parameters have already been replaced */ WarningCollector.NOOP); ExpressionInterpreter optimizer = ExpressionInterpreter.expressionOptimizer(expression, metadata, session, expressionTypes); - return literalEncoder.toExpression(optimizer.optimize(NoOpSymbolResolver.INSTANCE), expressionTypes.get(NodeRef.of(expression))); + return literalEncoder.toExpression(optimizer.optimize(NoOpVariableResolver.INSTANCE), expressionTypes.get(NodeRef.of(expression))); } private boolean areExpressionsEquivalent(Expression leftExpression, Expression rightExpression) @@ -1000,7 +1001,7 @@ private Object nullInputEvaluator(final Collection emptyList(), /* parameters have already been replaced */ WarningCollector.NOOP); return ExpressionInterpreter.expressionOptimizer(expression, metadata, session, expressionTypes) - .optimize(symbol -> nullVariableNames.contains(symbol.getName()) ? null : symbol.toSymbolReference()); + .optimize(variable -> nullVariableNames.contains(variable.getName()) ? null : new Symbol(variable.getName()).toSymbolReference()); } private Predicate joinEqualityExpression(final Collection leftVariables) diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PropertyDerivations.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PropertyDerivations.java index eb07ea6cc0967..d1514ab3dc822 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PropertyDerivations.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PropertyDerivations.java @@ -41,7 +41,7 @@ import com.facebook.presto.sql.parser.SqlParser; import com.facebook.presto.sql.planner.ExpressionDomainTranslator; import com.facebook.presto.sql.planner.ExpressionInterpreter; -import com.facebook.presto.sql.planner.NoOpSymbolResolver; +import com.facebook.presto.sql.planner.NoOpVariableResolver; import com.facebook.presto.sql.planner.RowExpressionInterpreter; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.planner.optimizations.ActualProperties.Global; @@ -652,7 +652,7 @@ public ActualProperties visitProject(ProjectNode node, List in Map, Type> expressionTypes = getExpressionTypes(session, metadata, parser, types, castToExpression(expression), emptyList(), WarningCollector.NOOP); Type type = requireNonNull(expressionTypes.get(NodeRef.of(castToExpression(expression)))); ExpressionInterpreter optimizer = ExpressionInterpreter.expressionOptimizer(castToExpression(expression), metadata, session, expressionTypes); - Object value = optimizer.optimize(NoOpSymbolResolver.INSTANCE); + Object value = optimizer.optimize(NoOpVariableResolver.INSTANCE); if (value instanceof SymbolReference) { VariableReferenceExpression variable = toVariableReference((SymbolReference) value, types); diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/FunctionAssertions.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/FunctionAssertions.java index 847c40f61a319..f0b4282de184b 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/FunctionAssertions.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/FunctionAssertions.java @@ -62,6 +62,7 @@ import com.facebook.presto.sql.gen.ExpressionCompiler; import com.facebook.presto.sql.parser.SqlParser; import com.facebook.presto.sql.planner.ExpressionInterpreter; +import com.facebook.presto.sql.planner.Symbol; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.tree.Cast; import com.facebook.presto.sql.tree.DefaultTraversalVisitor; @@ -857,7 +858,8 @@ private Object interpret(Expression expression, Type expectedType, Session sessi Map, Type> expressionTypes = getExpressionTypes(session, metadata, SQL_PARSER, SYMBOL_TYPES, expression, emptyList(), WarningCollector.NOOP); ExpressionInterpreter evaluator = ExpressionInterpreter.expressionInterpreter(expression, metadata, session, expressionTypes); - Object result = evaluator.evaluate(symbol -> { + Object result = evaluator.evaluate(variable -> { + Symbol symbol = new Symbol(variable.getName()); int position = 0; int channel = INPUT_MAPPING.get(new VariableReferenceExpression(symbol.getName(), SYMBOL_TYPES.get(symbol.toSymbolReference()))); Type type = SYMBOL_TYPES.get(symbol.toSymbolReference()); diff --git a/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java b/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java index 7f9b89a4bd75b..71352031a90fc 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java @@ -1567,7 +1567,8 @@ private static Object optimize(Expression expression) { Map, Type> expressionTypes = getExpressionTypes(TEST_SESSION, METADATA, SQL_PARSER, SYMBOL_TYPES, expression, emptyList(), WarningCollector.NOOP); ExpressionInterpreter interpreter = expressionOptimizer(expression, METADATA, TEST_SESSION, expressionTypes); - return interpreter.optimize(symbol -> { + return interpreter.optimize(variable -> { + Symbol symbol = new Symbol(variable.getName()); Object value = symbolConstant(symbol); if (value == null) { return symbol.toSymbolReference(); @@ -1578,7 +1579,8 @@ private static Object optimize(Expression expression) private static Object optimize(RowExpression expression, Level level) { - return new RowExpressionInterpreter(expression, METADATA, TEST_SESSION.toConnectorSession(), level).optimize(symbol -> { + return new RowExpressionInterpreter(expression, METADATA, TEST_SESSION.toConnectorSession(), level).optimize(variable -> { + Symbol symbol = new Symbol(variable.getName()); Object value = symbolConstant(symbol); if (value == null) { return new VariableReferenceExpression(symbol.getName(), SYMBOL_TYPES.get(symbol.toSymbolReference())); diff --git a/presto-main/src/test/java/com/facebook/presto/sql/TestingRowExpressionTranslator.java b/presto-main/src/test/java/com/facebook/presto/sql/TestingRowExpressionTranslator.java index 69fd145f12fad..e6cb9d073b8fc 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/TestingRowExpressionTranslator.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/TestingRowExpressionTranslator.java @@ -23,7 +23,7 @@ import com.facebook.presto.sql.parser.SqlParser; import com.facebook.presto.sql.planner.ExpressionInterpreter; import com.facebook.presto.sql.planner.LiteralEncoder; -import com.facebook.presto.sql.planner.NoOpSymbolResolver; +import com.facebook.presto.sql.planner.NoOpVariableResolver; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.relational.RowExpressionOptimizer; import com.facebook.presto.sql.relational.SqlToRowExpressionTranslator; @@ -92,7 +92,7 @@ Expression simplifyExpression(Expression expression) Map, Type> expressionTypes = getExpressionTypes(expression, TypeProvider.empty()); ExpressionInterpreter interpreter = ExpressionInterpreter.expressionOptimizer(expression, metadata, TEST_SESSION, expressionTypes); - Object value = interpreter.optimize(NoOpSymbolResolver.INSTANCE); + Object value = interpreter.optimize(NoOpVariableResolver.INSTANCE); return literalEncoder.toExpression(value, expressionTypes.get(NodeRef.of(expression))); } diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPickTableLayout.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPickTableLayout.java index 4f6257829d15a..2b8aedf2260c9 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPickTableLayout.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPickTableLayout.java @@ -41,6 +41,7 @@ import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.filter; import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.values; import static com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder.expression; +import static com.facebook.presto.sql.relational.Expressions.variable; import static io.airlift.slice.Slices.utf8Slice; public class TestPickTableLayout @@ -104,6 +105,17 @@ public void eliminateTableScanWhenNoLayoutExist() ImmutableList.of(p.variable("orderstatus", createVarcharType(1))), ImmutableMap.of(p.variable("orderstatus", createVarcharType(1)), new TpchColumnHandle("orderstatus", createVarcharType(1)))))) .matches(values("A")); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("orderstatus", createVarcharType(1)); + return p.filter(p.rowExpression("orderstatus = 'G'"), + p.tableScan( + ordersTableHandle, + ImmutableList.of(variable("orderstatus", createVarcharType(1))), + ImmutableMap.of(variable("orderstatus", createVarcharType(1)), new TpchColumnHandle("orderstatus", createVarcharType(1))))); + }) + .matches(values("A")); } @Test @@ -119,6 +131,19 @@ public void replaceWithExistsWhenNoLayoutExist() TupleDomain.none(), TupleDomain.none()))) .matches(values("A")); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("nationkey"); + return p.filter(p.rowExpression("nationkey = BIGINT '44'"), + p.tableScan( + nationTableHandle, + ImmutableList.of(variable("nationkey", BIGINT)), + ImmutableMap.of(variable("nationkey", BIGINT), columnHandle), + TupleDomain.none(), + TupleDomain.none())); + }) + .matches(values("A")); } @Test @@ -133,6 +158,19 @@ public void doesNotFireIfRuleNotChangePlan() TupleDomain.all(), TupleDomain.all()))) .doesNotFire(); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("nationkey"); + return p.filter(p.rowExpression("nationkey % 17 = BIGINT '44' AND nationkey % 15 = BIGINT '43'"), + p.tableScan( + nationTableHandle, + ImmutableList.of(variable("nationkey", BIGINT)), + ImmutableMap.of(variable("nationkey", BIGINT), new TpchColumnHandle("nationkey", BIGINT)), + TupleDomain.all(), + TupleDomain.all())); + }) + .doesNotFire(); } @Test @@ -165,6 +203,18 @@ public void ruleAddedTableLayoutToFilterTableScan() ImmutableMap.of(p.variable("orderstatus", createVarcharType(1)), new TpchColumnHandle("orderstatus", createVarcharType(1)))))) .matches( constrainedTableScanWithTableLayout("orders", filterConstraint, ImmutableMap.of("orderstatus", "orderstatus"))); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("orderstatus", createVarcharType(1)); + return p.filter(p.rowExpression("orderstatus = CAST ('F' AS VARCHAR(1))"), + p.tableScan( + ordersTableHandle, + ImmutableList.of(variable("orderstatus", createVarcharType(1))), + ImmutableMap.of(variable("orderstatus", createVarcharType(1)), new TpchColumnHandle("orderstatus", createVarcharType(1))))); + }) + .matches( + constrainedTableScanWithTableLayout("orders", filterConstraint, ImmutableMap.of("orderstatus", "orderstatus"))); } @Test @@ -181,6 +231,21 @@ public void ruleAddedNewTableLayoutIfTableScanHasEmptyConstraint() "orders", ImmutableMap.of("orderstatus", singleValue(createVarcharType(1), utf8Slice("F"))), ImmutableMap.of("orderstatus", "orderstatus"))); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("orderstatus", createVarcharType(1)); + return p.filter(expression("orderstatus = 'F'"), + p.tableScan( + ordersTableHandle, + ImmutableList.of(variable("orderstatus", createVarcharType(1))), + ImmutableMap.of(variable("orderstatus", createVarcharType(1)), new TpchColumnHandle("orderstatus", createVarcharType(1))))); + }) + .matches( + constrainedTableScanWithTableLayout( + "orders", + ImmutableMap.of("orderstatus", singleValue(createVarcharType(1), utf8Slice("F"))), + ImmutableMap.of("orderstatus", "orderstatus"))); } @Test @@ -197,6 +262,20 @@ public void ruleWithPushdownableToTableLayoutPredicate() "orders", ImmutableMap.of("orderstatus", singleValue(orderStatusType, utf8Slice("O"))), ImmutableMap.of("orderstatus", "orderstatus"))); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("orderstatus", orderStatusType); + return p.filter(p.rowExpression("orderstatus = 'O'"), + p.tableScan( + ordersTableHandle, + ImmutableList.of(variable("orderstatus", orderStatusType)), + ImmutableMap.of(variable("orderstatus", orderStatusType), new TpchColumnHandle("orderstatus", orderStatusType)))); + }) + .matches(constrainedTableScanWithTableLayout( + "orders", + ImmutableMap.of("orderstatus", singleValue(orderStatusType, utf8Slice("O"))), + ImmutableMap.of("orderstatus", "orderstatus"))); } @Test @@ -215,5 +294,21 @@ public void nonDeterministicPredicate() "orders", ImmutableMap.of("orderstatus", singleValue(orderStatusType, utf8Slice("O"))), ImmutableMap.of("orderstatus", "orderstatus")))); + + tester().assertThat(pickTableLayout.pickTableLayoutForPredicate()) + .on(p -> { + p.variable("orderstatus", orderStatusType); + return p.filter(p.rowExpression("orderstatus = 'O' AND rand() = 0"), + p.tableScan( + ordersTableHandle, + ImmutableList.of(variable("orderstatus", orderStatusType)), + ImmutableMap.of(variable("orderstatus", orderStatusType), new TpchColumnHandle("orderstatus", orderStatusType)))); + }) + .matches( + filter("rand() = 0", + constrainedTableScanWithTableLayout( + "orders", + ImmutableMap.of("orderstatus", singleValue(orderStatusType, utf8Slice("O"))), + ImmutableMap.of("orderstatus", "orderstatus")))); } }