Make PlanOptimizers not implement Provider

We were previously relying on that for injection, which makes it
hard to track down where the list of optimizers comes from
when debugging.

presto-main/src/main/java/com/facebook/presto/execution/SqlQueryExecution.java

@@ -41,6 +41,7 @@
 import com.facebook.presto.sql.planner.Plan;
 import com.facebook.presto.sql.planner.PlanFragmenter;
 import com.facebook.presto.sql.planner.PlanNodeIdAllocator;
+import com.facebook.presto.sql.planner.PlanOptimizers;
 import com.facebook.presto.sql.planner.StageExecutionPlan;
 import com.facebook.presto.sql.planner.SubPlan;
 import com.facebook.presto.sql.planner.optimizations.PlanOptimizer;
@@ -501,7 +502,7 @@ public static class SqlQueryExecutionFactory
                 SplitManager splitManager,
                 NodePartitioningManager nodePartitioningManager,
                 NodeScheduler nodeScheduler,
-                List<PlanOptimizer> planOptimizers,
+                PlanOptimizers planOptimizers,
                 RemoteTaskFactory remoteTaskFactory,
                 TransactionManager transactionManager,
                 @ForQueryExecution ExecutorService executor,
@@ -518,7 +519,7 @@ public static class SqlQueryExecutionFactory
             this.splitManager = requireNonNull(splitManager, "splitManager is null");
             this.nodePartitioningManager = requireNonNull(nodePartitioningManager, "nodePartitioningManager is null");
             this.nodeScheduler = requireNonNull(nodeScheduler, "nodeScheduler is null");
-            this.planOptimizers = requireNonNull(planOptimizers, "planOptimizers is null");
+            requireNonNull(planOptimizers, "planOptimizers is null");
             this.remoteTaskFactory = requireNonNull(remoteTaskFactory, "remoteTaskFactory is null");
             this.transactionManager = requireNonNull(transactionManager, "transactionManager is null");
             requireNonNull(featuresConfig, "featuresConfig is null");
@@ -528,6 +529,8 @@ public static class SqlQueryExecutionFactory
             this.queryExplainer = requireNonNull(queryExplainer, "queryExplainer is null");
 
             this.executionPolicies = requireNonNull(executionPolicies, "schedulerPolicies is null");
+
+            this.planOptimizers = planOptimizers.get();
         }
 
         @Override

presto-main/src/main/java/com/facebook/presto/server/ServerMainModule.java

@@ -99,8 +99,7 @@
 import com.facebook.presto.sql.planner.CompilerConfig;
 import com.facebook.presto.sql.planner.LocalExecutionPlanner;
 import com.facebook.presto.sql.planner.NodePartitioningManager;
-import com.facebook.presto.sql.planner.PlanOptimizersFactory;
-import com.facebook.presto.sql.planner.optimizations.PlanOptimizer;
+import com.facebook.presto.sql.planner.PlanOptimizers;
 import com.facebook.presto.sql.tree.Expression;
 import com.facebook.presto.sql.tree.FunctionCall;
 import com.facebook.presto.transaction.ForTransactionManager;
@@ -124,7 +123,6 @@
 
 import javax.inject.Singleton;
 
-import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
@@ -370,7 +368,7 @@ protected void setup(Binder binder)
         configBinder(binder).bindConfig(PluginManagerConfig.class);
 
         // optimizers
-        binder.bind(new TypeLiteral<List<PlanOptimizer>>() {}).toProvider(PlanOptimizersFactory.class).in(Scopes.SINGLETON);
+        binder.bind(PlanOptimizers.class).in(Scopes.SINGLETON);
 
         // block encodings
         binder.bind(BlockEncodingManager.class).in(Scopes.SINGLETON);

presto-main/src/main/java/com/facebook/presto/sql/analyzer/QueryExplainer.java

@@ -22,6 +22,7 @@
 import com.facebook.presto.sql.planner.Plan;
 import com.facebook.presto.sql.planner.PlanFragmenter;
 import com.facebook.presto.sql.planner.PlanNodeIdAllocator;
