Remove SPI dependency + nits

presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java

@@ -2294,9 +2294,9 @@ public static DataSize getFilterAndProjectMinOutputPageSize(Session session)
         return session.getSystemProperty(FILTER_AND_PROJECT_MIN_OUTPUT_PAGE_SIZE, DataSize.class);
     }
 
-    public static boolean isMaterializeAllCtes(Session session)
+    public static CteMaterializationStrategy getCteMaterializationStrategy(Session session)
     {
-        return session.getSystemProperty(CTE_MATERIALIZATION_STRATEGY, CteMaterializationStrategy.class).equals(CteMaterializationStrategy.ALL);
+        return session.getSystemProperty(CTE_MATERIALIZATION_STRATEGY, CteMaterializationStrategy.class);
     }
 
     public static int getFilterAndProjectMinOutputPageRowCount(Session session)

presto-main/src/main/java/com/facebook/presto/sql/planner/BasePlanFragmenter.java

@@ -207,7 +207,7 @@ public PlanNode visitSequence(SequenceNode node, RewriteContext<FragmentProperti
         // Since this is topologically sorted by the LogicalCtePlanner, need to make sure that execution order follows
         // Can be optimized further to avoid non dependents from getting blocked
         int n = node.getCteProducers().size();
-        Preconditions.checkArgument(n >= 1, "Sequence Node has 0 cte producers");
+        checkArgument(n >= 1, "Sequence Node has 0 cte producers");
         PlanNode source = node.getCteProducers().get(n - 1);
         FragmentProperties childProperties = new FragmentProperties(new PartitioningScheme(
                 Partitioning.create(SINGLE_DISTRIBUTION, ImmutableList.of()),

presto-main/src/main/java/com/facebook/presto/sql/planner/RelationPlanner.java

@@ -107,8 +107,8 @@
 import java.util.Set;
 import java.util.stream.IntStream;
 
+import static com.facebook.presto.SystemSessionProperties.getCteMaterializationStrategy;
 import static com.facebook.presto.SystemSessionProperties.getQueryAnalyzerTimeout;
-import static com.facebook.presto.SystemSessionProperties.isMaterializeAllCtes;
 import static com.facebook.presto.common.type.TypeUtils.isEnumType;
 import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
 import static com.facebook.presto.spi.StandardErrorCode.QUERY_PLANNING_TIMEOUT;
@@ -118,6 +118,7 @@
 import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.getSourceLocation;
 import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.isEqualComparisonExpression;
 import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.resolveEnumLiteral;
+import static com.facebook.presto.sql.analyzer.FeaturesConfig.CteMaterializationStrategy.ALL;
 import static com.facebook.presto.sql.analyzer.SemanticExceptions.notSupportedException;
 import static com.facebook.presto.sql.planner.PlannerUtils.newVariable;
 import static com.facebook.presto.sql.planner.TranslateExpressionsUtil.toRowExpression;
@@ -186,7 +187,7 @@ protected RelationPlan visitTable(Table node, SqlPlannerContext context)
             context.getNestedCteStack().push(cteName, namedQuery.getQuery());
             RelationPlan subPlan = process(namedQuery.getQuery(), context);
             context.getNestedCteStack().pop(namedQuery.getQuery());
-            boolean shouldBeMaterialized = isMaterializeAllCtes(session) && isCteMaterializable(subPlan.getRoot().getOutputVariables());
+            boolean shouldBeMaterialized = getCteMaterializationStrategy(session).equals(ALL) && isCteMaterializable(subPlan.getRoot().getOutputVariables());
             session.getCteInformationCollector().addCTEReference(cteName, namedQuery.isFromView(), shouldBeMaterialized);
             if (shouldBeMaterialized) {
                 subPlan = new RelationPlan(

presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/LogicalCteOptimizer.java

@@ -39,8 +39,9 @@
 import java.util.Optional;
 import java.util.Stack;
 
-import static com.facebook.presto.SystemSessionProperties.isMaterializeAllCtes;
+import static com.facebook.presto.SystemSessionProperties.getCteMaterializationStrategy;
 import static com.facebook.presto.common.type.BigintType.BIGINT;
+import static com.facebook.presto.sql.analyzer.FeaturesConfig.CteMaterializationStrategy.ALL;
 import static com.google.common.base.Preconditions.checkArgument;
 
 /*
@@ -79,7 +80,7 @@ public LogicalCteOptimizer(Metadata metadata)
     @Override
     public PlanOptimizerResult optimize(PlanNode plan, Session session, TypeProvider types, VariableAllocator variableAllocator, PlanNodeIdAllocator idAllocator, WarningCollector warningCollector)
     {
-        if (!isMaterializeAllCtes(session)
+        if (!getCteMaterializationStrategy(session).equals(ALL)
                 || session.getCteInformationCollector().getCTEInformationList().stream().noneMatch(CTEInformation::isMaterialized)) {
             return PlanOptimizerResult.optimizerResult(plan, false);
         }

presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PhysicalCteOptimizer.java

@@ -45,14 +45,15 @@
 import java.util.Map;
 import java.util.Optional;
 
+import static com.facebook.presto.SystemSessionProperties.getCteMaterializationStrategy;
 import static com.facebook.presto.SystemSessionProperties.getHashPartitionCount;
 import static com.facebook.presto.SystemSessionProperties.getPartitioningProviderCatalog;
-import static com.facebook.presto.SystemSessionProperties.isMaterializeAllCtes;
 import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
 import static com.facebook.presto.sql.TemporaryTableUtil.assignPartitioningVariables;
 import static com.facebook.presto.sql.TemporaryTableUtil.assignTemporaryTableColumnNames;
 import static com.facebook.presto.sql.TemporaryTableUtil.createTemporaryTableScan;
 import static com.facebook.presto.sql.TemporaryTableUtil.createTemporaryTableWriteWithoutExchanges;
+import static com.facebook.presto.sql.analyzer.FeaturesConfig.CteMaterializationStrategy.ALL;
 import static com.facebook.presto.sql.planner.optimizations.CteUtils.getCtePartitionIndex;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static java.lang.String.format;
@@ -86,7 +87,7 @@ public PhysicalCteOptimizer(Metadata metadata)
     @Override
     public PlanOptimizerResult optimize(PlanNode plan, Session session, TypeProvider types, VariableAllocator variableAllocator, PlanNodeIdAllocator idAllocator, WarningCollector warningCollector)
     {
-        if (!isMaterializeAllCtes(session)
+        if (!getCteMaterializationStrategy(session).equals(ALL)
                 || session.getCteInformationCollector().getCTEInformationList().stream().noneMatch(CTEInformation::isMaterialized)) {
             return PlanOptimizerResult.optimizerResult(plan, false);
         }

presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java

@@ -1397,7 +1397,7 @@ public NodeRepresentation addNode(PlanNode rootNode, String name, String identif
     public static String getCteExecutionOrder(SequenceNode node)
     {
         List<CteProducerNode> cteProducers = node.getCteProducers().stream()
-                .peek(c -> checkArgument(c instanceof CteProducerNode, "Child of sequence node is not an instance of CteProducerNode"))
+                .filter(c -> (c instanceof CteProducerNode))
                 .map(CteProducerNode.class::cast)
                 .collect(Collectors.toList());
         if (cteProducers.isEmpty()) {

presto-spi/pom.xml

@@ -88,7 +88,7 @@
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>
-            <scope>provided</scope>
+            <scope>test</scope>
         </dependency>
 
         <dependency>

presto-spi/src/main/java/com/facebook/presto/spi/plan/CteConsumerNode.java

@@ -17,15 +17,13 @@
 import com.facebook.presto.spi.relation.VariableReferenceExpression;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.collect.ImmutableList;
 
 import javax.annotation.concurrent.Immutable;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 @Immutable
 public final class CteConsumerNode
         extends PlanNode
@@ -59,7 +57,7 @@ public CteConsumerNode(
     public List<PlanNode> getSources()
     {
         // CteConsumer should be the leaf node
-        return ImmutableList.of();
+        return Collections.emptyList();
     }
 
     @Override
@@ -92,4 +90,11 @@ public String getCteName()
     {
         return cteName;
     }
+
+    private static void checkArgument(boolean condition, String message)
+    {
+        if (!condition) {
+            throw new IllegalArgumentException(message);
+        }
+    }
 }

presto-spi/src/main/java/com/facebook/presto/spi/plan/CteProducerNode.java

@@ -17,14 +17,12 @@
 import com.facebook.presto.spi.relation.VariableReferenceExpression;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.collect.Iterables;
 
 import javax.annotation.concurrent.Immutable;
 
 import java.util.List;
 import java.util.Optional;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Collections.singletonList;
 
 @Immutable
@@ -86,7 +84,7 @@ public List<VariableReferenceExpression> getOutputVariables()
     public PlanNode replaceChildren(List<PlanNode> newChildren)
     {
         checkArgument(newChildren.size() == 1, "expected newChildren to contain 1 node");
-        return new CteProducerNode(newChildren.get(0).getSourceLocation(), getId(), getStatsEquivalentPlanNode(), Iterables.getOnlyElement(newChildren),
+        return new CteProducerNode(newChildren.get(0).getSourceLocation(), getId(), getStatsEquivalentPlanNode(), newChildren.get(0),
                 cteName, rowCountVariable, originalOutputVariables);
     }
 
@@ -111,4 +109,11 @@ public VariableReferenceExpression getRowCountVariable()
     {
         return rowCountVariable;
     }
+
+    private static void checkArgument(boolean condition, String message)
+    {
+        if (!condition) {
+            throw new IllegalArgumentException(message);
+        }
+    }
 }

presto-spi/src/main/java/com/facebook/presto/spi/plan/CteReferenceNode.java

@@ -17,14 +17,12 @@
 import com.facebook.presto.spi.relation.VariableReferenceExpression;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.collect.Iterables;
 
 import javax.annotation.concurrent.Immutable;
 
 import java.util.List;
 import java.util.Optional;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Collections.singletonList;
 
 @Immutable
@@ -78,7 +76,7 @@ public List<VariableReferenceExpression> getOutputVariables()
     public PlanNode replaceChildren(List<PlanNode> newChildren)
     {
         checkArgument(newChildren.size() == 1, "expected newChildren to contain 1 node");
-        return new CteReferenceNode(newChildren.get(0).getSourceLocation(), getId(), getStatsEquivalentPlanNode(), Iterables.getOnlyElement(newChildren), cteName);
+        return new CteReferenceNode(newChildren.get(0).getSourceLocation(), getId(), getStatsEquivalentPlanNode(), newChildren.get(0), cteName);
     }
 
     @Override
@@ -97,4 +95,11 @@ public String getCteName()
     {
         return cteName;
     }
+
+    private static void checkArgument(boolean condition, String message)
+    {
+        if (!condition) {
+            throw new IllegalArgumentException(message);
+        }
+    }
 }