Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Merge pull request #1018 from penghuo/object-query-pr
Browse files Browse the repository at this point in the history
Support Struct Data Query In SQL/PPL
  • Loading branch information
penghuo committed Jan 30, 2021
2 parents 15a31d2 + 8004a80 commit 1dbf244
Show file tree
Hide file tree
Showing 24 changed files with 569 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ private Expression visitIdentifier(String ident, AnalysisContext context) {
return ref;
}

// Array type is not supporte yet.
private boolean isTypeNotSupported(ExprType type) {
return "struct".equalsIgnoreCase(type.typeName())
|| "array".equalsIgnoreCase(type.typeName());
return "array".equalsIgnoreCase(type.typeName());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,34 @@ public String unqualified(QualifiedName fullName) {
private boolean isQualifierIndexOrAlias(QualifiedName fullName) {
Optional<String> qualifier = fullName.first();
if (qualifier.isPresent()) {
if (isFieldName(qualifier.get())) {
return false;
}
resolveQualifierSymbol(fullName, qualifier.get());
return true;
}
return false;
}

private boolean isFieldName(String qualifier) {
try {
// Resolve the qualifier in Namespace.FIELD_NAME
context.peek().resolve(new Symbol(Namespace.FIELD_NAME, qualifier));
return true;
} catch (SemanticCheckException e2) {
return false;
}
}

private void resolveQualifierSymbol(QualifiedName fullName, String qualifier) {
try {
context.peek().resolve(new Symbol(Namespace.INDEX_NAME, qualifier));
} catch (SemanticCheckException e) {
// Throw syntax check intentionally to indicate fall back to old engine.
// Need change to semantic check exception in future.
throw new SyntaxCheckException(String.format(
"The qualifier [%s] of qualified name [%s] must be an index name or its alias",
qualifier, fullName));
"The qualifier [%s] of qualified name [%s] must be an field name, index name or its "
+ "alias", qualifier, fullName));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public UnresolvedPlan values(List<Literal>... values) {
return new Values(Arrays.asList(values));
}

public static UnresolvedExpression qualifiedName(String... parts) {
public static QualifiedName qualifiedName(String... parts) {
return new QualifiedName(Arrays.asList(parts));
}

Expand Down Expand Up @@ -178,7 +178,7 @@ public static Literal nullLiteral() {
}

public static Map map(String origin, String target) {
return new Map(new Field(origin), new Field(target));
return new Map(field(origin), field(target));
}

public static Map map(UnresolvedExpression origin, UnresolvedExpression target) {
Expand Down Expand Up @@ -281,27 +281,27 @@ public AllFields allFields() {
}

public Field field(UnresolvedExpression field) {
return new Field((QualifiedName) field);
}

public Field field(String field) {
return new Field(field);
}

public Field field(UnresolvedExpression field, Argument... fieldArgs) {
return new Field(field, Arrays.asList(fieldArgs));
return field(field, Arrays.asList(fieldArgs));
}

public Field field(String field) {
return field(qualifiedName(field));
}

public Field field(String field, Argument... fieldArgs) {
return new Field(field, Arrays.asList(fieldArgs));
return field(field, Arrays.asList(fieldArgs));
}

public Field field(UnresolvedExpression field, List<Argument> fieldArgs) {
return new Field(field, fieldArgs);
}

public Field field(String field, List<Argument> fieldArgs) {
return new Field(field, fieldArgs);
return field(qualifiedName(field), fieldArgs);
}

public Alias alias(String name, UnresolvedExpression expr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,24 @@
@Getter
@ToString
@EqualsAndHashCode(callSuper = false)
@AllArgsConstructor
public class Field extends UnresolvedExpression {
private UnresolvedExpression field;
private List<Argument> fieldArgs = Collections.emptyList();

public Field(QualifiedName field) {
this.field = field;
}
private final UnresolvedExpression field;

public Field(String field) {
this.field = new QualifiedName(field);
private final List<Argument> fieldArgs;

/**
* Constructor of Field.
*/
public Field(UnresolvedExpression field) {
this(field, Collections.emptyList());
}

public Field(String field, List<Argument> fieldArgs) {
this.field = new QualifiedName(field);
/**
* Constructor of Field.
*/
public Field(UnresolvedExpression field, List<Argument> fieldArgs) {
this.field = field;
this.fieldArgs = fieldArgs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,19 @@ public String toString() {

@Override
public BindingTuple bindingTuples() {
return new LazyBindingTuple(
bindingName -> valueMap.getOrDefault(bindingName, ExprMissingValue.of()));
return new LazyBindingTuple(() -> this);
}

@Override
public Map<String, ExprValue> tupleValue() {
return valueMap;
}

@Override
public ExprValue keyValue(String key) {
return valueMap.getOrDefault(key, ExprMissingValue.of());
}

/**
* Override the equals method.
* @return true for equal, otherwise false.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,4 +196,12 @@ default List<ExprValue> collectionValue() {
throw new ExpressionEvaluationException(
"invalid to get collectionValue from value of type " + type());
}

/**
* Get the value specified by key from {@link ExprTupleValue}.
* This method only be implemented in {@link ExprTupleValue}.
*/
default ExprValue keyValue(String key) {
return ExprMissingValue.of();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@

package com.amazon.opendistroforelasticsearch.sql.expression;

import static com.amazon.opendistroforelasticsearch.sql.utils.ExpressionUtils.PATH_SEP;

import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue;
import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue;
import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType;
import com.amazon.opendistroforelasticsearch.sql.expression.env.Environment;
import java.util.Arrays;
import java.util.List;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
Expand All @@ -28,8 +33,23 @@ public class ReferenceExpression implements Expression {
@Getter
private final String attr;

@Getter
private final List<String> paths;

private final ExprType type;

/**
* Constructor of ReferenceExpression.
* @param ref the field name. e.g. addr.state/addr.
* @param type type.
*/
public ReferenceExpression(String ref, ExprType type) {
this.attr = ref;
// Todo. the define of paths need to be redefined after adding multiple index/variable support.
this.paths = Arrays.asList(ref.split("\\."));
this.type = type;
}

@Override
public ExprValue valueOf(Environment<Expression, ExprValue> env) {
return env.resolve(this);
Expand All @@ -49,4 +69,51 @@ public <T, C> T accept(ExpressionNodeVisitor<T, C> visitor, C context) {
public String toString() {
return attr;
}

/**
* Resolve the ExprValue from {@link ExprTupleValue} using paths.
* Considering the following sample data.
* {
* "name": "bob smith"
* "project.year": 1990,
* "project": {
* "year": "2020"
* }
* "address": {
* "state": "WA",
* "city": "seattle",
* "project.year": 1990
* }
* "address.local": {
* "state": "WA",
* }
* }
* The paths could be
* 1. top level, e.g. "name", which will be resolved as "bob smith"
* 2. multiple paths, e.g. "name.address.state", which will be resolved as "WA"
* 3. special case, the "." is the path separator, but it is possible that the path include
* ".", for handling this use case, we define the resolve rule as bellow, e.g. "project.year" is
* resolved as 1990 instead of 2020. Note. This logic only applied top level none object field.
* e.g. "address.local.state" been resolved to Missing. but "address.project.year" could been
* resolved as 1990.
*
* <p>Resolve Rule
* 1. Resolve the full name by combine the paths("x"."y"."z") as whole ("x.y.z").
* 2. Resolve the path recursively through ExprValue.
*
* @param value {@link ExprTupleValue}.
* @return {@link ExprTupleValue}.
*/
public ExprValue resolve(ExprTupleValue value) {
return resolve(value, paths);
}

private ExprValue resolve(ExprValue value, List<String> paths) {
final ExprValue wholePathValue = value.keyValue(String.join(PATH_SEP, paths));
if (!wholePathValue.isMissing() || paths.size() == 1) {
return wholePathValue;
} else {
return resolve(value.keyValue(paths.get(0)), paths.subList(1, paths.size()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@

package com.amazon.opendistroforelasticsearch.sql.storage.bindingtuple;

import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue;
import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue;
import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression;
import java.util.function.Function;
import java.util.function.Supplier;
import lombok.RequiredArgsConstructor;

/**
* Lazy Implementation of {@link BindingTuple}.
*/
@RequiredArgsConstructor
public class LazyBindingTuple extends BindingTuple {
private final Function<String, ExprValue> lazyBinding;
private final Supplier<ExprTupleValue> lazyBinding;

@Override
public ExprValue resolve(ReferenceExpression ref) {
return lazyBinding.apply(ref.getAttr());
return ref.resolve(lazyBinding.get());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
@UtilityClass
public class ExpressionUtils {

public static String PATH_SEP = ".";

/**
* Format the list of {@link Expression}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,20 @@ public void qualified_name_with_qualifier() {
qualifiedName("index_alias", "integer_value")
);

analysisContext.peek().define(new Symbol(Namespace.FIELD_NAME, "nested_field"), STRUCT);
analysisContext.peek().define(new Symbol(Namespace.FIELD_NAME, "object_field"), STRUCT);
analysisContext.peek().define(new Symbol(Namespace.FIELD_NAME, "object_field.integer_value"),
INTEGER);
assertAnalyzeEqual(
DSL.ref("object_field.integer_value", INTEGER),
qualifiedName("object_field", "integer_value")
);

SyntaxCheckException exception =
assertThrows(SyntaxCheckException.class,
() -> analyze(qualifiedName("nested_field", "integer_value")));
assertEquals(
"The qualifier [nested_field] of qualified name [nested_field.integer_value] "
+ "must be an index name or its alias",
+ "must be an field name, index name or its alias",
exception.getMessage()
);
analysisContext.pop();
Expand Down Expand Up @@ -237,17 +244,6 @@ public void case_clause() {
AstDSL.stringLiteral("test"))));
}

@Test
public void skip_struct_data_type() {
SyntaxCheckException exception =
assertThrows(SyntaxCheckException.class,
() -> analyze(qualifiedName("struct_value")));
assertEquals(
"Identifier [struct_value] of type [STRUCT] is not supported yet",
exception.getMessage()
);
}

@Test
public void skip_array_data_type() {
SyntaxCheckException exception =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ void should_return_original_name_if_no_qualifier() {

@Test
void should_report_error_if_qualifier_is_not_index() {
runInScope(new Symbol(Namespace.FIELD_NAME, "a"), ARRAY, () -> {
runInScope(new Symbol(Namespace.FIELD_NAME, "aIndex"), ARRAY, () -> {
SyntaxCheckException error = assertThrows(SyntaxCheckException.class,
() -> qualifierAnalyzer.unqualified("a", "integer_value"));
assertEquals("The qualifier [a] of qualified name [a.integer_value] "
+ "must be an index name or its alias", error.getMessage());
+ "must be an field name, index name or its alias", error.getMessage());
});
}

Expand All @@ -65,7 +65,8 @@ void should_report_error_if_qualifier_is_not_exist() {
SyntaxCheckException error = assertThrows(SyntaxCheckException.class,
() -> qualifierAnalyzer.unqualified("a", "integer_value"));
assertEquals(
"The qualifier [a] of qualified name [a.integer_value] must be an index name or its alias",
"The qualifier [a] of qualified name [a.integer_value] must be an field name, index name "
+ "or its alias",
error.getMessage());
}

Expand All @@ -76,6 +77,13 @@ void should_return_qualified_name_if_qualifier_is_index() {
);
}

@Test
void should_return_qualified_name_if_qualifier_is_field() {
runInScope(new Symbol(Namespace.FIELD_NAME, "a"), STRUCT, () ->
assertEquals("a.integer_value", qualifierAnalyzer.unqualified("a", "integer_value"))
);
}

@Test
void should_report_error_if_more_parts_in_qualified_name() {
runInScope(new Symbol(Namespace.INDEX_NAME, "a"), STRUCT, () ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package com.amazon.opendistroforelasticsearch.sql.data.model;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException;
import org.junit.jupiter.api.Assertions;
Expand All @@ -31,4 +32,9 @@ public void getShortValueFromIncompatibleExprValue() {
.assertThrows(ExpressionEvaluationException.class, () -> booleanValue.shortValue());
assertEquals("invalid to get shortValue from value of type BOOLEAN", exception.getMessage());
}

@Test
public void key_value() {
assertTrue(new ExprIntegerValue(1).keyValue("path").isMissing());
}
}

0 comments on commit 1dbf244

Please sign in to comment.