Merge f0355e6 into 35a76e8

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionReferenceOptimizer.java

@@ -94,7 +94,7 @@ public Expression visitAggregator(Aggregator<?> node, AnalysisContext context) {
   class ExpressionMapBuilder extends LogicalPlanNodeVisitor<Void, Void> {
 
     @Override
-    protected Void visitNode(LogicalPlan plan, Void context) {
+    public Void visitNode(LogicalPlan plan, Void context) {
       plan.getChild().forEach(child -> child.accept(this, context));
       return null;
     }

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementor.java

@@ -21,8 +21,6 @@
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalEval;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalFilter;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalHead;
-import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalIndexScan;
-import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalIndexScanAggregation;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanNodeVisitor;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject;
@@ -144,18 +142,6 @@ public PhysicalPlan visitRelation(LogicalRelation node, C context) {
         + "implementing and optimizing logical plan with relation involved");
   }
 
-  @Override
-  public PhysicalPlan visitIndexScan(LogicalIndexScan plan, C context) {
-    throw new UnsupportedOperationException("Storage engine is responsible for "
-        + "implementing and optimizing logical plan with relation involved");
-  }
-
-  @Override
-  public PhysicalPlan visitIndexScanAggregation(LogicalIndexScanAggregation plan, C context) {
-    throw new UnsupportedOperationException("Storage engine is responsible for "
-        + "implementing and optimizing logical plan with relation involved");
-  }
-
   protected PhysicalPlan visitChild(LogicalPlan node, C context) {
     // Logical operators visited here must have a single child
     return node.getChild().get(0).accept(this, context);

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/Planner.java

@@ -56,14 +56,15 @@ public PhysicalPlan plan(LogicalPlan plan) {
     }
 
     Table table = storageEngine.getTable(tableName);
-    return table.implement(optimize(plan));
+    return table.implement(
+        table.optimize(optimize(plan)));
   }
 
   private String findTableName(LogicalPlan plan) {
     return plan.accept(new LogicalPlanNodeVisitor<String, Object>() {
 
       @Override
-      protected String visitNode(LogicalPlan node, Object context) {
+      public String visitNode(LogicalPlan node, Object context) {
         List<LogicalPlan> children = node.getChild();
         if (children.isEmpty()) {
           return "";

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanDSL.java

@@ -107,22 +107,6 @@ public static LogicalPlan rareTopN(LogicalPlan input, CommandType commandType, i
     return new LogicalRareTopN(input, commandType, noOfResults, Arrays.asList(fields), groupByList);
   }
 
-  public static LogicalPlan indexScan(String tableName, Expression filter) {
-    return new LogicalIndexScan(tableName, filter);
-  }
-
-  public static LogicalPlan indexScanAgg(String tableName, List<NamedAggregator> aggregators,
-                                         List<NamedExpression> groupByList) {
-    return new LogicalIndexScanAggregation(tableName, aggregators, groupByList);
-  }
-
-  public static LogicalPlan indexScanAgg(String tableName,
-                                         Expression filter,
-                                         List<NamedAggregator> aggregators,
-                                         List<NamedExpression> groupByList) {
-    return new LogicalIndexScanAggregation(tableName, filter, aggregators, groupByList);
-  }
-
   @SafeVarargs
   public LogicalPlan values(List<LiteralExpression>... values) {
     return new LogicalValues(Arrays.asList(values));

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitor.java

@@ -23,7 +23,7 @@
  */
 public abstract class LogicalPlanNodeVisitor<R, C> {
 
-  protected R visitNode(LogicalPlan plan, C context) {
+  public R visitNode(LogicalPlan plan, C context) {
     return null;
   }
 
@@ -78,12 +78,4 @@ public R visitValues(LogicalValues plan, C context) {
   public R visitRareTopN(LogicalRareTopN plan, C context) {
     return visitNode(plan, context);
   }
-
-  public R visitIndexScan(LogicalIndexScan plan, C context) {
-    return visitNode(plan, context);
-  }
-
-  public R visitIndexScanAggregation(LogicalIndexScanAggregation plan, C context) {
-    return visitNode(plan, context);
-  }
 }

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/optimizer/LogicalPlanOptimizer.java

@@ -21,10 +21,7 @@
 
 import com.amazon.opendistroforelasticsearch.sql.expression.DSL;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan;
-import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.rule.MergeAggAndIndexScan;
-import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.rule.MergeAggAndRelation;
 import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.rule.MergeFilterAndFilter;
-import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.rule.MergeFilterAndRelation;
 import com.facebook.presto.matching.Match;
 import java.util.Arrays;
 import java.util.List;
@@ -41,8 +38,10 @@ public class LogicalPlanOptimizer {
 
   private final List<Rule<?>> rules;
 
-  private LogicalPlanOptimizer(
-      List<Rule<?>> rules) {
+  /**
+   * Create {@link LogicalPlanOptimizer} with customized rules.
+   */
+  public LogicalPlanOptimizer(List<Rule<?>> rules) {
     this.rules = rules;
   }
 
@@ -51,9 +50,6 @@ private LogicalPlanOptimizer(
    */
   public static LogicalPlanOptimizer create(DSL dsl) {
     return new LogicalPlanOptimizer(Arrays.asList(
-        new MergeFilterAndRelation(),
-        new MergeAggAndIndexScan(),
-        new MergeAggAndRelation(),
         new MergeFilterAndFilter(dsl)));
   }
 
@@ -65,7 +61,7 @@ public LogicalPlan optimize(LogicalPlan plan) {
     optimized.replaceChildPlans(
         optimized.getChild().stream().map(this::optimize).collect(
             Collectors.toList()));
-    return internalOptimize(plan);
+    return internalOptimize(optimized);
   }
 
   private LogicalPlan internalOptimize(LogicalPlan plan) {

core/src/main/java/com/amazon/opendistroforelasticsearch/sql/storage/Table.java

@@ -38,4 +38,15 @@ public interface Table {
    */
   PhysicalPlan implement(LogicalPlan plan);
 
+  /**
+   * Optimize the {@link LogicalPlan} by storage engine rule.
+   * The default optimize solution is no optimization.
+   *
+   * @param plan logical plan.
+   * @return logical plan.
+   */
+  default LogicalPlan optimize(LogicalPlan plan) {
+    return plan;
+  }
+
 }

core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementorTest.java

@@ -48,8 +48,6 @@
 import com.amazon.opendistroforelasticsearch.sql.expression.aggregation.NamedAggregator;
 import com.amazon.opendistroforelasticsearch.sql.expression.window.WindowDefinition;
 import com.amazon.opendistroforelasticsearch.sql.expression.window.ranking.RowNumberFunction;
-import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalIndexScan;
-import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalIndexScanAggregation;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL;
 import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation;
@@ -213,18 +211,4 @@ public void visitWindowOperatorShouldReturnPhysicalWindowOperator() {
 
     assertEquals(physicalPlan, logicalPlan.accept(implementor, null));
   }
-
-  @Test
-  public void visitIndexScanShouldThrowException() {
-    assertThrows(UnsupportedOperationException.class,
-        () -> new LogicalIndexScan("test", filter).accept(implementor, null));
-  }
-
-  @Test
-  public void visitIndexScanAggShouldThrowException() {
-    assertThrows(UnsupportedOperationException.class,
-        () -> new LogicalIndexScanAggregation("test", Arrays.asList(aggregator),
-            Arrays.asList(groupBy)).accept(implementor,
-            null));
-  }
 }