Merge branch '2.x' into backport2.x-1933

Signed-off-by: Mitchell Gale <[email protected]>

Author: MitchellGale

build.gradle

@@ -87,7 +87,11 @@ spotless {
             include 'core/src/main/java/org/opensearch/sql/DataSourceSchemaName.java',
                     'core/src/test/java/org/opensearch/sql/data/**/*.java',
                     'core/src/test/java/org/opensearch/sql/config/**/*.java',
-                    'core/src/test/java/org/opensearch/sql/analysis/**/*.java'
+                    'core/src/test/java/org/opensearch/sql/analysis/**/*.java',
+                    'core/src/main/java/org/opensearch/sql/planner/**/*.java',
+                    'core/src/main/java/org/opensearch/sql/storage/**/*.java',
+                    'core/src/main/java/org/opensearch/sql/utils/**/*.java',
+                    'core/src/main/java/org/opensearch/sql/monitor/**/*.java'
             exclude '**/build/**', '**/build-*/**'
         }
         importOrder()

core/build.gradle

@@ -33,8 +33,6 @@ repositories {
     mavenCentral()
 }
 
-// Being ignored as a temporary measure before being removed in favour of
-// spotless https://github.com/opensearch-project/sql/issues/1101
 checkstyleTest.ignoreFailures = true
 checkstyleMain.ignoreFailures = true
 

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

@@ -161,9 +161,9 @@ public LogicalPlan visitRelation(Relation node, AnalysisContext context) {
               dataSourceSchemaIdentifierNameResolver.getIdentifierName());
     }
     table.getFieldTypes().forEach((k, v) -> curEnv.define(new Symbol(Namespace.FIELD_NAME, k), v));
-    table.getReservedFieldTypes().forEach(
-        (k, v) -> curEnv.addReservedWord(new Symbol(Namespace.FIELD_NAME, k), v)
-    );
+    table
+        .getReservedFieldTypes()
+        .forEach((k, v) -> curEnv.define(new Symbol(Namespace.HIDDEN_FIELD_NAME, k), v));
 
     // Put index name or its alias in index namespace on type environment so qualifier
     // can be removed when analyzing qualified name. The value (expr type) here doesn't matter.
