Skip to content

Commit

Permalink
Simplifying flattening logic. Removed nested function classes making …
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
forestmvey committed Apr 5, 2023
1 parent 404ccc3 commit e8114bd
Show file tree
Hide file tree
Showing 32 changed files with 309 additions and 575 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -59,11 +63,31 @@ 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;
}

private void validateArgs(List<UnresolvedExpression> args) {
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())
);
}
}
}

private ReferenceExpression generatePath(String field) {
return new ReferenceExpression(field.substring(0, field.lastIndexOf(".")), STRING);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down

This file was deleted.

Loading

0 comments on commit e8114bd

Please sign in to comment.