+import com.facebook.presto.sql.planner.PlanOptimizers;
 import com.facebook.presto.sql.planner.PlanPrinter;
 import com.facebook.presto.sql.planner.SubPlan;
 import com.facebook.presto.sql.planner.optimizations.PlanOptimizer;
@@ -49,14 +50,14 @@ public class QueryExplainer
 
     @Inject
     public QueryExplainer(
-            List<PlanOptimizer> planOptimizers,
+            PlanOptimizers planOptimizers,
             Metadata metadata,
             AccessControl accessControl,
             SqlParser sqlParser,
             Map<Class<? extends Statement>, DataDefinitionTask<?>> dataDefinitionTask,
             FeaturesConfig featuresConfig)
     {
-        this(planOptimizers,
+        this(planOptimizers.get(),
                 metadata,
                 accessControl,
                 sqlParser,

presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizersFactory.java → presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java

@@ -50,22 +50,19 @@
 import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
 
-import javax.inject.Provider;
-
 import java.util.List;
 
-public class PlanOptimizersFactory
-        implements Provider<List<PlanOptimizer>>
+public class PlanOptimizers
 {
     private final List<PlanOptimizer> optimizers;
 
     @Inject
-    public PlanOptimizersFactory(Metadata metadata, SqlParser sqlParser, FeaturesConfig featuresConfig)
+    public PlanOptimizers(Metadata metadata, SqlParser sqlParser, FeaturesConfig featuresConfig)
     {
         this(metadata, sqlParser, featuresConfig, false);
     }
 
-    public PlanOptimizersFactory(Metadata metadata, SqlParser sqlParser, FeaturesConfig featuresConfig, boolean forceSingleNode)
+    public PlanOptimizers(Metadata metadata, SqlParser sqlParser, FeaturesConfig featuresConfig, boolean forceSingleNode)
     {
         ImmutableList.Builder<PlanOptimizer> builder = ImmutableList.builder();
 
@@ -137,8 +134,7 @@ public PlanOptimizersFactory(Metadata metadata, SqlParser sqlParser, FeaturesCon
         this.optimizers = builder.build();
     }
 
-    @Override
-    public synchronized List<PlanOptimizer> get()
+    public List<PlanOptimizer> get()
     {
         return optimizers;
     }

presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java

@@ -102,7 +102,7 @@
 import com.facebook.presto.sql.planner.Plan;
 import com.facebook.presto.sql.planner.PlanFragmenter;
 import com.facebook.presto.sql.planner.PlanNodeIdAllocator;
-import com.facebook.presto.sql.planner.PlanOptimizersFactory;
+import com.facebook.presto.sql.planner.PlanOptimizers;
 import com.facebook.presto.sql.planner.PlanPrinter;
 import com.facebook.presto.sql.planner.SubPlan;
 import com.facebook.presto.sql.planner.optimizations.PlanOptimizer;
@@ -138,8 +138,6 @@
 import io.airlift.units.Duration;
 import org.intellij.lang.annotations.Language;
 
-import javax.inject.Provider;
-
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -607,16 +605,16 @@ public Plan createPlan(Session session, @Language("SQL") String sql, LogicalPlan
                 .setExperimentalSyntaxEnabled(true)
                 .setDistributedIndexJoinsEnabled(false)
                 .setOptimizeHashGeneration(true);
-        PlanOptimizersFactory planOptimizersFactory = new PlanOptimizersFactory(metadata, sqlParser, featuresConfig, true);
-        return createPlan(session, sql, featuresConfig, planOptimizersFactory, stage);
+        PlanOptimizers planOptimizers = new PlanOptimizers(metadata, sqlParser, featuresConfig, true);
+        return createPlan(session, sql, featuresConfig, planOptimizers.get(), stage);
     }
 
-    public Plan createPlan(Session session, @Language("SQL") String sql, FeaturesConfig featuresConfig, Provider<List<PlanOptimizer>> optimizerProvider)
+    public Plan createPlan(Session session, @Language("SQL") String sql, FeaturesConfig featuresConfig, List<PlanOptimizer> optimizers)
     {
-        return createPlan(session, sql, featuresConfig, optimizerProvider, LogicalPlanner.Stage.OPTIMIZED_AND_VALIDATED);
+        return createPlan(session, sql, featuresConfig, optimizers, LogicalPlanner.Stage.OPTIMIZED_AND_VALIDATED);
     }
 
-    public Plan createPlan(Session session, @Language("SQL") String sql, FeaturesConfig featuresConfig, Provider<List<PlanOptimizer>> optimizerProvider, LogicalPlanner.Stage stage)
+    public Plan createPlan(Session session, @Language("SQL") String sql, FeaturesConfig featuresConfig, List<PlanOptimizer> optimizers, LogicalPlanner.Stage stage)
     {
         Statement wrapped = sqlParser.createStatement(sql);
         Statement statement = unwrapExecuteStatement(wrapped, sqlParser, session);
@@ -632,15 +630,15 @@ public Plan createPlan(Session session, @Language("SQL") String sql, FeaturesCon
         PlanNodeIdAllocator idAllocator = new PlanNodeIdAllocator();
 
         QueryExplainer queryExplainer = new QueryExplainer(
-                optimizerProvider.get(),
+                optimizers,
                 metadata,
                 accessControl,
                 sqlParser,
                 dataDefinitionTask,
                 featuresConfig.isExperimentalSyntaxEnabled());
         Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.of(queryExplainer), featuresConfig.isExperimentalSyntaxEnabled(), parameters);
 
-        LogicalPlanner logicalPlanner = new LogicalPlanner(session, optimizerProvider.get(), idAllocator, metadata, sqlParser);
+        LogicalPlanner logicalPlanner = new LogicalPlanner(session, optimizers, idAllocator, metadata, sqlParser);
 
         Analysis analysis = analyzer.analyze(statement);
         return logicalPlanner.plan(analysis, stage);

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

@@ -31,8 +31,6 @@
 import org.intellij.lang.annotations.Language;
 import org.testng.annotations.Test;
 
-import javax.inject.Provider;
-
 import java.util.List;
 import java.util.Optional;
 
@@ -334,11 +332,11 @@ private Plan unitPlan(@Language("SQL") String sql)
                 .setExperimentalSyntaxEnabled(true)
                 .setDistributedIndexJoinsEnabled(false)
                 .setOptimizeHashGeneration(true);
-        Provider<List<PlanOptimizer>> optimizerProvider = () -> ImmutableList.of(
+        List<PlanOptimizer> optimizers = ImmutableList.of(
                         new UnaliasSymbolReferences(),
                         new PruneIdentityProjections(),
                         new MergeIdenticalWindows(),
                         new PruneUnreferencedOutputs());
-        return queryRunner.inTransaction(transactionSession -> queryRunner.createPlan(transactionSession, sql, featuresConfig, optimizerProvider));
+        return queryRunner.inTransaction(transactionSession -> queryRunner.createPlan(transactionSession, sql, featuresConfig, optimizers));
     }
 }

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueryFramework.java

@@ -20,7 +20,7 @@
 import com.facebook.presto.sql.analyzer.FeaturesConfig;
 import com.facebook.presto.sql.analyzer.QueryExplainer;
 import com.facebook.presto.sql.parser.SqlParser;
-import com.facebook.presto.sql.planner.PlanOptimizersFactory;
+import com.facebook.presto.sql.planner.PlanOptimizers;
 import com.facebook.presto.sql.planner.optimizations.PlanOptimizer;
 import com.facebook.presto.sql.tree.ExplainType;
 import com.facebook.presto.testing.MaterializedResult;
@@ -292,7 +292,7 @@ private QueryExplainer getQueryExplainer()
         Metadata metadata = queryRunner.getMetadata();
         FeaturesConfig featuresConfig = new FeaturesConfig().setExperimentalSyntaxEnabled(true).setOptimizeHashGeneration(true);
         boolean forceSingleNode = queryRunner.getNodeCount() == 1;
-        List<PlanOptimizer> optimizers = new PlanOptimizersFactory(metadata, sqlParser, featuresConfig, forceSingleNode).get();
+        List<PlanOptimizer> optimizers = new PlanOptimizers(metadata, sqlParser, featuresConfig, forceSingleNode).get();
         return new QueryExplainer(
                 optimizers,
                 metadata,