From 535f940621bf9363f15468d18dcb1173480e8339 Mon Sep 17 00:00:00 2001 From: Karol Sobczak Date: Wed, 9 Aug 2017 12:51:52 +0200 Subject: [PATCH] Add Lookup#resolveGroup method and make Lookup#resolve depricated In the future, equivalence based memo would store multiple plan nodes per group. Therefore resolve() that returns singleton should be unused. --- .../planner/iterative/IterativeOptimizer.java | 2 +- .../presto/sql/planner/iterative/Lookup.java | 24 +++++++++++++++---- .../presto/sql/planner/iterative/Memo.java | 3 ++- .../iterative/rule/test/RuleAssert.java | 3 ++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/IterativeOptimizer.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/IterativeOptimizer.java index 0aa5bfbb6318..47e219321541 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/IterativeOptimizer.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/IterativeOptimizer.java @@ -76,7 +76,7 @@ public PlanNode optimize(PlanNode plan, Session session, Map types } Memo memo = new Memo(idAllocator, plan); - Lookup lookup = Lookup.from(memo::resolve); + Lookup lookup = Lookup.from(planNode -> ImmutableList.of(memo.resolve(planNode))); Matcher matcher = new PlanNodeMatcher(lookup); Duration timeout = SystemSessionProperties.getOptimizerTimeout(session); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Lookup.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Lookup.java index 107ee2d7a5a2..610bac4216c7 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Lookup.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Lookup.java @@ -14,10 +14,13 @@ package com.facebook.presto.sql.planner.iterative; import com.facebook.presto.sql.planner.plan.PlanNode; +import com.google.common.collect.ImmutableList; +import java.util.List; import java.util.function.Function; import static com.google.common.base.Verify.verify; +import static com.google.common.collect.Iterables.getOnlyElement; public interface Lookup { @@ -28,7 +31,20 @@ public interface Lookup * If the node is not a GroupReference, it returns the * argument as is. */ - PlanNode resolve(PlanNode node); + @Deprecated + default PlanNode resolve(PlanNode node) + { + return getOnlyElement(resolveGroup(node)); + } + + /** + * Resolves nodes by materializing GroupReference nodes + * representing symbolic references to other nodes. + *

+ * If the node is not a GroupReference, it returns the + * singleton of the argument node. + */ + List resolveGroup(PlanNode node); /** * A Lookup implementation that does not perform lookup. It satisfies contract @@ -38,18 +54,18 @@ static Lookup noLookup() { return node -> { verify(!(node instanceof GroupReference), "Unexpected GroupReference"); - return node; + return ImmutableList.of(node); }; } - static Lookup from(Function resolver) + static Lookup from(Function> resolver) { return node -> { if (node instanceof GroupReference) { return resolver.apply((GroupReference) node); } - return node; + return ImmutableList.of(node); }; } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Memo.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Memo.java index eddd93a7a71c..8c52a8438b5a 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Memo.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/Memo.java @@ -15,6 +15,7 @@ import com.facebook.presto.sql.planner.PlanNodeIdAllocator; import com.facebook.presto.sql.planner.plan.PlanNode; +import com.google.common.collect.ImmutableList; import java.util.HashMap; import java.util.HashSet; @@ -92,7 +93,7 @@ public PlanNode extract() private PlanNode extract(PlanNode node) { - return resolveGroupReferences(node, Lookup.from(this::resolve)); + return resolveGroupReferences(node, Lookup.from(planNode -> ImmutableList.of(this.resolve(planNode)))); } public PlanNode replace(int group, PlanNode node, String reason) diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/RuleAssert.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/RuleAssert.java index 7792cd1ab76b..38c09b5afef2 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/RuleAssert.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/RuleAssert.java @@ -33,6 +33,7 @@ import com.facebook.presto.sql.planner.plan.PlanNodeId; import com.facebook.presto.sql.planner.planPrinter.PlanPrinter; import com.facebook.presto.transaction.TransactionManager; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import java.util.Map; @@ -146,7 +147,7 @@ private RuleApplication applyRule() { SymbolAllocator symbolAllocator = new SymbolAllocator(symbols); Memo memo = new Memo(idAllocator, plan); - Lookup lookup = Lookup.from(memo::resolve); + Lookup lookup = Lookup.from(planNode -> ImmutableList.of(memo.resolve(planNode))); PlanNode memoRoot = memo.getNode(memo.getRootGroup());