@@ -207,12 +207,15 @@ public LogicalPlan visitTableFunction(TableFunction node, AnalysisContext contex
     TypeEnvironment curEnv = context.peek();
     Table table = tableFunctionImplementation.applyArguments();
     table.getFieldTypes().forEach((k, v) -> curEnv.define(new Symbol(Namespace.FIELD_NAME, k), v));
-    table.getReservedFieldTypes().forEach(
-        (k, v) -> curEnv.addReservedWord(new Symbol(Namespace.FIELD_NAME, k), v)
-    );
-    curEnv.define(new Symbol(Namespace.INDEX_NAME,
-            dataSourceSchemaIdentifierNameResolver.getIdentifierName()), STRUCT);
-    return new LogicalRelation(dataSourceSchemaIdentifierNameResolver.getIdentifierName(),
+    table
+        .getReservedFieldTypes()
+        .forEach((k, v) -> curEnv.define(new Symbol(Namespace.HIDDEN_FIELD_NAME, k), v));
+    curEnv.define(
+        new Symbol(
+            Namespace.INDEX_NAME, dataSourceSchemaIdentifierNameResolver.getIdentifierName()),
+        STRUCT);
+    return new LogicalRelation(
+        dataSourceSchemaIdentifierNameResolver.getIdentifierName(),
         tableFunctionImplementation.applyArguments());
   }
 

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

@@ -371,10 +371,10 @@ public Expression visitQualifiedName(QualifiedName node, AnalysisContext context
     // check for reserved words in the identifier
     for (String part : node.getParts()) {
       for (TypeEnvironment typeEnv = context.peek();
-           typeEnv != null;
-           typeEnv = typeEnv.getParent()) {
-        Optional<ExprType> exprType = typeEnv.getReservedSymbolTable().lookup(
-            new Symbol(Namespace.FIELD_NAME, part));
+          typeEnv != null;
+          typeEnv = typeEnv.getParent()) {
+        Optional<ExprType> exprType =
+            Optional.ofNullable(typeEnv.lookupAllFields(Namespace.HIDDEN_FIELD_NAME).get(part));
         if (exprType.isPresent()) {
           return visitMetadata(
               qualifierAnalyzer.unqualified(node),

core/src/main/java/org/opensearch/sql/analysis/TypeEnvironment.java

@@ -29,9 +29,6 @@ public class TypeEnvironment implements Environment<Symbol, ExprType> {
   private final TypeEnvironment parent;
   private final SymbolTable symbolTable;
 
-  @Getter
-  private final SymbolTable reservedSymbolTable;
-
   /**
    * Constructor with empty symbol tables.
    *
@@ -40,7 +37,6 @@ public class TypeEnvironment implements Environment<Symbol, ExprType> {
   public TypeEnvironment(TypeEnvironment parent) {
     this.parent = parent;
     this.symbolTable = new SymbolTable();
-    this.reservedSymbolTable = new SymbolTable();
   }
 
   /**
@@ -52,7 +48,6 @@ public TypeEnvironment(TypeEnvironment parent) {
   public TypeEnvironment(TypeEnvironment parent, SymbolTable symbolTable) {
     this.parent = parent;
     this.symbolTable = symbolTable;
-    this.reservedSymbolTable = new SymbolTable();
   }
 
   /**
@@ -133,8 +128,4 @@ public void clearAllFields() {
     lookupAllFields(FIELD_NAME).keySet().forEach(
         v -> remove(new Symbol(Namespace.FIELD_NAME, v)));
   }
-
-  public void addReservedWord(Symbol symbol, ExprType type) {
-    reservedSymbolTable.store(symbol, type);
-  }
 }

core/src/main/java/org/opensearch/sql/analysis/symbol/Namespace.java

@@ -13,6 +13,7 @@ public enum Namespace {
 
   INDEX_NAME("Index"),
   FIELD_NAME("Field"),
+  HIDDEN_FIELD_NAME("HiddenField"),
   FUNCTION_NAME("Function");
 
   private final String name;

core/src/main/java/org/opensearch/sql/monitor/AlwaysHealthyMonitor.java

@@ -3,19 +3,13 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.monitor;
 
-/**
- * Always healthy resource monitor.
- */
+/** Always healthy resource monitor. */
 public class AlwaysHealthyMonitor extends ResourceMonitor {
-  public static final ResourceMonitor ALWAYS_HEALTHY_MONITOR =
-      new AlwaysHealthyMonitor();
+  public static final ResourceMonitor ALWAYS_HEALTHY_MONITOR = new AlwaysHealthyMonitor();
 
-  /**
-   * always healthy.
-   */
+  /** always healthy. */
   @Override
   public boolean isHealthy() {
     return true;

core/src/main/java/org/opensearch/sql/monitor/ResourceMonitor.java

@@ -3,12 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.monitor;
 
 /**
- * The abstract interface of ResourceMonitor.
- * When an fault is detected, the circuit breaker is open.
+ * The abstract interface of ResourceMonitor. When an fault is detected, the circuit breaker is
+ * open.
  */
 public abstract class ResourceMonitor {
   /**

core/src/main/java/org/opensearch/sql/planner/DefaultImplementor.java

@@ -45,13 +45,12 @@
 
 /**
  * Default implementor for implementing logical to physical translation. "Default" here means all
- * logical operator will be translated to correspondent physical operator to pipeline operations
- * in post-processing style in memory.
- * Different storage can override methods here to optimize default pipelining operator, for example
- * a storage has the flexibility to override visitFilter and visitRelation to push down filtering
- * operation and return a single physical index scan operator.
+ * logical operator will be translated to correspondent physical operator to pipeline operations in
+ * post-processing style in memory. Different storage can override methods here to optimize default
+ * pipelining operator, for example a storage has the flexibility to override visitFilter and
+ * visitRelation to push down filtering operation and return a single physical index scan operator.
  *
- * @param <C>   context type
+ * @param <C> context type
  */
 public class DefaultImplementor<C> extends LogicalPlanNodeVisitor<PhysicalPlan, C> {
 
@@ -62,8 +61,7 @@ public PhysicalPlan visitRareTopN(LogicalRareTopN node, C context) {
         node.getCommandType(),
         node.getNoOfResults(),
         node.getFieldList(),
-        node.getGroupByList()
-    );
+        node.getGroupByList());
   }
 
   @Override
@@ -78,16 +76,14 @@ public PhysicalPlan visitDedupe(LogicalDedupe node, C context) {
 
   @Override
   public PhysicalPlan visitProject(LogicalProject node, C context) {
-    return new ProjectOperator(visitChild(node, context), node.getProjectList(),
-        node.getNamedParseExpressions());
+    return new ProjectOperator(
+        visitChild(node, context), node.getProjectList(), node.getNamedParseExpressions());
   }
 
   @Override
   public PhysicalPlan visitWindow(LogicalWindow node, C context) {
     return new WindowOperator(
-        visitChild(node, context),
-        node.getWindowFunction(),
-        node.getWindowDefinition());
+        visitChild(node, context), node.getWindowFunction(), node.getWindowDefinition());
   }
 
   @Override
@@ -148,8 +144,9 @@ public PhysicalPlan visitTableWriteBuilder(TableWriteBuilder plan, C context) {
 
   @Override
   public PhysicalPlan visitRelation(LogicalRelation node, C context) {
-    throw new UnsupportedOperationException("Storage engine is responsible for "
-        + "implementing and optimizing logical plan with relation involved");
+    throw new UnsupportedOperationException(
+        "Storage engine is responsible for "
+            + "implementing and optimizing logical plan with relation involved");
   }
 
   @Override

core/src/main/java/org/opensearch/sql/planner/PlanContext.java

@@ -9,13 +9,10 @@
 import lombok.Getter;
 import org.opensearch.sql.storage.split.Split;
 
-/**
- * Plan context hold planning related information.
- */
+/** Plan context hold planning related information. */
 public class PlanContext {
 
-  @Getter
-  private final Optional<Split> split;
+  @Getter private final Optional<Split> split;
 
   public PlanContext(Split split) {
     this.split = Optional.of(split);

core/src/main/java/org/opensearch/sql/planner/PlanNode.java

@@ -3,14 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.planner;
 
 import java.util.List;
 
-/**
- * The definition of Plan Node.
- */
+/** The definition of Plan Node. */
 public interface PlanNode<T extends PlanNode> {
 
   /**

core/src/main/java/org/opensearch/sql/planner/Planner.java

@@ -3,10 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.planner;
 
-
 import java.util.List;
 import lombok.RequiredArgsConstructor;
 import org.opensearch.sql.planner.logical.LogicalPlan;
@@ -16,17 +14,15 @@
 import org.opensearch.sql.planner.physical.PhysicalPlan;
 import org.opensearch.sql.storage.Table;
 
-/**
- * Planner that plans and chooses the optimal physical plan.
- */
+/** Planner that plans and chooses the optimal physical plan. */
 @RequiredArgsConstructor
 public class Planner {
 
   private final LogicalPlanOptimizer logicalOptimizer;
 
   /**
-   * Generate optimal physical plan for logical plan. If no table involved,
-   * translate logical plan to physical by default implementor.
+   * Generate optimal physical plan for logical plan. If no table involved, translate logical plan
+   * to physical by default implementor.<br>
    * TODO: for now just delegate entire logical plan to storage engine.
    *
    * @param plan logical plan
@@ -37,28 +33,28 @@ public PhysicalPlan plan(LogicalPlan plan) {
     if (table == null) {
       return plan.accept(new DefaultImplementor<>(), null);
     }
-    return table.implement(
-        table.optimize(optimize(plan)));
+    return table.implement(table.optimize(optimize(plan)));
   }
 
   private Table findTable(LogicalPlan plan) {
-    return plan.accept(new LogicalPlanNodeVisitor<Table, Object>() {
-
-      @Override
-      public Table visitNode(LogicalPlan node, Object context) {
-        List<LogicalPlan> children = node.getChild();
-        if (children.isEmpty()) {
-          return null;
-        }
-        return children.get(0).accept(this, context);
-      }
-
-      @Override
-      public Table visitRelation(LogicalRelation node, Object context) {
-        return node.getTable();
-      }
-
-    }, null);
+    return plan.accept(
+        new LogicalPlanNodeVisitor<Table, Object>() {
+
+          @Override
+          public Table visitNode(LogicalPlan node, Object context) {
+            List<LogicalPlan> children = node.getChild();
+            if (children.isEmpty()) {
+              return null;
+            }
+            return children.get(0).accept(this, context);
+          }
+
+          @Override
+          public Table visitRelation(LogicalRelation node, Object context) {
+            return node.getTable();
+          }
+        },
+        null);
   }
 
   private LogicalPlan optimize(LogicalPlan plan) {

core/src/main/java/org/opensearch/sql/planner/SerializablePlan.java

@@ -10,36 +10,37 @@
 /**
  * All subtypes of PhysicalPlan which needs to be serialized (in cursor, for pagination feature)
  * should follow one of the following options.
+ *
  * <ul>
  *   <li>Both:
- *     <ul>
- *       <li>Override both methods from {@link Externalizable}.</li>
- *       <li>Define a public no-arg constructor.</li>
- *     </ul>
- *   </li>
- *   <li>
- *     Overwrite {@link #getPlanForSerialization} to return
- *     another instance of {@link SerializablePlan}.
- *   </li>
+ *       <ul>
+ *         <li>Override both methods from {@link Externalizable}.
+ *         <li>Define a public no-arg constructor.
+ *       </ul>
+ *   <li>Overwrite {@link #getPlanForSerialization} to return another instance of {@link
+ *       SerializablePlan}.
  * </ul>
  */
 public interface SerializablePlan extends Externalizable {
 
   /**
-   * Override to return child or delegated plan, so parent plan should skip this one
-   * for serialization, but it should try to serialize grandchild plan.
-   * Imagine plan structure like this
+   * Override to return child or delegated plan, so parent plan should skip this one for
+   * serialization, but it should try to serialize grandchild plan. Imagine plan structure like this
+   *
    * <pre>
    *    A         -> this
    *    `- B      -> child
    *      `- C    -> this
    * </pre>
-   * In that case only plans A and C should be attempted to serialize.
-   * It is needed to skip a `ResourceMonitorPlan` instance only, actually.
    *
-   *  <pre>{@code
-   *    * A.writeObject(B.getPlanForSerialization());
-   *  }</pre>
+   * In that case only plans A and C should be attempted to serialize. It is needed to skip a
+   * `ResourceMonitorPlan` instance only, actually.
+   *
+   * <pre>{@code
+   * * A.writeObject(B.getPlanForSerialization());
+   *
+   * }</pre>
+   *
    * @return Next plan for serialization.
    */
   default SerializablePlan getPlanForSerialization() {

core/src/main/java/org/opensearch/sql/planner/logical/LogicalAD.java

@@ -18,6 +18,7 @@ public class LogicalAD extends LogicalPlan {
 
   /**
    * Constructor of LogicalAD.
+   *
    * @param child child logical plan
    * @param arguments arguments of the algorithm
    */