Simplifying flattening logic. Removed nested function classes making …

…nested a generic function. Updating nested to use function name as column name differing from legacy implementation. Adding IT tests to cover additional non-nested type cases.

Signed-off-by: forestmvey <[email protected]>

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

@@ -7,12 +7,15 @@
 
 import static org.opensearch.sql.data.type.ExprCoreType.STRING;
 
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import lombok.RequiredArgsConstructor;
 import org.opensearch.sql.ast.AbstractNodeVisitor;
 import org.opensearch.sql.ast.expression.Alias;
 import org.opensearch.sql.ast.expression.Function;
+import org.opensearch.sql.ast.expression.QualifiedName;
 import org.opensearch.sql.ast.expression.UnresolvedExpression;
 import org.opensearch.sql.expression.NamedExpression;
 import org.opensearch.sql.expression.ReferenceExpression;
@@ -45,6 +48,7 @@ public LogicalPlan visitFunction(Function node, AnalysisContext context) {
     if (node.getFuncName().equalsIgnoreCase(BuiltinFunctionName.NESTED.name())) {
 
       List<UnresolvedExpression> expressions = node.getFuncArgs();
+      validateArgs(expressions);
       ReferenceExpression nestedField =
           (ReferenceExpression)expressionAnalyzer.analyze(expressions.get(0), context);
       Map<String, ReferenceExpression> args;
@@ -59,11 +63,48 @@ public LogicalPlan visitFunction(Function node, AnalysisContext context) {
             "path", generatePath(nestedField.toString())
         );
       }
-      return new LogicalNested(child, List.of(args), namedExpressions);
+      if (child instanceof LogicalNested) {
+        ((LogicalNested)child).addFields(args);
+        return child;
+      } else {
+        return new LogicalNested(child, new ArrayList<>(Arrays.asList(args)), namedExpressions);
+      }
     }
     return null;
   }
 
+  /**
+   * Validate each parameter used in nested function in SELECT clause. Any supplied parameter
+   * for a nested function in a SELECT statement must be a valid qualified name, and the field
+   * parameter must be nested at least one level.
+   * @param args : Arguments in nested function.
+   */
+  private void validateArgs(List<UnresolvedExpression> args) {
+    if (args.size() < 1 || args.size() > 2) {
+      throw new IllegalArgumentException(
+          "on nested object only allowed 2 parameters (field,path) or 1 parameter (field)"
+      );
+    }
+
+    for (int i = 0; i < args.size(); i++) {
+      if (!(args.get(i) instanceof QualifiedName)) {
+        throw new IllegalArgumentException(
+            String.format("Illegal nested field name: %s", args.get(i).toString())
+        );
+      }
+      if (i == 0 && ((QualifiedName)args.get(i)).getParts().size() < 2) {
+        throw new IllegalArgumentException(
+            String.format("Illegal nested field name: %s", args.get(i).toString())
+        );
+      }
+    }
+  }
+
+  /**
+   * Generate nested path dynamically. Assumes at least one level of nesting in supplied string.
+   * @param field : Nested field to generate path of.
+   * @return : Path of field derived from last level of nesting.
+   */
   private ReferenceExpression generatePath(String field) {
     return new ReferenceExpression(field.substring(0, field.lastIndexOf(".")), STRING);
   }

core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java

