diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java index 961b6bdd16..b8e418a43a 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java @@ -194,7 +194,7 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) { for (UnresolvedExpression expr : node.getAggExprList()) { NamedExpression aggExpr = namedExpressionAnalyzer.analyze(expr, context); aggregatorBuilder - .add(new NamedAggregator(aggExpr.getName(), (Aggregator) aggExpr.getDelegated())); + .add(new NamedAggregator(aggExpr.getNameOrAlias(), (Aggregator) aggExpr.getDelegated())); } ImmutableList aggregators = aggregatorBuilder.build(); @@ -210,7 +210,7 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) { aggregators.forEach(aggregator -> newEnv.define(new Symbol(Namespace.FIELD_NAME, aggregator.getName()), aggregator.type())); groupBys.forEach(group -> newEnv.define(new Symbol(Namespace.FIELD_NAME, - group.getName()), group.type())); + group.getNameOrAlias()), group.type())); return new LogicalAggregation(child, aggregators, groupBys); } @@ -291,7 +291,7 @@ public LogicalPlan visitProject(Project node, AnalysisContext context) { context.push(); TypeEnvironment newEnv = context.peek(); namedExpressions.forEach(expr -> newEnv.define(new Symbol(Namespace.FIELD_NAME, - expr.getName()), expr.type())); + expr.getNameOrAlias()), expr.type())); return new LogicalProject(child, namedExpressions); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionReferenceOptimizer.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionReferenceOptimizer.java index b999f50f15..b98c7be53e 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionReferenceOptimizer.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionReferenceOptimizer.java @@ -136,7 +136,8 @@ public Void visitAggregation(LogicalAggregation plan, Void context) { new ReferenceExpression(namedAggregator.getName(), namedAggregator.type()))); // Create the mapping for all the group by. plan.getGroupByList().forEach(groupBy -> expressionMap - .put(groupBy.getDelegated(), new ReferenceExpression(groupBy.getName(), groupBy.type()))); + .put(groupBy.getDelegated(), + new ReferenceExpression(groupBy.getNameOrAlias(), groupBy.type()))); return null; } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java index dbeb5453b5..2aba72f407 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java @@ -409,6 +409,10 @@ public FunctionExpression strcmp(Expression... expressions) { return function(BuiltinFunctionName.STRCMP, expressions); } + public FunctionExpression right(Expression... expressions) { + return function(BuiltinFunctionName.RIGHT, expressions); + } + public FunctionExpression and(Expression... expressions) { return function(BuiltinFunctionName.AND, expressions); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionNodeVisitor.java index b3aec09ee2..6b593d5acc 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionNodeVisitor.java @@ -62,7 +62,7 @@ public T visitLiteral(LiteralExpression node, C context) { } public T visitNamed(NamedExpression node, C context) { - return visitNode(node, context); + return node.getDelegated().accept(this, context); } public T visitReference(ReferenceExpression node, C context) { diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpression.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpression.java index 17fd1225eb..8153239ebd 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpression.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpression.java @@ -24,7 +24,6 @@ import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; -import lombok.ToString; /** * Named expression that represents expression with name. @@ -33,6 +32,7 @@ */ @AllArgsConstructor @EqualsAndHashCode +@Getter @RequiredArgsConstructor public class NamedExpression implements Expression { @@ -44,13 +44,11 @@ public class NamedExpression implements Expression { /** * Expression that being named. */ - @Getter private final Expression delegated; /** * Optional alias. */ - @Getter private String alias; @Override @@ -67,7 +65,7 @@ public ExprType type() { * Get expression name using name or its alias (if it's present). * @return expression name */ - public String getName() { + public String getNameOrAlias() { return Strings.isNullOrEmpty(alias) ? name : alias; } @@ -78,7 +76,7 @@ public T accept(ExpressionNodeVisitor visitor, C context) { @Override public String toString() { - return getName(); + return getNameOrAlias(); } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/AggregatorFunction.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/AggregatorFunction.java index a09a2c6833..e467c38585 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/AggregatorFunction.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/AggregatorFunction.java @@ -28,6 +28,8 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIME; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIMESTAMP; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionRepository; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionBuilder; @@ -35,7 +37,13 @@ import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionResolver; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionSignature; import com.google.common.collect.ImmutableMap; + +import java.util.ArrayList; import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; + import lombok.experimental.UtilityClass; /** @@ -73,27 +81,11 @@ private static FunctionResolver avg() { private static FunctionResolver count() { FunctionName functionName = BuiltinFunctionName.COUNT.getName(); - return new FunctionResolver( - functionName, - new ImmutableMap.Builder() - .put(new FunctionSignature(functionName, Collections.singletonList(INTEGER)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(LONG)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(FLOAT)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(STRING)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(STRUCT)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(ARRAY)), - arguments -> new CountAggregator(arguments, INTEGER)) - .put(new FunctionSignature(functionName, Collections.singletonList(BOOLEAN)), - arguments -> new CountAggregator(arguments, INTEGER)) - .build() - ); + FunctionResolver functionResolver = new FunctionResolver(functionName, + ExprCoreType.coreTypes().stream().collect(Collectors.toMap( + type -> new FunctionSignature(functionName, Collections.singletonList(type)), + type -> arguments -> new CountAggregator(arguments, INTEGER)))); + return functionResolver; } private static FunctionResolver sum() { diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java index 8c00c120d0..3c009ea067 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java @@ -131,6 +131,7 @@ public enum BuiltinFunctionName { CONCAT_WS(FunctionName.of("concat_ws")), LENGTH(FunctionName.of("length")), STRCMP(FunctionName.of("strcmp")), + RIGHT(FunctionName.of("right")), /** * NULL Test. diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunction.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunction.java index 4b2de00f97..e1a24f29bc 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunction.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunction.java @@ -60,6 +60,7 @@ public void register(BuiltinFunctionRepository repository) { repository.register(concat_ws()); repository.register(length()); repository.register(strcmp()); + repository.register(right()); } /** @@ -194,6 +195,16 @@ private FunctionResolver strcmp() { INTEGER, STRING, STRING)); } + /** + * Returns the rightmost len characters from the string str, or NULL if any argument is NULL. + * Supports following signatures: + * (STRING, INTEGER) -> STRING + */ + private FunctionResolver right() { + return define(BuiltinFunctionName.RIGHT.getName(), + impl(nullMissingHandling(TextFunction::exprRight), STRING, STRING, INTEGER)); + } + private static ExprValue exprSubstrStart(ExprValue exprValue, ExprValue start) { int startIdx = start.integerValue(); if (startIdx == 0) { @@ -225,5 +236,10 @@ private static ExprValue exprSubStr(String str, int start, int len) { } return new ExprStringValue(str.substring(start, start + len)); } + + private static ExprValue exprRight(ExprValue str, ExprValue len) { + return new ExprStringValue(str.stringValue().substring( + str.stringValue().length() - len.integerValue())); + } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/AggregationOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/AggregationOperator.java index 3bcf8301f5..ec38e3911f 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/AggregationOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/AggregationOperator.java @@ -172,7 +172,7 @@ public GroupKey(ExprValue value) { public LinkedHashMap groupKeyMap() { LinkedHashMap map = new LinkedHashMap<>(); for (int i = 0; i < groupByExprList.size(); i++) { - map.put(groupByExprList.get(i).getName(), groupByValueList.get(i)); + map.put(groupByExprList.get(i).getNameOrAlias(), groupByValueList.get(i)); } return map; } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperator.java index 32a6906298..c0fde16367 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperator.java @@ -62,7 +62,7 @@ public ExprValue next() { ImmutableMap.Builder mapBuilder = new Builder<>(); for (NamedExpression expr : projectList) { ExprValue exprValue = expr.valueOf(inputValue.bindingTuples()); - mapBuilder.put(expr.getName(), exprValue); + mapBuilder.put(expr.getNameOrAlias(), exprValue); } return ExprTupleValue.fromExprValueMap(mapBuilder.build()); } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/NamedExpressionAnalyzerTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/NamedExpressionAnalyzerTest.java index 4386ec7501..1ef80e8248 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/NamedExpressionAnalyzerTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/NamedExpressionAnalyzerTest.java @@ -41,6 +41,6 @@ void visit_named_seleteitem() { new NamedExpressionAnalyzer(expressionAnalyzer); NamedExpression analyze = analyzer.analyze(alias, analysisContext); - assertEquals("integer_value", analyze.getName()); + assertEquals("integer_value", analyze.getNameOrAlias()); } } \ No newline at end of file diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpressionTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpressionTest.java index c8330d2fba..dbfca07b76 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpressionTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/NamedExpressionTest.java @@ -30,7 +30,7 @@ void name_an_expression() { LiteralExpression delegated = DSL.literal(10); NamedExpression namedExpression = DSL.named("10", delegated); - assertEquals("10", namedExpression.getName()); + assertEquals("10", namedExpression.getNameOrAlias()); assertEquals(delegated.type(), namedExpression.type()); assertEquals(delegated.valueOf(valueEnv()), namedExpression.valueOf(valueEnv())); } @@ -39,7 +39,7 @@ void name_an_expression() { void name_an_expression_with_alias() { LiteralExpression delegated = DSL.literal(10); NamedExpression namedExpression = DSL.named("10", delegated, "ten"); - assertEquals("ten", namedExpression.getName()); + assertEquals("ten", namedExpression.getNameOrAlias()); } @Test @@ -48,7 +48,7 @@ void name_an_named_expression() { Expression expression = DSL.named("10", delegated, "ten"); NamedExpression namedExpression = DSL.named(expression); - assertEquals("ten", namedExpression.getName()); + assertEquals("ten", namedExpression.getNameOrAlias()); } } \ No newline at end of file diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/CountAggregatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/CountAggregatorTest.java index 4f42bec8fa..1190cc01df 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/CountAggregatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/aggregation/CountAggregatorTest.java @@ -17,12 +17,16 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.ARRAY; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATETIME; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DOUBLE; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.FLOAT; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.LONG; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRUCT; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIMESTAMP; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -58,6 +62,24 @@ public void count_double_field_expression() { assertEquals(4, result.value()); } + @Test + public void count_date_field_expression() { + ExprValue result = aggregation(dsl.count(DSL.ref("date_value", DATE)), tuples); + assertEquals(4, result.value()); + } + + @Test + public void count_timestamp_field_expression() { + ExprValue result = aggregation(dsl.count(DSL.ref("timestamp_value", TIMESTAMP)), tuples); + assertEquals(4, result.value()); + } + + @Test + public void count_datetime_field_expression() { + ExprValue result = aggregation(dsl.count(DSL.ref("datetime_value", DATETIME)), tuples); + assertEquals(4, result.value()); + } + @Test public void count_arithmetic_expression() { ExprValue result = aggregation(dsl.count( diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunctionTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunctionTest.java index d66c89089f..5c1f1728c5 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunctionTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/text/TextFunctionTest.java @@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprIntegerValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprStringValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; @@ -296,6 +297,23 @@ void strcmp() { assertEquals(missingValue(), eval(dsl.strcmp(missingRef, nullRef))); } + @Test + void right() { + FunctionExpression expression = dsl.right( + DSL.literal(new ExprStringValue("foobarbar")), + DSL.literal(new ExprIntegerValue(4))); + assertEquals(STRING, expression.type()); + assertEquals("rbar", eval(expression).stringValue()); + + when(nullRef.type()).thenReturn(STRING); + when(missingRef.type()).thenReturn(INTEGER); + assertEquals(missingValue(), eval(dsl.right(nullRef, missingRef))); + assertEquals(nullValue(), eval(dsl.right(nullRef, DSL.literal(new ExprIntegerValue(1))))); + + when(nullRef.type()).thenReturn(INTEGER); + assertEquals(nullValue(), eval(dsl.right(DSL.literal(new ExprStringValue("value")), nullRef))); + } + void testConcatString(List strings) { String expected = null; if (strings.stream().noneMatch(Objects::isNull)) { diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperatorTest.java index 5baf541f62..873c0f2734 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/ProjectOperatorTest.java @@ -104,11 +104,11 @@ public void project_keep_missing_value() { public void project_schema() { PhysicalPlan project = project(inputPlan, DSL.named("response", DSL.ref("response", INTEGER)), - DSL.named("action", DSL.ref("action", STRING))); + DSL.named("action", DSL.ref("action", STRING), "act")); assertThat(project.schema().getColumns(), contains( new ExecutionEngine.Schema.Column("response", null, INTEGER), - new ExecutionEngine.Schema.Column("action", null, STRING) + new ExecutionEngine.Schema.Column("action", "act", STRING) )); } } diff --git a/docs/experiment/ppl/functions/string.rst b/docs/experiment/ppl/functions/string.rst index 0afdd90195..3f90395d38 100644 --- a/docs/experiment/ppl/functions/string.rst +++ b/docs/experiment/ppl/functions/string.rst @@ -150,6 +150,29 @@ Example:: +---------------------+---------------------+ +RIGHT +----- + +Description +>>>>>>>>>>> + +Usage: right(str, len) returns the rightmost len characters from the string str, or NULL if any argument is NULL. + +Argument type: STRING, INTEGER + +Return type: STRING + +Example:: + + od> source=people | eval `RIGHT('helloworld', 5)` = RIGHT('helloworld', 5), `RIGHT('HELLOWORLD', 0)` = RIGHT('HELLOWORLD', 0) | fields `RIGHT('helloworld', 5)`, `RIGHT('HELLOWORLD', 0)` + fetched rows / total rows = 1/1 + +--------------------------+--------------------------+ + | RIGHT('helloworld', 5) | RIGHT('HELLOWORLD', 0) | + |--------------------------+--------------------------| + | world | | + +--------------------------+--------------------------+ + + RTRIM ----- diff --git a/docs/experiment/ppl/index.rst b/docs/experiment/ppl/index.rst index c4f13bd599..2c1b19e7ff 100644 --- a/docs/experiment/ppl/index.rst +++ b/docs/experiment/ppl/index.rst @@ -77,3 +77,7 @@ The query start with search command and then flowing a set of command delimited - `Identifiers `_ - `Data Types `_ + +* **Limitations** + + - `Limitations `_ \ No newline at end of file diff --git a/docs/experiment/ppl/limitations/limitations.rst b/docs/experiment/ppl/limitations/limitations.rst new file mode 100644 index 0000000000..fe9e678c6e --- /dev/null +++ b/docs/experiment/ppl/limitations/limitations.rst @@ -0,0 +1,21 @@ + +=========== +Limitations +=========== + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 2 + + +Introduction +============ + +In this doc, the restrictions and limitations of PPL is covered as follows. + +Limitations on Fields +===================== + +We are not supporting use `alias field type `_ as identifier. It will throw exception ``can't resolve Symbol``. diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 0e3831673f..9233084d4d 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -1760,9 +1760,21 @@ RIGHT Description >>>>>>>>>>> -Specifications: +Usage: right(str, len) returns the rightmost len characters from the string str, or NULL if any argument is NULL. + +Argument type: STRING, INTEGER + +Return type: STRING -1. RIGHT(STRING T, INTEGER) -> T +Example:: + + od> SELECT RIGHT('helloworld', 5), RIGHT('HELLOWORLD', 0) + fetched rows / total rows = 1/1 + +--------------------------+--------------------------+ + | RIGHT('helloworld', 5) | RIGHT('HELLOWORLD', 0) | + |--------------------------+--------------------------| + | world | | + +--------------------------+--------------------------+ RTRIM diff --git a/docs/user/limitations/limitations.rst b/docs/user/limitations/limitations.rst index cab4eac590..cd39a1472a 100644 --- a/docs/user/limitations/limitations.rst +++ b/docs/user/limitations/limitations.rst @@ -20,6 +20,12 @@ Limitations on Identifiers Using Elasticsearch cluster name as catalog name to qualify an index name, such as ``my_cluster.my_index``, is not supported for now. +Limitations on Fields +===================== + +We are not supporting use `alias field type `_ as identifier. It will throw exception ``can't resolve Symbol``. + + Limitations on Aggregations =========================== diff --git a/docs/user/optimization/optimization.rst b/docs/user/optimization/optimization.rst index 9acbe1ff8c..c4aab0e3aa 100644 --- a/docs/user/optimization/optimization.rst +++ b/docs/user/optimization/optimization.rst @@ -44,7 +44,7 @@ The consecutive Filter operator will be merged as one Filter operator:: { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false)" + "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false)" }, "children": [] } @@ -71,7 +71,7 @@ The Filter operator should be push down under Sort operator:: { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false)" + "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false)" }, "children": [] } @@ -85,6 +85,31 @@ Elasticsearch Specific Optimization The Elasticsearch `Query DSL `_ and `Aggregation `_ also enabling the storage engine specific optimization. +Push Project Into Query DSL +--------------------------- +The Project list will push down to Query DSL to `filter the source `_:: + + sh$ curl -sS -H 'Content-Type: application/json' \ + ... -X POST localhost:9200/_opendistro/_sql/_explain \ + ... -d '{"query" : "SELECT age FROM accounts"}' + { + "root": { + "name": "ProjectOperator", + "description": { + "fields": "[age]" + }, + "children": [ + { + "name": "ElasticsearchIndexScan", + "description": { + "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\"],\"excludes\":[]}}, searchDone=false)" + }, + "children": [] + } + ] + } + } + Filter Merge Into Query DSL --------------------------- @@ -103,7 +128,7 @@ The Filter operator will merge into Elasticsearch Query DSL:: { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false)" + "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false)" }, "children": [] } @@ -129,7 +154,7 @@ The Sort operator will merge into Elasticsearch Query DSL:: { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false)" + "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":200,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false)" }, "children": [] } @@ -191,7 +216,7 @@ The Limit operator will merge in Elasticsearch Query DSL:: { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":5,\"size\":10,\"timeout\":\"1m\"}, searchDone=false)" + "request": "ElasticsearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":5,\"size\":10,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\"],\"excludes\":[]}}, searchDone=false)" }, "children": [] } diff --git a/doctest/bootstrap.sh b/doctest/bootstrap.sh index 55d841c957..29f6105386 100755 --- a/doctest/bootstrap.sh +++ b/doctest/bootstrap.sh @@ -21,5 +21,4 @@ fi $DIR/.venv/bin/pip install -U pip setuptools wheel $DIR/.venv/bin/pip install -r $DIR/requirements.txt -# Temporary fix, add odfe-sql-cli dependency into requirements.txt once we have released cli to PyPI -$DIR/.venv/bin/pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple odfe-sql-cli==0.0.2 +$DIR/.venv/bin/pip install -e ../sql-cli diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScan.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScan.java index 72305dc30b..ade0e695a4 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScan.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScan.java @@ -20,10 +20,12 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.NamedExpression; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanNodeVisitor; import com.google.common.collect.ImmutableList; import java.util.List; +import java.util.Set; import lombok.Builder; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -54,7 +56,7 @@ public class ElasticsearchLogicalIndexScan extends LogicalPlan { * Projection List. */ @Setter - private List projectList; + private Set projectList; /** * Sort List. @@ -75,7 +77,7 @@ public class ElasticsearchLogicalIndexScan extends LogicalPlan { public ElasticsearchLogicalIndexScan( String relationName, Expression filter, - List projectList, + Set projectList, List> sortList, Integer limit, Integer offset) { super(ImmutableList.of()); @@ -95,4 +97,13 @@ public R accept(LogicalPlanNodeVisitor visitor, C context) { public boolean hasLimit() { return limit != null; } + + /** + * Test has projects or not. + * + * @return true for has projects, otherwise false. + */ + public boolean hasProjects() { + return projectList != null && !projectList.isEmpty(); + } } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalPlanOptimizerFactory.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalPlanOptimizerFactory.java index 7ad6f96ea6..b5ec7614cb 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalPlanOptimizerFactory.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalPlanOptimizerFactory.java @@ -25,6 +25,8 @@ import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.MergeSortAndIndexAgg; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.MergeSortAndIndexScan; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.MergeSortAndRelation; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.PushProjectAndIndexScan; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.PushProjectAndRelation; import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.LogicalPlanOptimizer; import java.util.Arrays; import lombok.experimental.UtilityClass; @@ -48,7 +50,9 @@ public static LogicalPlanOptimizer create() { new MergeSortAndIndexAgg(), new MergeSortAndIndexScan(), new MergeLimitAndRelation(), - new - MergeLimitAndIndexScan())); + new MergeLimitAndIndexScan(), + new PushProjectAndRelation(), + new PushProjectAndIndexScan() + )); } } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/OptimizationRuleUtils.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/OptimizationRuleUtils.java index 9326df86cb..b2d3ae746e 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/OptimizationRuleUtils.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/OptimizationRuleUtils.java @@ -18,8 +18,14 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; +import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionNodeVisitor; +import com.amazon.opendistroforelasticsearch.sql.expression.NamedExpression; import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalSort; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import lombok.experimental.UtilityClass; @UtilityClass @@ -50,4 +56,37 @@ public static boolean sortByDefaultOptionOnly(LogicalSort logicalSort) { || Sort.SortOption.DEFAULT_DESC.equals(sort.getLeft())) .reduce(true, Boolean::logicalAnd); } + + /** + * Find reference expression from expression. + * @param expressions a list of expression. + * + * @return a list of ReferenceExpression + */ + public static Set findReferenceExpressions( + List expressions) { + Set projectList = new HashSet<>(); + for (NamedExpression namedExpression : expressions) { + projectList.addAll(findReferenceExpression(namedExpression)); + } + return projectList; + } + + /** + * Find reference expression from expression. + * @param expression expression. + * + * @return a list of ReferenceExpression + */ + public static List findReferenceExpression( + NamedExpression expression) { + List results = new ArrayList<>(); + expression.accept(new ExpressionNodeVisitor() { + @Override + public Object visitReference(ReferenceExpression node, Object context) { + return results.add(node); + } + }, null); + return results; + } } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/PushProjectAndIndexScan.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/PushProjectAndIndexScan.java new file mode 100644 index 0000000000..5a1d4b9300 --- /dev/null +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/PushProjectAndIndexScan.java @@ -0,0 +1,73 @@ +/* + * + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + * + */ + +package com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule; + +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.OptimizationRuleUtils.findReferenceExpressions; +import static com.amazon.opendistroforelasticsearch.sql.planner.optimizer.pattern.Patterns.source; +import static com.facebook.presto.matching.Pattern.typeOf; + +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.ElasticsearchLogicalIndexScan; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; +import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.Rule; +import com.facebook.presto.matching.Capture; +import com.facebook.presto.matching.Captures; +import com.facebook.presto.matching.Pattern; +import java.util.Set; + +/** + * Push Project list into ElasticsearchLogicalIndexScan. + */ +public class PushProjectAndIndexScan implements Rule { + + private final Capture indexScanCapture; + + private final Pattern pattern; + + private Set pushDownProjects; + + /** + * Constructor of MergeProjectAndIndexScan. + */ + public PushProjectAndIndexScan() { + this.indexScanCapture = Capture.newCapture(); + this.pattern = typeOf(LogicalProject.class).matching( + project -> { + pushDownProjects = findReferenceExpressions(project.getProjectList()); + return !pushDownProjects.isEmpty(); + }).with(source() + .matching(typeOf(ElasticsearchLogicalIndexScan.class) + .matching(indexScan -> !indexScan.hasProjects()) + .capturedAs(indexScanCapture))); + + } + + @Override + public Pattern pattern() { + return pattern; + } + + @Override + public LogicalPlan apply(LogicalProject project, + Captures captures) { + ElasticsearchLogicalIndexScan indexScan = captures.get(indexScanCapture); + indexScan.setProjectList(pushDownProjects); + return new LogicalProject(indexScan, project.getProjectList()); + } +} diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/PushProjectAndRelation.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/PushProjectAndRelation.java new file mode 100644 index 0000000000..f8960de02b --- /dev/null +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/rule/PushProjectAndRelation.java @@ -0,0 +1,77 @@ +/* + * + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + * + */ + +package com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule; + +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.rule.OptimizationRuleUtils.findReferenceExpressions; +import static com.amazon.opendistroforelasticsearch.sql.planner.optimizer.pattern.Patterns.source; +import static com.facebook.presto.matching.Pattern.typeOf; + +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.ElasticsearchLogicalIndexScan; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation; +import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.Rule; +import com.facebook.presto.matching.Capture; +import com.facebook.presto.matching.Captures; +import com.facebook.presto.matching.Pattern; +import java.util.Set; + +/** + * Push Project list into Relation. The transformed plan is Project - IndexScan + */ +public class PushProjectAndRelation implements Rule { + + private final Capture relationCapture; + + private final Pattern pattern; + + private Set pushDownProjects; + + /** + * Constructor of MergeProjectAndRelation. + */ + public PushProjectAndRelation() { + this.relationCapture = Capture.newCapture(); + this.pattern = typeOf(LogicalProject.class) + .matching(project -> { + pushDownProjects = findReferenceExpressions(project.getProjectList()); + return !pushDownProjects.isEmpty(); + }) + .with(source().matching(typeOf(LogicalRelation.class).capturedAs(relationCapture))); + } + + @Override + public Pattern pattern() { + return pattern; + } + + @Override + public LogicalPlan apply(LogicalProject project, + Captures captures) { + LogicalRelation relation = captures.get(relationCapture); + return new LogicalProject( + ElasticsearchLogicalIndexScan + .builder() + .relationName(relation.getRelationName()) + .projectList(findReferenceExpressions(project.getProjectList())) + .build(), + project.getProjectList() + ); + } +} diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequest.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequest.java index 9d8dce2be5..626ae96d4e 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequest.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequest.java @@ -34,6 +34,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; /** @@ -113,7 +114,10 @@ public Map getFieldTypes() { Map fieldTypes = new HashMap<>(); Map indexMappings = client.getIndexMappings(indexName); for (IndexMapping indexMapping : indexMappings.values()) { - fieldTypes.putAll(indexMapping.getAllFieldTypes(this::transformESTypeToExprType)); + fieldTypes + .putAll(indexMapping.getAllFieldTypes(this::transformESTypeToExprType).entrySet().stream() + .filter(entry -> !ExprCoreType.UNKNOWN.equals(entry.getValue())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); } return fieldTypes; } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndex.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndex.java index 093d182f9f..accfff5285 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndex.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndex.java @@ -124,6 +124,10 @@ public PhysicalPlan visitIndexScan(ElasticsearchLogicalIndexScan node, if (node.getLimit() != null) { context.pushDownLimit(node.getLimit(), node.getOffset()); } + + if (node.hasProjects()) { + context.pushDownProjects(node.getProjectList()); + } return indexScan; } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScan.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScan.java index 34ff157e34..56de50e414 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScan.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScan.java @@ -27,12 +27,15 @@ import com.amazon.opendistroforelasticsearch.sql.elasticsearch.request.ElasticsearchQueryRequest; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.request.ElasticsearchRequest; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.response.ElasticsearchResponse; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; import com.amazon.opendistroforelasticsearch.sql.storage.TableScanOperator; import com.google.common.collect.Iterables; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.ToString; @@ -152,6 +155,16 @@ public void pushDownLimit(Integer limit, Integer offset) { sourceBuilder.from(offset).size(limit); } + /** + * Push down project list to DSL requets. + */ + public void pushDownProjects(Set projects) { + SearchSourceBuilder sourceBuilder = request.getSourceBuilder(); + final Set projectsSet = + projects.stream().map(ReferenceExpression::getAttr).collect(Collectors.toSet()); + sourceBuilder.fetchSource(projectsSet.toArray(new String[0]), new String[0]); + } + public void pushTypeMapping(Map typeMapping) { request.getExprValueFactory().setTypeMapping(typeMapping); } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/AggregationQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/AggregationQueryBuilder.java index b2d16fdf4f..d0081ae27d 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/AggregationQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/AggregationQueryBuilder.java @@ -101,7 +101,7 @@ public Map buildTypeMapping( List groupByList) { ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); namedAggregatorList.forEach(agg -> builder.put(agg.getName(), agg.type())); - groupByList.forEach(group -> builder.put(group.getName(), group.type())); + groupByList.forEach(group -> builder.put(group.getNameOrAlias(), group.type())); return builder.build(); } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/dsl/BucketAggregationBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/dsl/BucketAggregationBuilder.java index 3449f1bc9f..158a685a31 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/dsl/BucketAggregationBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/aggregation/dsl/BucketAggregationBuilder.java @@ -47,7 +47,7 @@ public List> build( new ImmutableList.Builder<>(); for (Pair groupPair : groupList) { TermsValuesSourceBuilder valuesSourceBuilder = - new TermsValuesSourceBuilder(groupPair.getLeft().getName()) + new TermsValuesSourceBuilder(groupPair.getLeft().getNameOrAlias()) .missingBucket(true) .order(groupPair.getRight()); resultBuilder diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicOptimizerTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicOptimizerTest.java index 8cca66daae..6176c91c03 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicOptimizerTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicOptimizerTest.java @@ -24,6 +24,8 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.indexScan; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.indexScanAgg; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.noProjects; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.projects; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.aggregation; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.filter; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.limit; @@ -33,12 +35,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.optimizer.LogicalPlanOptimizer; import com.google.common.collect.ImmutableList; -import java.util.Collections; +import com.google.common.collect.ImmutableSet; import org.apache.commons.lang3.tuple.Pair; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -56,7 +59,8 @@ void project_filter_merge_with_relation() { assertEquals( project( indexScan("schema", - dsl.equal(DSL.ref("intV", INTEGER), DSL.literal(integerValue(1)))), + dsl.equal(DSL.ref("intV", INTEGER), DSL.literal(integerValue(1))), + ImmutableSet.of(DSL.ref("intV", INTEGER))), DSL.named("i", DSL.ref("intV", INTEGER)) ), optimize( @@ -136,7 +140,7 @@ void aggregation_cant_merge_indexScan_with_project() { aggregation( ElasticsearchLogicalIndexScan.builder().relationName("schema") .filter(dsl.equal(DSL.ref("intV", INTEGER), DSL.literal(integerValue(1)))) - .projectList(Collections.singletonList(DSL.named("i", DSL.ref("intV", INTEGER)))) + .projectList(ImmutableSet.of(DSL.ref("intV", INTEGER))) .build(), ImmutableList .of(DSL.named("AVG(intV)", @@ -148,7 +152,7 @@ void aggregation_cant_merge_indexScan_with_project() { ElasticsearchLogicalIndexScan.builder().relationName("schema") .filter(dsl.equal(DSL.ref("intV", INTEGER), DSL.literal(integerValue(1)))) .projectList( - Collections.singletonList(DSL.named("i", DSL.ref("intV", INTEGER)))) + ImmutableSet.of(DSL.ref("intV", INTEGER))) .build(), ImmutableList .of(DSL.named("AVG(intV)", @@ -341,7 +345,7 @@ void sort_with_customized_option_should_not_merge_with_indexAgg() { void limit_merge_with_relation() { assertEquals( project( - indexScan("schema", 1, 1), + indexScan("schema", 1, 1, projects(DSL.ref("intV", INTEGER))), DSL.named("intV", DSL.ref("intV", INTEGER)) ), optimize( @@ -362,7 +366,8 @@ void limit_merge_with_index_scan() { project( indexScan("schema", dsl.equal(DSL.ref("intV", INTEGER), DSL.literal(integerValue(1))), - 1, 1 + 1, 1, + projects(DSL.ref("intV", INTEGER)) ), DSL.named("intV", DSL.ref("intV", INTEGER)) ), @@ -386,7 +391,8 @@ void limit_merge_with_index_scan_sort() { indexScan("schema", dsl.equal(DSL.ref("intV", INTEGER), DSL.literal(integerValue(1))), 1, 1, - Pair.of(Sort.SortOption.DEFAULT_ASC, DSL.ref("longV", LONG)) + Utils.sort(DSL.ref("longV", LONG), Sort.SortOption.DEFAULT_ASC), + projects(DSL.ref("intV", INTEGER)) ), DSL.named("intV", DSL.ref("intV", INTEGER)) ), @@ -412,7 +418,7 @@ void aggregation_cant_merge_index_scan_with_limit() { assertEquals( project( aggregation( - indexScan("schema", 10, 0), + indexScan("schema", 10, 0, noProjects()), ImmutableList .of(DSL.named("AVG(intV)", dsl.avg(DSL.ref("intV", INTEGER)))), @@ -422,7 +428,7 @@ void aggregation_cant_merge_index_scan_with_limit() { optimize( project( aggregation( - indexScan("schema", 10, 0), + indexScan("schema", 10, 0, noProjects()), ImmutableList .of(DSL.named("AVG(intV)", dsl.avg(DSL.ref("intV", INTEGER)))), @@ -431,6 +437,90 @@ void aggregation_cant_merge_index_scan_with_limit() { DSL.named("AVG(intV)", DSL.ref("AVG(intV)", DOUBLE))))); } + @Test + void push_down_projectList_to_relation() { + assertEquals( + project( + indexScan("schema", projects(DSL.ref("intV", INTEGER))), + DSL.named("i", DSL.ref("intV", INTEGER)) + ), + optimize( + project( + relation("schema"), + DSL.named("i", DSL.ref("intV", INTEGER))) + ) + ); + } + + /** + * Project(intV, abs(intV)) -> Relation. + * -- will be optimized as + * Project(intV, abs(intV)) -> Relation(project=intV). + */ + @Test + void push_down_should_handle_duplication() { + assertEquals( + project( + indexScan("schema", projects(DSL.ref("intV", INTEGER))), + DSL.named("i", DSL.ref("intV", INTEGER)), + DSL.named("absi", dsl.abs(DSL.ref("intV", INTEGER))) + ), + optimize( + project( + relation("schema"), + DSL.named("i", DSL.ref("intV", INTEGER)), + DSL.named("absi", dsl.abs(DSL.ref("intV", INTEGER)))) + ) + ); + } + + /** + * Project(ListA) -> Project(ListB) -> Relation. + * -- will be optimized as + * Project(ListA) -> Project(ListB) -> Relation(project=ListB). + */ + @Test + void only_one_project_should_be_push() { + assertEquals( + project( + project( + indexScan("schema", + projects(DSL.ref("intV", INTEGER), DSL.ref("stringV", STRING)) + ), + DSL.named("i", DSL.ref("intV", INTEGER)), + DSL.named("s", DSL.ref("stringV", STRING)) + ), + DSL.named("i", DSL.ref("intV", INTEGER)) + ), + optimize( + project( + project( + relation("schema"), + DSL.named("i", DSL.ref("intV", INTEGER)), + DSL.named("s", DSL.ref("stringV", STRING)) + ), + DSL.named("i", DSL.ref("intV", INTEGER)) + ) + ) + ); + } + + @Test + void project_literal_no_push() { + assertEquals( + project( + relation("schema"), + DSL.named("i", DSL.literal("str")) + ), + optimize( + project( + relation("schema"), + DSL.named("i", DSL.literal("str")) + ) + ) + ); + } + private LogicalPlan optimize(LogicalPlan plan) { final LogicalPlanOptimizer optimizer = ElasticsearchLogicalPlanOptimizerFactory.create(); final LogicalPlan optimize = optimizer.optimize(plan); diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScanTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScanTest.java new file mode 100644 index 0000000000..8db3abbadb --- /dev/null +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/planner/logical/ElasticsearchLogicalIndexScanTest.java @@ -0,0 +1,35 @@ +/* + * + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + * + */ + +package com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical; + +import static org.junit.jupiter.api.Assertions.assertFalse; + +import com.google.common.collect.ImmutableSet; +import org.junit.jupiter.api.Test; + +class ElasticsearchLogicalIndexScanTest { + + @Test + void has_projects() { + assertFalse(ElasticsearchLogicalIndexScan.builder() + .projectList(ImmutableSet.of()).build() + .hasProjects()); + + assertFalse(ElasticsearchLogicalIndexScan.builder().build().hasProjects()); + } +} \ No newline at end of file diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequestTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequestTest.java index 910fb055ca..08b1725d02 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequestTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/request/system/ElasticsearchDescribeIndexRequestTest.java @@ -18,6 +18,7 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.request.system; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.stringValue; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.hasEntry; @@ -25,10 +26,12 @@ import static org.mockito.Mockito.when; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.client.ElasticsearchClient; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.mapping.IndexMapping; import com.google.common.collect.ImmutableMap; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -65,4 +68,22 @@ void testToString() { assertEquals("ElasticsearchDescribeIndexRequest{indexName='index'}", new ElasticsearchDescribeIndexRequest(client, "index").toString()); } + + @Test + void filterOutUnknownType() { + when(client.getIndexMappings("index")) + .thenReturn( + ImmutableMap.of( + "test", + new IndexMapping( + ImmutableMap.builder() + .put("name", "keyword") + .put("@timestamp", "alias") + .build()))); + + final Map fieldTypes = + new ElasticsearchDescribeIndexRequest(client, "index").getFieldTypes(); + assertEquals(1, fieldTypes.size()); + assertThat(fieldTypes, hasEntry("name", STRING)); + } } \ No newline at end of file diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScanTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScanTest.java index ebb41d27ed..1b49dd55e5 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScanTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexScanTest.java @@ -36,8 +36,10 @@ import com.amazon.opendistroforelasticsearch.sql.elasticsearch.request.ElasticsearchQueryRequest; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.request.ElasticsearchRequest; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.response.ElasticsearchResponse; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; import com.google.common.collect.ImmutableMap; import java.util.Arrays; +import java.util.Set; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; @@ -155,7 +157,6 @@ PushDownAssertion shouldQuery(QueryBuilder expected) { indexScan.open(); return this; } - } private void mockResponse(ExprValue[]... searchHitBatches) { diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexTest.java index 6a71d1f603..8775fecd18 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/ElasticsearchIndexTest.java @@ -21,6 +21,8 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.indexScan; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.indexScanAgg; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.noProjects; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.utils.Utils.projects; import static com.amazon.opendistroforelasticsearch.sql.expression.DSL.literal; import static com.amazon.opendistroforelasticsearch.sql.expression.DSL.named; import static com.amazon.opendistroforelasticsearch.sql.expression.DSL.ref; @@ -36,6 +38,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.hasEntry; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -57,7 +61,6 @@ import com.amazon.opendistroforelasticsearch.sql.expression.aggregation.AvgAggregator; import com.amazon.opendistroforelasticsearch.sql.expression.aggregation.NamedAggregator; import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; -import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL; import com.amazon.opendistroforelasticsearch.sql.planner.physical.AggregationOperator; @@ -66,7 +69,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.SortOperator; import com.amazon.opendistroforelasticsearch.sql.storage.Table; import com.google.common.collect.ImmutableMap; import java.util.Arrays; @@ -74,6 +76,7 @@ import java.util.Map; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; +import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -359,7 +362,7 @@ void shouldImplIndexScanWithLimit() { project( indexScan( indexName, - 1, 1 + 1, 1, noProjects() ), named)); @@ -382,7 +385,8 @@ void shouldImplIndexScanWithSortAndLimit() { indexScan( indexName, sortExpr, - 1, 1 + 1, 1, + noProjects() ), named)); @@ -413,4 +417,27 @@ void shouldNotPushDownLimitFarFromRelationButUpdateScanSize() { assertTrue(plan instanceof ProjectOperator); assertTrue(((ProjectOperator) plan).getInput() instanceof LimitOperator); } + + @Test + void shouldPushDownProjects() { + when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + + String indexName = "test"; + ElasticsearchIndex index = new ElasticsearchIndex(client, settings, indexName); + PhysicalPlan plan = index.implement( + project( + indexScan( + indexName, projects(ref("intV", INTEGER)) + ), + named("i", ref("intV", INTEGER)))); + + assertTrue(plan instanceof ProjectOperator); + assertTrue(((ProjectOperator) plan).getInput() instanceof ElasticsearchIndexScan); + + final FetchSourceContext fetchSource = + ((ElasticsearchIndexScan) ((ProjectOperator) plan).getInput()).getRequest() + .getSourceBuilder().fetchSource(); + assertThat(fetchSource.includes(), arrayContaining("intV")); + assertThat(fetchSource.excludes(), emptyArray()); + } } diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/utils/Utils.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/utils/Utils.java index 5cf2a1fe11..bde6c054ac 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/utils/Utils.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/utils/Utils.java @@ -23,12 +23,15 @@ import com.amazon.opendistroforelasticsearch.sql.elasticsearch.planner.logical.ElasticsearchLogicalIndexScan; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.NamedExpression; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; import com.amazon.opendistroforelasticsearch.sql.expression.aggregation.AvgAggregator; import com.amazon.opendistroforelasticsearch.sql.expression.aggregation.NamedAggregator; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; +import com.google.common.collect.ImmutableSet; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; import lombok.experimental.UtilityClass; import org.apache.commons.lang3.tuple.Pair; @@ -67,10 +70,12 @@ public static LogicalPlan indexScan(String tableName, /** * Build ElasticsearchLogicalIndexScan. */ - public static LogicalPlan indexScan(String tableName, Integer offset, Integer limit) { + public static LogicalPlan indexScan(String tableName, Integer offset, Integer limit, + Set projectList) { return ElasticsearchLogicalIndexScan.builder().relationName(tableName) .offset(offset) .limit(limit) + .projectList(projectList) .build(); } @@ -79,11 +84,13 @@ public static LogicalPlan indexScan(String tableName, Integer offset, Integer li */ public static LogicalPlan indexScan(String tableName, Expression filter, - Integer offset, Integer limit) { + Integer offset, Integer limit, + Set projectList) { return ElasticsearchLogicalIndexScan.builder().relationName(tableName) .filter(filter) .offset(offset) .limit(limit) + .projectList(projectList) .build(); } @@ -93,12 +100,37 @@ public static LogicalPlan indexScan(String tableName, public static LogicalPlan indexScan(String tableName, Expression filter, Integer offset, Integer limit, - Pair... sorts) { + List> sorts, + Set projectList) { return ElasticsearchLogicalIndexScan.builder().relationName(tableName) .filter(filter) - .sortList(Arrays.asList(sorts)) + .sortList(sorts) .offset(offset) .limit(limit) + .projectList(projectList) + .build(); + } + + /** + * Build ElasticsearchLogicalIndexScan. + */ + public static LogicalPlan indexScan(String tableName, + Set projects) { + return ElasticsearchLogicalIndexScan.builder() + .relationName(tableName) + .projectList(projects) + .build(); + } + + /** + * Build ElasticsearchLogicalIndexScan. + */ + public static LogicalPlan indexScan(String tableName, Expression filter, + Set projects) { + return ElasticsearchLogicalIndexScan.builder() + .relationName(tableName) + .filter(filter) + .projectList(projects) .build(); } @@ -155,4 +187,12 @@ public static List> sort(Expression expr1, Sort.SortOption option2) { return Arrays.asList(Pair.of(option1, expr1), Pair.of(option2, expr2)); } + + public static Set projects(ReferenceExpression... expressions) { + return ImmutableSet.copyOf(expressions); + } + + public static Set noProjects() { + return null; + } } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationExpressionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationExpressionIT.java index 028b51b8c7..5d37b0ba0b 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationExpressionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationExpressionIT.java @@ -83,6 +83,7 @@ public void noGroupKeyAvgOnIntegerShouldPass() { @Test public void hasGroupKeyAvgOnIntegerShouldPass() { + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest(String.format( "SELECT gender, AVG(age) as avg " + "FROM %s " + @@ -91,7 +92,7 @@ public void hasGroupKeyAvgOnIntegerShouldPass() { verifySchema(response, schema("gender", null, "text"), - schema("avg", "avg", "double")); + schema("AVG(age)", "avg", "double")); verifyDataRows(response, rows("m", 34.25), rows("f", 33.666666666666664d)); @@ -181,6 +182,8 @@ public void AddLiteralOnGroupKeyShouldPass() { @Test public void logWithAddLiteralOnGroupKeyShouldPass() { + Assume.assumeTrue(isNewQueryEngineEabled()); + JSONObject response = executeJdbcRequest(String.format( "SELECT gender, Log(age+10) as logAge, max(balance) as max " + "FROM %s " + @@ -191,8 +194,8 @@ public void logWithAddLiteralOnGroupKeyShouldPass() { verifySchema(response, schema("gender", null, "text"), - schema("logAge", "logAge", "double"), - schema("max", "max", "long")); + schema("Log(age+10)", "logAge", "double"), + schema("max(balance)", "max", "long")); verifyDataRows(response, rows("m", 3.4011973816621555d, 49568), rows("m", 3.4339872044851463d, 49433)); @@ -264,7 +267,7 @@ public void aggregateCastStatementShouldNotReturnZero() { "SELECT SUM(CAST(male AS INT)) AS male_sum FROM %s", Index.BANK.getName())); - verifySchema(response, schema("male_sum", "male_sum", "integer")); + verifySchema(response, schema("SUM(CAST(male AS INT))", "male_sum", "integer")); verifyDataRows(response, rows(4)); } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationIT.java index 921409f544..caae2f957d 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/AggregationIT.java @@ -45,6 +45,7 @@ import org.json.JSONArray; import org.json.JSONObject; import org.junit.Assert; +import org.junit.Assume; import org.junit.Ignore; import org.junit.Test; @@ -470,10 +471,12 @@ public void orderByAscTest() { @Test public void orderByAliasAscTest() { + Assume.assumeTrue(isNewQueryEngineEabled()); + JSONObject response = executeJdbcRequest(String.format("SELECT COUNT(*) as count FROM %s " + "GROUP BY gender ORDER BY count", TEST_INDEX_ACCOUNT)); - verifySchema(response, schema("count", "count", "integer")); + verifySchema(response, schema("COUNT(*)", "count", "integer")); verifyDataRowsInOrder(response, rows(493), rows(507)); @@ -492,10 +495,12 @@ public void orderByDescTest() throws IOException { @Test public void orderByAliasDescTest() throws IOException { + Assume.assumeTrue(isNewQueryEngineEabled()); + JSONObject response = executeJdbcRequest(String.format("SELECT COUNT(*) as count FROM %s " + "GROUP BY gender ORDER BY count DESC", TEST_INDEX_ACCOUNT)); - verifySchema(response, schema("count", "count", "integer")); + verifySchema(response, schema("COUNT(*)", "count", "integer")); verifyDataRowsInOrder(response, rows(507), rows(493)); @@ -503,13 +508,15 @@ public void orderByAliasDescTest() throws IOException { @Test public void orderByGroupFieldWithAlias() throws IOException { + Assume.assumeTrue(isNewQueryEngineEabled()); + // ORDER BY field name JSONObject response = executeJdbcRequest(String.format("SELECT gender as g, COUNT(*) as count " + "FROM %s GROUP BY gender ORDER BY gender", TEST_INDEX_ACCOUNT)); verifySchema(response, - schema("g", "g", "text"), - schema("count", "count", "integer")); + schema("gender", "g", "text"), + schema("COUNT(*)", "count", "integer")); verifyDataRowsInOrder(response, rows("f", 493), rows("m", 507)); @@ -519,8 +526,8 @@ public void orderByGroupFieldWithAlias() throws IOException { + "FROM %s GROUP BY gender ORDER BY g", TEST_INDEX_ACCOUNT)); verifySchema(response, - schema("g", "g", "text"), - schema("count", "count", "integer")); + schema("gender", "g", "text"), + schema("COUNT(*)", "count", "integer")); verifyDataRowsInOrder(response, rows("f", 493), rows("m", 507)); diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/PrettyFormatResponseIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/PrettyFormatResponseIT.java index 2fbf70dfeb..7ade383945 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/PrettyFormatResponseIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/PrettyFormatResponseIT.java @@ -357,6 +357,8 @@ public void aggregationFunctionInSelectCaseCheck() throws IOException { @Test public void aggregationFunctionInSelectWithAlias() throws IOException { + Assume.assumeFalse(isNewQueryEngineEabled()); + JSONObject response = executeQuery( String.format(Locale.ROOT, "SELECT COUNT(*) AS total FROM %s GROUP BY age", TestsConstants.TEST_INDEX_ACCOUNT)); diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLFunctionsIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLFunctionsIT.java index b95d07d11a..85c785a5f3 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLFunctionsIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLFunctionsIT.java @@ -227,7 +227,7 @@ public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { " ORDER BY balance DESC LIMIT 1"); verifySchema(response, - schema("cast_balance", null, "float")); + schema("CAST(balance AS FLOAT)", "cast_balance", "float")); verifyDataRows(response, rows(49989.0)); @@ -242,7 +242,7 @@ public void castIntFieldToFloatWithAliasJdbcFormatTest() { "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY jdbc_float_alias LIMIT 1"); verifySchema(response, - schema("jdbc_float_alias", null, "float")); + schema("CAST(balance AS FLOAT)", "jdbc_float_alias", "float")); verifyDataRows(response, rows(1011.0)); @@ -394,10 +394,10 @@ public void castBoolFieldToNumericValueInSelectClause() { verifySchema(response, schema("male", "boolean"), - schema("cast_int", "integer"), - schema("cast_long", "long"), - schema("cast_float", "float"), - schema("cast_double", "double") + schema("CAST(male AS INT)", "cast_int", "integer"), + schema("CAST(male AS LONG)", "cast_long", "long"), + schema("CAST(male AS FLOAT)", "cast_float", "float"), + schema("CAST(male AS DOUBLE)", "cast_double", "double") ); verifyDataRows(response, rows(true, 1, 1, 1.0, 1.0), @@ -419,7 +419,7 @@ public void castBoolFieldToNumericValueWithGroupByAlias() { ); verifySchema(response, - schema("cast_int", "cast_int", "integer"), + schema("CAST(male AS INT)", "cast_int", "integer"), schema("COUNT(*)", "integer") ); verifyDataRows(response, diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/ExplainIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/ExplainIT.java index 220a882bec..06d1c314d0 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/ExplainIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/ExplainIT.java @@ -60,7 +60,8 @@ public void testFilterPushDownExplain() throws Exception { "source=elasticsearch-sql_test_index_account" + "| where age > 30 " + "| where age < 40 " - + "| where balance > 10000 ") + + "| where balance > 10000 " + + "| fields age") ); } @@ -80,12 +81,14 @@ public void testFilterAndAggPushDownExplain() throws Exception { @Test public void testSortPushDownExplain() throws Exception { String expected = loadFromFile("expectedOutput/ppl/explain_sort_push.json"); + assertJsonEquals( expected, explainQueryToString( "source=elasticsearch-sql_test_index_account" + "| sort age " - + "| where age > 30") + + "| where age > 30" + + "| fields age") ); } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TextCommandIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TextCommandIT.java index 6b4e3182f7..ef8b1b1e50 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TextCommandIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TextCommandIT.java @@ -92,6 +92,11 @@ public void testTrim() throws IOException { verifyQuery("trim", "", "", "hello", "world", "helloworld"); } + @Test + public void testRight() throws IOException { + verifyQuery("right", "", ", 3", "llo", "rld", "rld"); + } + @Test public void testRtrim() throws IOException { verifyQuery("rtrim", "", "", "hello", "world", "helloworld"); diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/JdbcFormatIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/JdbcFormatIT.java index 51cf961ca5..836cb5636c 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/JdbcFormatIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/JdbcFormatIT.java @@ -52,7 +52,7 @@ public void testAliasInSchema() { JSONObject response = new JSONObject(executeQuery( "SELECT account_number AS acc FROM " + TEST_INDEX_BANK, "jdbc")); - verifySchema(response, schema("acc", "acc", "long")); + verifySchema(response, schema("account_number", "acc", "long")); } } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MetricsIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MetricsIT.java new file mode 100644 index 0000000000..473fe4402b --- /dev/null +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MetricsIT.java @@ -0,0 +1,78 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + * + */ + +package com.amazon.opendistroforelasticsearch.sql.sql; + +import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; + +import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; +import com.amazon.opendistroforelasticsearch.sql.legacy.metrics.MetricName; +import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.util.Locale; +import java.util.concurrent.TimeUnit; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.json.JSONObject; +import org.junit.Assert; +import org.junit.Test; + +public class MetricsIT extends SQLIntegTestCase { + + @Override + protected void init() throws Exception { + loadIndex(Index.BANK); + TestUtils.enableNewQueryEngine(client()); + } + + @Test + public void requestCount() throws IOException, InterruptedException { + int beforeQueries = requestTotal(); + executeQuery(String.format(Locale.ROOT, "select age from %s", TEST_INDEX_BANK)); + TimeUnit.SECONDS.sleep(2L); + + assertEquals(beforeQueries + 1, requestTotal()); + } + + private Request makeStatRequest() { + return new Request( + "GET", "/_opendistro/_sql/stats" + ); + } + + private int requestTotal() throws IOException { + JSONObject jsonObject = new JSONObject(executeStatRequest(makeStatRequest())); + return jsonObject.getInt(MetricName.REQ_TOTAL.getName()); + } + + private String executeStatRequest(final Request request) throws IOException { + Response response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + + InputStream is = response.getEntity().getContent(); + StringBuilder sb = new StringBuilder(); + try (BufferedReader br = new BufferedReader(new InputStreamReader(is))) { + String line; + while ((line = br.readLine()) != null) { + sb.append(line); + } + } + return sb.toString(); + } +} diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/TextFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/TextFunctionIT.java index d972a10c36..a096b5b201 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/TextFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/TextFunctionIT.java @@ -126,6 +126,11 @@ public void testStrcmp() throws IOException { verifyQuery("strcmp('hello', 'hello')", "integer", 0); } + @Test + public void testRight() throws IOException { + verifyQuery("right('variable', 4)", "keyword", "able"); + } + protected JSONObject executeQuery(String query) throws IOException { Request request = new Request("POST", QUERY_API_ENDPOINT); request.setJsonEntity(String.format(Locale.ROOT, "{\n" + " \"query\": \"%s\"\n" + "}", query)); diff --git a/integ-test/src/test/resources/correctness/bugfixes/916.txt b/integ-test/src/test/resources/correctness/bugfixes/916.txt new file mode 100644 index 0000000000..44a715f0eb --- /dev/null +++ b/integ-test/src/test/resources/correctness/bugfixes/916.txt @@ -0,0 +1 @@ +SELECT COUNT(timestamp) FROM kibana_sample_data_flights diff --git a/integ-test/src/test/resources/correctness/expressions/text_functions.txt b/integ-test/src/test/resources/correctness/expressions/text_functions.txt new file mode 100644 index 0000000000..a9d4ea76e8 --- /dev/null +++ b/integ-test/src/test/resources/correctness/expressions/text_functions.txt @@ -0,0 +1 @@ +RIGHT('Hello World', 5) as column \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/queries/aggregation.txt b/integ-test/src/test/resources/correctness/queries/aggregation.txt index 3a2081d9a8..e3878be86c 100644 --- a/integ-test/src/test/resources/correctness/queries/aggregation.txt +++ b/integ-test/src/test/resources/correctness/queries/aggregation.txt @@ -1,4 +1,5 @@ SELECT COUNT(AvgTicketPrice) FROM kibana_sample_data_flights +SELECT count(timestamp) from kibana_sample_data_flights SELECT AVG(AvgTicketPrice) FROM kibana_sample_data_flights SELECT SUM(AvgTicketPrice) FROM kibana_sample_data_flights SELECT MAX(AvgTicketPrice) FROM kibana_sample_data_flights diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json index 8e7949098e..1e5b36322b 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json @@ -2,13 +2,13 @@ "root": { "name": "ProjectOperator", "description": { - "fields": "[account_number, firstname, address, gender, city, lastname, balance, employer, state, age, email]" + "fields": "[age]" }, "children": [ { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName\u003delasticsearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"filter\":[{\"range\":{\"balance\":{\"from\":10000,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":null,\"to\":40,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone\u003dfalse)" + "request": "ElasticsearchQueryRequest(indexName\u003delasticsearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"filter\":[{\"range\":{\"balance\":{\"from\":10000,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":null,\"to\":40,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone\u003dfalse)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json index b5a9827521..1d62ac9f91 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json @@ -2,13 +2,13 @@ "root": { "name": "ProjectOperator", "description": { - "fields": "[account_number, firstname, address, gender, city, lastname, balance, employer, state, age, email]" + "fields": "[age]" }, "children": [ { "name": "ElasticsearchIndexScan", "description": { - "request": "ElasticsearchQueryRequest(indexName\u003delasticsearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone\u003dfalse)" + "request": "ElasticsearchQueryRequest(indexName\u003delasticsearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone\u003dfalse)" }, "children": [] } diff --git a/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryAction.java b/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryAction.java index 3e9809fead..5d0fe66221 100644 --- a/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryAction.java +++ b/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/RestSQLQueryAction.java @@ -18,14 +18,28 @@ import static com.amazon.opendistroforelasticsearch.sql.executor.ExecutionEngine.QueryResponse; import static com.amazon.opendistroforelasticsearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; +import static org.elasticsearch.rest.RestStatus.BAD_REQUEST; import static org.elasticsearch.rest.RestStatus.INTERNAL_SERVER_ERROR; import static org.elasticsearch.rest.RestStatus.OK; +import static org.elasticsearch.rest.RestStatus.SERVICE_UNAVAILABLE; + +import com.alibaba.druid.sql.parser.ParserException; import com.amazon.opendistroforelasticsearch.sql.common.antlr.SyntaxCheckException; import com.amazon.opendistroforelasticsearch.sql.common.response.ResponseListener; import com.amazon.opendistroforelasticsearch.sql.common.setting.Settings; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.security.SecurityAccess; +import com.amazon.opendistroforelasticsearch.sql.exception.QueryEngineException; +import com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException; import com.amazon.opendistroforelasticsearch.sql.executor.ExecutionEngine.ExplainResponse; +import com.amazon.opendistroforelasticsearch.sql.legacy.antlr.SqlAnalysisException; +import com.amazon.opendistroforelasticsearch.sql.legacy.exception.SQLFeatureDisabledException; +import com.amazon.opendistroforelasticsearch.sql.legacy.exception.SqlParseException; +import com.amazon.opendistroforelasticsearch.sql.legacy.executor.format.ErrorMessageFactory; +import com.amazon.opendistroforelasticsearch.sql.legacy.metrics.MetricName; +import com.amazon.opendistroforelasticsearch.sql.legacy.metrics.Metrics; +import com.amazon.opendistroforelasticsearch.sql.legacy.rewriter.matchtoterm.VerificationException; +import com.amazon.opendistroforelasticsearch.sql.legacy.utils.LogUtils; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.protocol.response.QueryResult; import com.amazon.opendistroforelasticsearch.sql.protocol.response.format.JdbcResponseFormatter; @@ -38,11 +52,13 @@ import com.amazon.opendistroforelasticsearch.sql.sql.domain.SQLQueryRequest; import java.io.IOException; import java.security.PrivilegedExceptionAction; +import java.sql.SQLFeatureNotSupportedException; import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestChannel; @@ -153,6 +169,7 @@ protected Object buildJsonObject(ExplainResponse response) { @Override public void onFailure(Exception e) { LOG.error("Error happened during explain", e); + logAndPublishMetrics(e); sendResponse(channel, INTERNAL_SERVER_ERROR, "Failed to explain the query due to error: " + e.getMessage()); } @@ -177,6 +194,7 @@ public void onResponse(QueryResponse response) { @Override public void onFailure(Exception e) { LOG.error("Error happened during query handling", e); + logAndPublishMetrics(e); sendResponse(channel, INTERNAL_SERVER_ERROR, formatter.format(e)); } }; @@ -195,4 +213,8 @@ private void sendResponse(RestChannel channel, RestStatus status, String content status, "application/json; charset=UTF-8", content)); } + private static void logAndPublishMetrics(Exception e) { + LOG.error(LogUtils.getRequestId() + " Server side error during query execution", e); + Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); + } } diff --git a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 index c13656cdf3..15b8830a31 100644 --- a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 @@ -228,6 +228,7 @@ CONCAT: 'CONCAT'; CONCAT_WS: 'CONCAT_WS'; LENGTH: 'LENGTH'; STRCMP: 'STRCMP'; +RIGHT: 'RIGHT'; // BOOL FUNCTIONS LIKE: 'LIKE'; diff --git a/ppl/src/main/antlr/OpenDistroPPLParser.g4 b/ppl/src/main/antlr/OpenDistroPPLParser.g4 index 3378c407b2..d724beb0de 100644 --- a/ppl/src/main/antlr/OpenDistroPPLParser.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLParser.g4 @@ -258,6 +258,7 @@ conditionFunctionBase textFunctionBase : SUBSTR | SUBSTRING | TRIM | LTRIM | RTRIM | LOWER | UPPER | CONCAT | CONCAT_WS | LENGTH | STRCMP + | RIGHT ; /** operators */ diff --git a/protocol/src/main/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResult.java b/protocol/src/main/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResult.java index 83a09366b9..5deb7e0f56 100644 --- a/protocol/src/main/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResult.java +++ b/protocol/src/main/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResult.java @@ -19,6 +19,7 @@ import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.executor.ExecutionEngine; +import com.amazon.opendistroforelasticsearch.sql.executor.ExecutionEngine.Schema.Column; import java.util.Collection; import java.util.Iterator; import java.util.LinkedHashMap; @@ -53,11 +54,13 @@ public int size() { /** * Parse column name from results. * - * @return mapping from column names to its expression type + * @return mapping from column names to its expression type. + * note that column name could be original name or its alias if any. */ public Map columnNameTypes() { Map colNameTypes = new LinkedHashMap<>(); - schema.getColumns().forEach(column -> colNameTypes.put(column.getName(), + schema.getColumns().forEach(column -> colNameTypes.put( + getColumnName(column), column.getExprType().typeName().toLowerCase())); return colNameTypes; } @@ -72,6 +75,10 @@ public Iterator iterator() { .iterator(); } + private String getColumnName(Column column) { + return (column.getAlias() != null) ? column.getAlias() : column.getName(); + } + private Object[] convertExprValuesToValues(Collection exprValues) { return exprValues .stream() diff --git a/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResultTest.java b/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResultTest.java index 785b8949af..5373650b25 100644 --- a/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResultTest.java +++ b/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/QueryResultTest.java @@ -33,8 +33,8 @@ class QueryResultTest { private ExecutionEngine.Schema schema = new ExecutionEngine.Schema(ImmutableList.of( - new ExecutionEngine.Schema.Column("name", "name", STRING), - new ExecutionEngine.Schema.Column("age", "age", INTEGER))); + new ExecutionEngine.Schema.Column("name", null, STRING), + new ExecutionEngine.Schema.Column("age", null, INTEGER))); @Test @@ -63,6 +63,20 @@ void columnNameTypes() { ); } + @Test + void columnNameTypesWithAlias() { + ExecutionEngine.Schema schema = new ExecutionEngine.Schema(ImmutableList.of( + new ExecutionEngine.Schema.Column("name", "n", STRING))); + QueryResult response = new QueryResult( + schema, + Collections.singletonList(tupleValue(ImmutableMap.of("n", "John")))); + + assertEquals( + ImmutableMap.of("n", "string"), + response.columnNameTypes() + ); + } + @Test void columnNameTypesFromEmptyExprValues() { QueryResult response = new QueryResult( diff --git a/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/CsvResponseFormatterTest.java b/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/CsvResponseFormatterTest.java index fdae598121..f720a3e775 100644 --- a/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/CsvResponseFormatterTest.java +++ b/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/CsvResponseFormatterTest.java @@ -53,10 +53,10 @@ void formatResponse() { @Test void sanitizeHeaders() { ExecutionEngine.Schema schema = new ExecutionEngine.Schema(ImmutableList.of( - new ExecutionEngine.Schema.Column("=firstname", "firstname", STRING), - new ExecutionEngine.Schema.Column("+lastname", "lastname", STRING), - new ExecutionEngine.Schema.Column("-city", "city", STRING), - new ExecutionEngine.Schema.Column("@age", "age", INTEGER))); + new ExecutionEngine.Schema.Column("=firstname", null, STRING), + new ExecutionEngine.Schema.Column("+lastname", null, STRING), + new ExecutionEngine.Schema.Column("-city", null, STRING), + new ExecutionEngine.Schema.Column("@age", null, INTEGER))); QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of( "=firstname", "John", "+lastname", "Smith", "-city", "Seattle", "@age", 20)))); diff --git a/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index f7d8d6e710..6c72ed8d38 100644 --- a/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/com/amazon/opendistroforelasticsearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -36,8 +36,8 @@ class SimpleJsonResponseFormatterTest { private final ExecutionEngine.Schema schema = new ExecutionEngine.Schema(ImmutableList.of( - new ExecutionEngine.Schema.Column("firstname", "name", STRING), - new ExecutionEngine.Schema.Column("age", "age", INTEGER))); + new ExecutionEngine.Schema.Column("firstname", null, STRING), + new ExecutionEngine.Schema.Column("age", null, INTEGER))); @Test void formatResponse() { @@ -92,6 +92,21 @@ void formatResponsePretty() { formatter.format(response)); } + @Test + void formatResponseSchemaWithAlias() { + ExecutionEngine.Schema schema = new ExecutionEngine.Schema(ImmutableList.of( + new ExecutionEngine.Schema.Column("firstname", "name", STRING))); + QueryResult response = + new QueryResult( + schema, + ImmutableList.of(tupleValue(ImmutableMap.of("name", "John", "age", 20)))); + SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(COMPACT); + assertEquals( + "{\"schema\":[{\"name\":\"name\",\"type\":\"string\"}]," + + "\"datarows\":[[\"John\",20]],\"total\":1,\"size\":1}", + formatter.format(response)); + } + @Test void formatResponseWithMissingValue() { QueryResult response = diff --git a/sql-cli/src/odfe_sql_cli/formatter.py b/sql-cli/src/odfe_sql_cli/formatter.py index acfb401fe2..de8248ea13 100644 --- a/sql-cli/src/odfe_sql_cli/formatter.py +++ b/sql-cli/src/odfe_sql_cli/formatter.py @@ -74,7 +74,7 @@ def format_output(self, data): # get header and type as lists, for future usage for i in schema: - fields.append(i["name"]) + fields.append(i.get("alias", i["name"])) types.append(i["type"]) output = formatter.format_output(datarows, fields, **self.output_kwargs) diff --git a/sql-cli/tests/test_formatter.py b/sql-cli/tests/test_formatter.py index b0f85c34a5..3131eac91b 100644 --- a/sql-cli/tests/test_formatter.py +++ b/sql-cli/tests/test_formatter.py @@ -79,6 +79,29 @@ def test_format_output(self): ] assert list(results) == expected + def test_format_alias_output(self): + settings = OutputSettings(table_format="psql") + formatter = Formatter(settings) + data = { + "schema": [{"name": "name", "alias": "n", "type": "text"}], + "total": 1, + "datarows": [["Tim"]], + "size": 1, + "status": 200, + } + + results = formatter.format_output(data) + + expected = [ + "fetched rows / total rows = 1/1", + "+-----+", + "| n |", + "|-----|", + "| Tim |", + "+-----+", + ] + assert list(results) == expected + def test_format_array_output(self): settings = OutputSettings(table_format="psql") formatter = Formatter(settings) diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index 5dbc92c86d..cf4df2f4fe 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -345,7 +345,7 @@ dateTimeFunctionName textFunctionName : SUBSTR | SUBSTRING | TRIM | LTRIM | RTRIM | LOWER | UPPER - | CONCAT | CONCAT_WS | SUBSTR | LENGTH | STRCMP + | CONCAT | CONCAT_WS | SUBSTR | LENGTH | STRCMP | RIGHT ; functionArgs