Enforce/clarify the creation of PreferredProperties

Rewrite PreferredProperties to enforce that callers can only create valid forms of PreferredProperties. Also refactor the logic around PreferredProperties so that they are consolidated in the class itself instead of spread around the different callsites.
prestodb · Jun 6, 2015 · ede0059 · ede0059
1 parent 0f17fdf
commit ede0059
Show file tree

Hide file tree

Showing 3 changed files with 260 additions and 110 deletions.
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
@@ -34,7 +34,6 @@
 import com.facebook.presto.sql.planner.PlanNodeIdAllocator;
 import com.facebook.presto.sql.planner.Symbol;
 import com.facebook.presto.sql.planner.SymbolAllocator;
-import com.facebook.presto.sql.planner.optimizations.PreferredProperties.PartitioningPreferences;
 import com.facebook.presto.sql.planner.plan.AggregationNode;
 import com.facebook.presto.sql.planner.plan.ChildReplacer;
 import com.facebook.presto.sql.planner.plan.DistinctLimitNode;
@@ -163,7 +162,7 @@ protected PlanWithProperties visitPlan(PlanNode node, PreferredProperties prefer
         public PlanWithProperties visitProject(ProjectNode node, PreferredProperties preferred)
         {
             Map<Symbol, Symbol> identities = computeIdentityTranslations(node.getAssignments());
-            PreferredProperties translatedPreferred = PreferredProperties.translate(preferred, identities);
+            PreferredProperties translatedPreferred = preferred.translate(symbol -> Optional.ofNullable(identities.get(symbol)));
 
             return rebaseAndDeriveProperties(node, planChild(node, translatedPreferred));
         }
@@ -372,7 +371,7 @@ public PlanWithProperties visitWindow(WindowNode node, PreferredProperties prefe
         public PlanWithProperties visitRowNumber(RowNumberNode node, PreferredProperties preferred)
         {
             if (node.getPartitionBy().isEmpty()) {
-                PlanWithProperties child = planChild(node, PreferredProperties.unpartitioned());
+                PlanWithProperties child = planChild(node, PreferredProperties.undistributed());
 
                 if (child.getProperties().isDistributed()) {
                     child = withDerivedProperties(
@@ -459,7 +458,7 @@ public PlanWithProperties visitTopN(TopNNode node, PreferredProperties preferred
         @Override
         public PlanWithProperties visitSort(SortNode node, PreferredProperties preferred)
         {
-            PlanWithProperties child = planChild(node, PreferredProperties.unpartitioned());
+            PlanWithProperties child = planChild(node, PreferredProperties.undistributed());
 
             if (child.getProperties().isDistributed()) {
                 child = withDerivedProperties(
@@ -787,7 +786,7 @@ public PlanWithProperties visitIndexSource(IndexSourceNode node, PreferredProper
         @Override
         public PlanWithProperties visitUnion(UnionNode node, PreferredProperties preferred)
         {
-            if (!preferred.getPartitioningProperties().isPresent() || !preferred.getPartitioningProperties().get().isHashPartitioned()) {
+            if (!preferred.getGlobalProperties().isPresent() || !preferred.getGlobalProperties().get().isHashPartitioned()) {
                 // first, classify children into partitioned and unpartitioned
                 List<PlanNode> unpartitionedChildren = new ArrayList<>();
                 List<List<Symbol>> unpartitionedOutputLayouts = new ArrayList<>();
@@ -842,7 +841,7 @@ public PlanWithProperties visitUnion(UnionNode node, PreferredProperties preferr
             }
 
             // hash partition the sources
-            List<Symbol> hashingColumns = preferred.getPartitioningProperties().get().getHashPartitioningColumns().get();
+            List<Symbol> hashingColumns = preferred.getGlobalProperties().get().getPartitioningProperties().get().getHashingOrder().get();
 
             ImmutableList.Builder<PlanNode> partitionedSources = ImmutableList.builder();
             ImmutableListMultimap.Builder<Symbol, Symbol> outputToSourcesMapping = ImmutableListMultimap.builder();
@@ -956,17 +955,17 @@ private static <T> boolean hasLocalOptimization(List<LocalProperty<T>> desiredLa
 
     private static boolean meetsPartitioningRequirements(PreferredProperties preferred, ActualProperties actual)
     {
-        if (!preferred.getPartitioningProperties().isPresent()) {
+        if (!preferred.getGlobalProperties().isPresent()) {
             return true;
         }
-        PartitioningPreferences partitioningPreferences = preferred.getPartitioningProperties().get();
+        PreferredProperties.Global preferredGlobal = preferred.getGlobalProperties().get();
-        if (!partitioningPreferences.isPartitioned()) {
+        if (!preferredGlobal.isDistributed()) {
             return !actual.isDistributed();
         }
-        if (!partitioningPreferences.getPartitioningColumns().isPresent()) {
+        if (!preferredGlobal.getPartitioningProperties().isPresent()) {
             return actual.isDistributed();
         }
-        return actual.isPartitionedOn(partitioningPreferences.getPartitioningColumns().get());
+        return actual.isPartitionedOn(preferredGlobal.getPartitioningProperties().get().getPartitioningColumns());
     }
 
     // Prefer the match result that satisfied the most requirements