@@ -8,6 +8,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 import lombok.experimental.UtilityClass;
+import org.apache.commons.lang3.tuple.Pair;
 import org.opensearch.sql.data.model.ExprValue;
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
@@ -41,7 +42,7 @@ public void register(BuiltinFunctionRepository repository) {
     repository.register(wildcard_query(BuiltinFunctionName.WILDCARD_QUERY));
     repository.register(wildcard_query(BuiltinFunctionName.WILDCARDQUERY));
     // Functions supported in SELECT clause
-    repository.register(nested(BuiltinFunctionName.NESTED));
+    repository.register(nested());
   }
 
   private static FunctionResolver match_bool_prefix() {
@@ -88,11 +89,36 @@ private static FunctionResolver wildcard_query(BuiltinFunctionName wildcardQuery
     return new RelevanceFunctionResolver(funcName);
   }
 
-  private static FunctionResolver nested(BuiltinFunctionName nested) {
-    FunctionName funcName = nested.getName();
-    return new NestedFunctionResolver(funcName);
+  private static FunctionResolver nested() {
+    return new FunctionResolver() {
+      @Override
+      public Pair<FunctionSignature, FunctionBuilder> resolve(
+          FunctionSignature unresolvedSignature) {
+        return Pair.of(unresolvedSignature,
+            (functionProperties, arguments) ->
+            new FunctionExpression(BuiltinFunctionName.NESTED.getName(), arguments) {
+              @Override
+              public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
+                return valueEnv.resolve(getArguments().get(0));
+              }
+
+              @Override
+              public ExprType type() {
+                return getArguments().get(0).type();
+              }
+            });
+      }
+
+      @Override
+      public FunctionName getFunctionName() {
+        return BuiltinFunctionName.NESTED.getName();
+      }
+    };
   }
 
+
+
+
   public static class OpenSearchFunction extends FunctionExpression {
     private final FunctionName functionName;
     private final List<Expression> arguments;

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

@@ -27,13 +27,13 @@
 import org.opensearch.sql.planner.physical.EvalOperator;
 import org.opensearch.sql.planner.physical.FilterOperator;
 import org.opensearch.sql.planner.physical.LimitOperator;
+import org.opensearch.sql.planner.physical.NestedOperator;
 import org.opensearch.sql.planner.physical.PhysicalPlan;
 import org.opensearch.sql.planner.physical.ProjectOperator;
 import org.opensearch.sql.planner.physical.RareTopNOperator;
 import org.opensearch.sql.planner.physical.RemoveOperator;
 import org.opensearch.sql.planner.physical.RenameOperator;
 import org.opensearch.sql.planner.physical.SortOperator;
-import org.opensearch.sql.planner.physical.UnnestOperator;
 import org.opensearch.sql.planner.physical.ValuesOperator;
 import org.opensearch.sql.planner.physical.WindowOperator;
 import org.opensearch.sql.storage.read.TableScanBuilder;
@@ -98,7 +98,7 @@ public PhysicalPlan visitEval(LogicalEval node, C context) {
 
   @Override
   public PhysicalPlan visitUnnest(LogicalNested node, C context) {
-    return new UnnestOperator(visitChild(node, context), node.getFields());
+    return new NestedOperator(visitChild(node, context), node.getFields());
   }
 
   @Override

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

@@ -18,7 +18,7 @@
 @Getter
 @ToString
 public class LogicalNested extends LogicalPlan {
-  private final List<Map<String, ReferenceExpression>> fields;
+  private List<Map<String, ReferenceExpression>> fields;
   private final List<NamedExpression> projectList;
 
   /**
@@ -35,6 +35,10 @@ public LogicalNested(
     this.projectList = projectList;
   }
 
+  public void addFields(Map<String, ReferenceExpression> fields) {
+    this.fields.add(fields);
+  }
+
   @Override
   public <R, C> R accept(LogicalPlanNodeVisitor<R, C> visitor, C context) {
     return visitor.visitUnnest(this, context);

core/src/main/java/org/opensearch/sql/planner/optimizer/LogicalPlanOptimizer.java

@@ -14,7 +14,6 @@
 import java.util.stream.Collectors;
 import org.opensearch.sql.planner.logical.LogicalPlan;
 import org.opensearch.sql.planner.optimizer.rule.MergeFilterAndFilter;
-import org.opensearch.sql.planner.optimizer.rule.MergeNestedAndNested;
 import org.opensearch.sql.planner.optimizer.rule.PushFilterUnderSort;
 import org.opensearch.sql.planner.optimizer.rule.read.CreateTableScanBuilder;
 import org.opensearch.sql.planner.optimizer.rule.read.TableScanPushDown;
@@ -52,7 +51,6 @@ public static LogicalPlanOptimizer create() {
          * Phase 2: Transformations that rely on data source push down capability
          */
         new CreateTableScanBuilder(),
-        new MergeNestedAndNested(),
         TableScanPushDown.PUSH_DOWN_FILTER,
         TableScanPushDown.PUSH_DOWN_AGGREGATION,
         TableScanPushDown.PUSH_DOWN_SORT,