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

Support Struct Data Query In SQL/PPL #1018

Merged
merged 11 commits into from
Jan 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this / do we need to handle project.year is another object field? ex. project.year.month

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. add more cases in doc and UT.
We don't expect object filed name include "." which is not supported now.

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());
}
}
Loading