From 4ec92ab9954b1173aece543f119f9e4a6de27d3b Mon Sep 17 00:00:00 2001 From: Dai Date: Thu, 6 Aug 2020 16:57:42 -0700 Subject: [PATCH 01/13] Add lucene builder interface and term query impl --- .../script/filter/FilterQueryBuilder.java | 34 ++++++++-- .../script/filter/lucene/LuceneQuery.java | 66 +++++++++++++++++++ .../script/filter/lucene/TermQuery.java | 32 +++++++++ .../script/filter/FilterQueryBuilderTest.java | 13 ++-- 4 files changed, 131 insertions(+), 14 deletions(-) create mode 100644 elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java create mode 100644 elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index 82fe071c92..36ba4e3387 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -20,10 +20,16 @@ import static java.util.Collections.emptyMap; import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_TYPE; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.LuceneQuery; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.TermQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.serialization.ExpressionSerializer; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionNodeVisitor; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; +import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName; +import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionName; +import com.google.common.collect.ImmutableMap; +import java.util.Map; import java.util.function.BiFunction; import lombok.RequiredArgsConstructor; import org.elasticsearch.index.query.BoolQueryBuilder; @@ -40,6 +46,14 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor luceneQueries = + ImmutableMap.builder() + .put(BuiltinFunctionName.EQUAL.getName(), new TermQuery()) + .build(); + /** * Build Elasticsearch filter query from expression. * @param expr expression @@ -55,16 +69,22 @@ public QueryBuilder build(Expression expr) { } @Override - public QueryBuilder visitFunction(FunctionExpression node, Object context) { - switch (node.getFunctionName().getFunctionName()) { + public QueryBuilder visitFunction(FunctionExpression func, Object context) { + FunctionName name = func.getFunctionName(); + switch (name.getFunctionName()) { case "and": - return buildBoolQuery(node, context, BoolQueryBuilder::must); + return buildBoolQuery(func, context, BoolQueryBuilder::must); case "or": - return buildBoolQuery(node, context, BoolQueryBuilder::should); + return buildBoolQuery(func, context, BoolQueryBuilder::should); case "not": - return buildBoolQuery(node, context, BoolQueryBuilder::mustNot); - default: - return buildScriptQuery(node); + return buildBoolQuery(func, context, BoolQueryBuilder::mustNot); + default: { + LuceneQuery query = luceneQueries.get(name); + if (query != null && query.canSupport(func)) { + return query.build(func); + } + return buildScriptQuery(func); + } } } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java new file mode 100644 index 0000000000..ec7af1fd9d --- /dev/null +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java @@ -0,0 +1,66 @@ +/* + * 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.storage.script.filter.lucene; + +import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; +import com.amazon.opendistroforelasticsearch.sql.expression.LiteralExpression; +import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; +import org.elasticsearch.index.query.QueryBuilder; + +/** + * Lucene query builder that builds Lucene query from function expression. + */ +public interface LuceneQuery { + + /** + * Check if function expression supported by current Lucene query. + * Default behavior is that report supported if: + * 1. Left is a reference + * 2. Right side is a literal + * + * @param func function + * @return return true if supported, otherwise false. + */ + default boolean canSupport(FunctionExpression func) { + return (func.getArguments().size() == 2) + && (func.getArguments().get(0) instanceof ReferenceExpression) + && (func.getArguments().get(1) instanceof LiteralExpression); + } + + /** + * Build Lucene query from function expression. + * + * @param func function + * @return query + */ + default QueryBuilder build(FunctionExpression func) { + ReferenceExpression ref = (ReferenceExpression) func.getArguments().get(0); + LiteralExpression literal = (LiteralExpression) func.getArguments().get(1); + return doBuild(ref.getAttr(), literal.valueOf(null).value()); + } + + /** + * Build method that subclass implements by default which is to build query + * from reference and literal in function arguments. + * + * @param fieldName field name + * @param value value + * @return query + */ + QueryBuilder doBuild(String fieldName, Object value); + +} diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java new file mode 100644 index 0000000000..47adee060c --- /dev/null +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java @@ -0,0 +1,32 @@ +/* + * 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.storage.script.filter.lucene; + +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; + +/** + * Lucene term query. + */ +public class TermQuery implements LuceneQuery { + + @Override + public QueryBuilder doBuild(String fieldName, Object value) { + return QueryBuilders.termQuery(fieldName, value); + } + +} diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index 7d210c16b6..e9821dda3f 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -67,7 +67,7 @@ void should_return_null_if_exception() { assertNull( filterQueryBuilder.build( - dsl.equal(ref("age", INTEGER), literal(30)))); + dsl.equal(dsl.abs(ref("age", INTEGER)), literal(30)))); } @Test @@ -105,12 +105,11 @@ void can_build_query_for_and_or_expression() { + " \"bool\" : {\n" + " \"" + names[i] + "\" : [\n" + " {\n" - + " \"script\" : {\n" - + " \"script\" : {\n" - + " \"source\" : \"name = \\\"John\\\"\",\n" - + " \"lang\" : \"opendistro_expression\"\n" - + " },\n" - + " \"boost\" : 1.0\n" + + " \"term\" : {\n" + + " \"name\" : {\n" + + " \"value\" : \"John\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + " }\n" + " },\n" + " {\n" From f666886b39b82c6990e6010334b62c5ee12db4a2 Mon Sep 17 00:00:00 2001 From: Dai Date: Fri, 7 Aug 2020 10:05:57 -0700 Subject: [PATCH 02/13] Add lucene query interface and term query impl --- .../storage/ElasticsearchIndexScan.java | 4 ++++ .../storage/script/filter/lucene/LuceneQuery.java | 15 +++++++++------ .../storage/script/filter/lucene/TermQuery.java | 13 ++++++++++--- .../script/filter/FilterQueryBuilderTest.java | 7 ++++--- 4 files changed, 27 insertions(+), 12 deletions(-) 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 a28c00fd8d..ce0762e492 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 @@ -16,6 +16,9 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage; +import static org.elasticsearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME; +import static org.elasticsearch.search.sort.SortOrder.ASC; + import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.client.ElasticsearchClient; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.value.ElasticsearchExprValueFactory; @@ -104,6 +107,7 @@ public void pushDown(QueryBuilder query) { .must(query)); } } + source.sort(DOC_FIELD_NAME, ASC); // Make sure consistent order } @Override diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java index ec7af1fd9d..0c726f9018 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java @@ -16,6 +16,8 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; import com.amazon.opendistroforelasticsearch.sql.expression.LiteralExpression; import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; @@ -24,7 +26,7 @@ /** * Lucene query builder that builds Lucene query from function expression. */ -public interface LuceneQuery { +public abstract class LuceneQuery { /** * Check if function expression supported by current Lucene query. @@ -35,7 +37,7 @@ public interface LuceneQuery { * @param func function * @return return true if supported, otherwise false. */ - default boolean canSupport(FunctionExpression func) { + public boolean canSupport(FunctionExpression func) { return (func.getArguments().size() == 2) && (func.getArguments().get(0) instanceof ReferenceExpression) && (func.getArguments().get(1) instanceof LiteralExpression); @@ -47,10 +49,10 @@ default boolean canSupport(FunctionExpression func) { * @param func function * @return query */ - default QueryBuilder build(FunctionExpression func) { + public QueryBuilder build(FunctionExpression func) { ReferenceExpression ref = (ReferenceExpression) func.getArguments().get(0); LiteralExpression literal = (LiteralExpression) func.getArguments().get(1); - return doBuild(ref.getAttr(), literal.valueOf(null).value()); + return doBuild(ref.getAttr(), ref.type(), literal.valueOf(null)); } /** @@ -58,9 +60,10 @@ default QueryBuilder build(FunctionExpression func) { * from reference and literal in function arguments. * * @param fieldName field name - * @param value value + * @param fieldType expr fieldType + * @param literal expr literal * @return query */ - QueryBuilder doBuild(String fieldName, Object value); + protected abstract QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal); } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java index 47adee060c..8bcc250d7b 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java @@ -16,17 +16,24 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_TEXT_KEYWORD; + +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; /** * Lucene term query. */ -public class TermQuery implements LuceneQuery { +public class TermQuery extends LuceneQuery { @Override - public QueryBuilder doBuild(String fieldName, Object value) { - return QueryBuilders.termQuery(fieldName, value); + protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal) { + if (fieldType == ES_TEXT_KEYWORD) { + fieldName += ".keyword"; + } + return QueryBuilders.termQuery(fieldName, literal.value()); } } diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index e9821dda3f..da29051d18 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -18,6 +18,7 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; +import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_TEXT_KEYWORD; import static com.amazon.opendistroforelasticsearch.sql.expression.DSL.literal; import static com.amazon.opendistroforelasticsearch.sql.expression.DSL.ref; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -92,10 +93,10 @@ void can_build_query_for_and_or_expression() { String[] names = { "must", "should" }; Expression[] exprs = { dsl.and( - dsl.equal(ref("name", STRING), literal("John")), + dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")), dsl.greater(ref("age", INTEGER), literal(30))), dsl.or( - dsl.equal(ref("name", STRING), literal("John")), + dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")), dsl.greater(ref("age", INTEGER), literal(30))) }; @@ -106,7 +107,7 @@ void can_build_query_for_and_or_expression() { + " \"" + names[i] + "\" : [\n" + " {\n" + " \"term\" : {\n" - + " \"name\" : {\n" + + " \"name.keyword\" : {\n" + " \"value\" : \"John\",\n" + " \"boost\" : 1.0\n" + " }\n" From 1a332fa4997ea297bf098315218c49d55eb7924b Mon Sep 17 00:00:00 2001 From: Dai Date: Fri, 7 Aug 2020 11:47:20 -0700 Subject: [PATCH 03/13] Add range query impl --- .../script/filter/FilterQueryBuilder.java | 6 ++ .../script/filter/lucene/LuceneQuery.java | 2 +- .../script/filter/lucene/RangeQuery.java | 60 ++++++++++++ .../script/filter/lucene/TermQuery.java | 2 +- .../script/filter/FilterQueryBuilderTest.java | 91 +++++++++++++------ 5 files changed, 129 insertions(+), 32 deletions(-) create mode 100644 elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index 36ba4e3387..9d130c60b3 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -21,6 +21,8 @@ import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_TYPE; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.LuceneQuery; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery.Comparison; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.TermQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.serialization.ExpressionSerializer; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; @@ -52,6 +54,10 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor luceneQueries = ImmutableMap.builder() .put(BuiltinFunctionName.EQUAL.getName(), new TermQuery()) + .put(BuiltinFunctionName.LESS.getName(), new RangeQuery(Comparison.LT)) + .put(BuiltinFunctionName.GREATER.getName(), new RangeQuery(Comparison.GT)) + .put(BuiltinFunctionName.LTE.getName(), new RangeQuery(Comparison.LTE)) + .put(BuiltinFunctionName.GTE.getName(), new RangeQuery(Comparison.GTE)) .build(); /** diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java index 0c726f9018..3673888a65 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQuery.java @@ -24,7 +24,7 @@ import org.elasticsearch.index.query.QueryBuilder; /** - * Lucene query builder that builds Lucene query from function expression. + * Lucene query abstraction that builds Lucene query from function expression. */ public abstract class LuceneQuery { diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java new file mode 100644 index 0000000000..d5c5afebca --- /dev/null +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java @@ -0,0 +1,60 @@ +/* + * 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.storage.script.filter.lucene; + +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; +import lombok.RequiredArgsConstructor; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.RangeQueryBuilder; + +/** + * Lucene range query that builds range query for non-quality comparison. + */ +@RequiredArgsConstructor +public class RangeQuery extends LuceneQuery { + + public enum Comparison { + LT, GT, LTE, GTE + } + + /** + * Comparison that range query build for. + */ + private final Comparison comparison; + + @Override + protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal) { + Object value = literal.value(); + + RangeQueryBuilder query = QueryBuilders.rangeQuery(fieldName); + switch (comparison) { + case LT: + return query.lt(value); + case GT: + return query.gt(value); + case LTE: + return query.lte(value); + case GTE: + return query.gte(value); + default: + throw new IllegalStateException("Comparison is supported by range query: " + comparison); + } + } + +} diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java index 8bcc250d7b..e2c053b3c2 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java @@ -24,7 +24,7 @@ import org.elasticsearch.index.query.QueryBuilders; /** - * Lucene term query. + * Lucene term query that build term query for equality comparison for different data types. */ public class TermQuery extends LuceneQuery { diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index da29051d18..1432e2e541 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -25,12 +25,12 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.serialization.ExpressionSerializer; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; +import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayNameGeneration; @@ -53,17 +53,11 @@ class FilterQueryBuilderTest { @BeforeEach void set_up() { - doAnswer(invocation -> { - Expression expr = invocation.getArgument(0); - return expr.toString(); - }).when(serializer).serialize(any()); - filterQueryBuilder = new FilterQueryBuilder(serializer); } @Test void should_return_null_if_exception() { - reset(serializer); when(serializer.serialize(any())).thenThrow(IllegalStateException.class); assertNull( @@ -71,33 +65,72 @@ void should_return_null_if_exception() { dsl.equal(dsl.abs(ref("age", INTEGER)), literal(30)))); } + @Test + void can_build_query_for_equality_expression() { + assertEquals( + "{\n" + + " \"term\" : {\n" + + " \"name\" : {\n" + + " \"value\" : \"John\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.equal( + ref("name", STRING), literal("John"))) + ); + } + @Test void can_build_query_for_comparison_expression() { + assertEquals( + "{\n" + + " \"range\" : {\n" + + " \"age\" : {\n" + + " \"from\" : 30,\n" + + " \"to\" : null,\n" + + " \"include_lower\" : false,\n" + + " \"include_upper\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.greater( + ref("age", INTEGER), literal(30)))); + } + + @Test + void can_build_query_for_function_expression() { + doAnswer(invocation -> { + Expression expr = invocation.getArgument(0); + return expr.toString(); + }).when(serializer).serialize(any()); + assertEquals( "{\n" + " \"script\" : {\n" + " \"script\" : {\n" - + " \"source\" : \"age > 30\",\n" + + " \"source\" : \"abs(age) = 30\",\n" + " \"lang\" : \"opendistro_expression\"\n" + " },\n" + " \"boost\" : 1.0\n" + " }\n" + "}", buildQuery( - dsl.greater( - ref("age", INTEGER), literal(30)))); + dsl.equal( + dsl.abs(ref("age", INTEGER)), literal(30)))); } @Test void can_build_query_for_and_or_expression() { String[] names = { "must", "should" }; + FunctionExpression expr1 = dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")); + FunctionExpression expr2 = dsl.equal(ref("age", INTEGER), literal(30)); Expression[] exprs = { - dsl.and( - dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")), - dsl.greater(ref("age", INTEGER), literal(30))), - dsl.or( - dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")), - dsl.greater(ref("age", INTEGER), literal(30))) + dsl.and(expr1, expr2), + dsl.or(expr1, expr2) }; for (int i = 0; i < names.length; i++) { @@ -114,12 +147,11 @@ void can_build_query_for_and_or_expression() { + " }\n" + " },\n" + " {\n" - + " \"script\" : {\n" - + " \"script\" : {\n" - + " \"source\" : \"age > 30\",\n" - + " \"lang\" : \"opendistro_expression\"\n" - + " },\n" - + " \"boost\" : 1.0\n" + + " \"term\" : {\n" + + " \"age\" : {\n" + + " \"value\" : 30,\n" + + " \"boost\" : 1.0\n" + + " }\n" + " }\n" + " }\n" + " ],\n" @@ -132,18 +164,17 @@ void can_build_query_for_and_or_expression() { } @Test - void can_build_query_for_or_expression() { + void can_build_query_for_not_expression() { assertEquals( "{\n" + " \"bool\" : {\n" + " \"must_not\" : [\n" + " {\n" - + " \"script\" : {\n" - + " \"script\" : {\n" - + " \"source\" : \"age > 30\",\n" - + " \"lang\" : \"opendistro_expression\"\n" - + " },\n" - + " \"boost\" : 1.0\n" + + " \"term\" : {\n" + + " \"age\" : {\n" + + " \"value\" : 30,\n" + + " \"boost\" : 1.0\n" + + " }\n" + " }\n" + " }\n" + " ],\n" @@ -153,7 +184,7 @@ void can_build_query_for_or_expression() { + "}", buildQuery( dsl.not( - dsl.greater( + dsl.equal( ref("age", INTEGER), literal(30))))); } From e95041264648b8a71f7e1a58a1a1291a682fd3aa Mon Sep 17 00:00:00 2001 From: Dai Date: Fri, 7 Aug 2020 12:04:15 -0700 Subject: [PATCH 04/13] Add more UT for range query --- .../script/filter/FilterQueryBuilderTest.java | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index 1432e2e541..4198437e9d 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -32,6 +32,8 @@ import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; +import com.google.common.collect.ImmutableMap; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.DisplayNameGenerator; @@ -84,21 +86,27 @@ void can_build_query_for_equality_expression() { @Test void can_build_query_for_comparison_expression() { - assertEquals( - "{\n" - + " \"range\" : {\n" - + " \"age\" : {\n" - + " \"from\" : 30,\n" - + " \"to\" : null,\n" - + " \"include_lower\" : false,\n" - + " \"include_upper\" : true,\n" - + " \"boost\" : 1.0\n" - + " }\n" - + " }\n" - + "}", - buildQuery( - dsl.greater( - ref("age", INTEGER), literal(30)))); + Expression[] params = {ref("age", INTEGER), literal(30)}; + Map ranges = ImmutableMap.of( + dsl.less(params), new Object[]{null, 30, true, false}, + dsl.greater(params), new Object[]{30, null, false, true}, + dsl.lte(params), new Object[]{null, 30, true, true}, + dsl.gte(params), new Object[]{30, null, true, true}); + + ranges.forEach((expr, range) -> + assertEquals( + "{\n" + + " \"range\" : {\n" + + " \"age\" : {\n" + + " \"from\" : " + range[0] + ",\n" + + " \"to\" : " + range[1] + ",\n" + + " \"include_lower\" : " + range[2] + ",\n" + + " \"include_upper\" : " + range[3] + ",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery(expr))); } @Test From a2dcde417fb70bcc640e64709dbd9c1566dec81e Mon Sep 17 00:00:00 2001 From: Dai Date: Fri, 7 Aug 2020 12:31:13 -0700 Subject: [PATCH 05/13] Add wildcard query impl --- .../script/filter/FilterQueryBuilder.java | 2 + .../script/filter/lucene/WildcardQuery.java | 37 ++++++++++++++ .../script/filter/FilterQueryBuilderTest.java | 49 +++++++++++++++++-- 3 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/WildcardQuery.java diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index 9d130c60b3..aa4623a5e4 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -24,6 +24,7 @@ import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery.Comparison; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.TermQuery; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.WildcardQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.serialization.ExpressionSerializer; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionNodeVisitor; @@ -58,6 +59,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor ranges = ImmutableMap.of( dsl.less(params), new Object[]{null, 30, true, false}, @@ -110,7 +110,24 @@ void can_build_query_for_comparison_expression() { } @Test - void can_build_query_for_function_expression() { + void should_build_wildcard_query_for_like_expression() { + assertEquals( + "{\n" + + " \"wildcard\" : {\n" + + " \"name\" : {\n" + + " \"wildcard\" : \"*John?\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.like( + ref("name", STRING), literal("%John_"))) + ); + } + + @Test + void should_build_script_query_for_function_expression() { doAnswer(invocation -> { Expression expr = invocation.getArgument(0); return expr.toString(); @@ -132,7 +149,29 @@ void can_build_query_for_function_expression() { } @Test - void can_build_query_for_and_or_expression() { + void should_build_script_query_for_comparison_between_fields() { + doAnswer(invocation -> { + Expression expr = invocation.getArgument(0); + return expr.toString(); + }).when(serializer).serialize(any()); + + assertEquals( + "{\n" + + " \"script\" : {\n" + + " \"script\" : {\n" + + " \"source\" : \"age1 = age2\",\n" + + " \"lang\" : \"opendistro_expression\"\n" + + " },\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}", + buildQuery( + dsl.equal( + ref("age1", INTEGER), ref("age2", INTEGER)))); + } + + @Test + void can_build_bool_query_for_and_or_expression() { String[] names = { "must", "should" }; FunctionExpression expr1 = dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")); FunctionExpression expr2 = dsl.equal(ref("age", INTEGER), literal(30)); @@ -172,7 +211,7 @@ void can_build_query_for_and_or_expression() { } @Test - void can_build_query_for_not_expression() { + void can_build_bool_query_for_not_expression() { assertEquals( "{\n" + " \"bool\" : {\n" From 40e659b8ae745d4cb9ad39cf59cc229688027570 Mon Sep 17 00:00:00 2001 From: Dai Date: Fri, 7 Aug 2020 12:59:58 -0700 Subject: [PATCH 06/13] Add exists query impl --- .../script/filter/FilterQueryBuilder.java | 2 + .../script/filter/lucene/ExistsQuery.java | 42 +++++++++++++++++++ .../script/filter/lucene/LuceneQuery.java | 5 ++- .../script/filter/lucene/RangeQuery.java | 2 +- .../script/filter/lucene/TermQuery.java | 2 +- .../script/filter/lucene/WildcardQuery.java | 11 +++-- .../script/filter/FilterQueryBuilderTest.java | 20 +++++++-- 7 files changed, 73 insertions(+), 11 deletions(-) create mode 100644 elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/ExistsQuery.java diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index aa4623a5e4..8ca8f4bd5b 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -20,6 +20,7 @@ import static java.util.Collections.emptyMap; import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_TYPE; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.ExistsQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.LuceneQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery.Comparison; @@ -60,6 +61,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor Date: Fri, 7 Aug 2020 13:55:19 -0700 Subject: [PATCH 07/13] Pass jacoco test --- .../script/filter/FilterQueryBuilder.java | 2 - .../script/filter/lucene/ExistsQuery.java | 42 ------------------- .../script/filter/lucene/RangeQuery.java | 2 +- .../script/filter/FilterQueryBuilderTest.java | 14 ------- .../script/filter/lucene/LuceneQueryTest.java | 34 +++++++++++++++ .../script/filter/lucene/RangeQueryTest.java | 39 +++++++++++++++++ 6 files changed, 74 insertions(+), 59 deletions(-) delete mode 100644 elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/ExistsQuery.java create mode 100644 elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java create mode 100644 elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index 8ca8f4bd5b..aa4623a5e4 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -20,7 +20,6 @@ import static java.util.Collections.emptyMap; import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_TYPE; -import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.ExistsQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.LuceneQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery.Comparison; @@ -61,7 +60,6 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor { diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java new file mode 100644 index 0000000000..33c47cd461 --- /dev/null +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java @@ -0,0 +1,34 @@ +/* + * 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.storage.script.filter.lucene; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +class LuceneQueryTest { + + @Test + void should_throw_exception_if_not_implemented() { + assertThrows(UnsupportedOperationException.class, () -> + new LuceneQuery(){}.doBuild(null, null, null)); + } + +} \ No newline at end of file diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java new file mode 100644 index 0000000000..c1ed7e5393 --- /dev/null +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java @@ -0,0 +1,39 @@ +/* + * 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.storage.script.filter.lucene; + +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; +import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery.Comparison; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +class RangeQueryTest { + + @Test + void should_throw_exception_for_unsupported_comparison() { + // Note that since we do switch check on enum comparison, this should'be impossible + assertThrows(IllegalStateException.class, () -> + new RangeQuery(Comparison.BETWEEN) + .doBuild("name", STRING, ExprValueUtils.stringValue("John"))); + } + +} \ No newline at end of file From 2f347b011ace1a0c6fcb253c8350bf40ad87d513 Mon Sep 17 00:00:00 2001 From: Dai Date: Tue, 11 Aug 2020 09:13:08 -0700 Subject: [PATCH 08/13] Prepare PR --- .../sql/elasticsearch/storage/ElasticsearchIndexScan.java | 5 ++++- .../storage/script/filter/lucene/TermQuery.java | 2 +- .../elasticsearch/storage/ElasticsearchIndexScanTest.java | 6 +++++- .../storage/script/filter/FilterQueryBuilderTest.java | 4 ++-- 4 files changed, 12 insertions(+), 5 deletions(-) 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 ce0762e492..5e20e1c26c 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 @@ -107,7 +107,10 @@ public void pushDown(QueryBuilder query) { .must(query)); } } - source.sort(DOC_FIELD_NAME, ASC); // Make sure consistent order + + if (source.sorts() == null) { + source.sort(DOC_FIELD_NAME, ASC); // Make sure consistent order + } } @Override diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java index 4042118575..01e3c4d9f0 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/TermQuery.java @@ -30,7 +30,7 @@ public class TermQuery extends LuceneQuery { @Override protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue literal) { - if (fieldType == ES_TEXT_KEYWORD) { + if (fieldType == ES_TEXT_KEYWORD) { // Assume inner field name is always "keyword" fieldName += ".keyword"; } return QueryBuilders.termQuery(fieldName, literal.value()); 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 a6fd518a8d..253abe1130 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 @@ -17,6 +17,8 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; +import static org.elasticsearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME; +import static org.elasticsearch.search.sort.SortOrder.ASC; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -131,7 +133,9 @@ PushDownAssertion pushDown(QueryBuilder query) { PushDownAssertion shouldQuery(QueryBuilder expected) { ElasticsearchRequest request = new ElasticsearchRequest("test"); - request.getSourceBuilder().query(expected); + request.getSourceBuilder() + .query(expected) + .sort(DOC_FIELD_NAME, ASC); when(client.search(request)).thenReturn(response); indexScan.open(); return this; diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index ec413979ee..28d102f435 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -169,7 +169,7 @@ void should_build_script_query_for_comparison_between_fields() { } @Test - void can_build_bool_query_for_and_or_expression() { + void should_build_bool_query_for_and_or_expression() { String[] names = { "must", "should" }; FunctionExpression expr1 = dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")); FunctionExpression expr2 = dsl.equal(ref("age", INTEGER), literal(30)); @@ -209,7 +209,7 @@ void can_build_bool_query_for_and_or_expression() { } @Test - void can_build_bool_query_for_not_expression() { + void should_build_bool_query_for_not_expression() { assertEquals( "{\n" + " \"bool\" : {\n" From 24b5e1a0b251e6707c5ed4f97ec207148e8bf28e Mon Sep 17 00:00:00 2001 From: Dai Date: Thu, 13 Aug 2020 07:38:47 -0700 Subject: [PATCH 09/13] Only push down filter close to relation --- .../storage/ElasticsearchIndex.java | 5 ++++ .../storage/ElasticsearchIndexTest.java | 23 +++++++++++++++ .../sql/ppl/StatsCommandIT.java | 28 +++++++++++++++++++ 3 files changed, 56 insertions(+) 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 05c3743f37..880372080d 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 @@ -94,6 +94,11 @@ public PhysicalPlan implement(LogicalPlan plan) { return plan.accept(new DefaultImplementor() { @Override public PhysicalPlan visitFilter(LogicalFilter node, ElasticsearchIndexScan context) { + // For now (without optimizer), only push down filter close to relation + if (!(node.getChild().get(0) instanceof LogicalRelation)) { + return super.visitFilter(node, context); + } + FilterQueryBuilder queryBuilder = new FilterQueryBuilder(new DefaultExpressionSerializer()); 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 e239cbe5c2..52e1851e29 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 @@ -55,6 +55,7 @@ import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; @@ -209,4 +210,26 @@ void shouldDiscardPhysicalFilterIfConditionPushedDown() { assertTrue(((ProjectOperator) plan).getInput() instanceof ElasticsearchIndexScan); } + @Test + void shouldNotPushDownFilterFarFromRelation() { + ReferenceExpression field = ref("name", STRING); + NamedExpression named = named("n", field); + Expression filterExpr = dsl.equal(field, literal("John")); + List groupByExprs = Arrays.asList(ref("age", INTEGER)); + List aggregators = Arrays.asList(new AvgAggregator(groupByExprs, DOUBLE)); + + String indexName = "test"; + ElasticsearchIndex index = new ElasticsearchIndex(client, indexName); + PhysicalPlan plan = index.implement( + filter( + aggregation( + relation(indexName), + aggregators, + groupByExprs + ), + filterExpr)); + + assertTrue(plan instanceof FilterOperator); + } + } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/StatsCommandIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/StatsCommandIT.java index 819f752e22..1faf35aab5 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/StatsCommandIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/StatsCommandIT.java @@ -98,4 +98,32 @@ public void testStatsNested() throws IOException { + "}\n", result); } + + @Test + public void testStatsWhere() throws IOException { + String result = executeQueryToString(String.format( + "source=%s | stats sum(balance) as a by gender | where a > 13000000", TEST_INDEX_ACCOUNT)); + assertEquals( + "{\n" + + " \"schema\": [\n" + + " {\n" + + " \"name\": \"a\",\n" + + " \"type\": \"long\"\n" + + " },\n" + + " {\n" + + " \"name\": \"gender\",\n" + + " \"type\": \"string\"\n" + + " }\n" + + " ],\n" + + " \"total\": 1,\n" + + " \"datarows\": [[\n" + + " 13082527,\n" + + " \"M\"\n" + + " ]],\n" + + " \"size\": 1\n" + + "}\n", + result + ); + } + } From 366f42ccc472a74a09bd0a2fba27413ba6dc4aae Mon Sep 17 00:00:00 2001 From: Dai Date: Thu, 13 Aug 2020 11:36:57 -0700 Subject: [PATCH 10/13] Prepare PR --- .../storage/ElasticsearchIndexScan.java | 12 ++++++------ .../storage/script/filter/FilterQueryBuilder.java | 2 +- .../storage/ElasticsearchIndexScanTest.java | 10 +++++----- .../script/filter/FilterQueryBuilderTest.java | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) 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 5e20e1c26c..b42a15ad9a 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 @@ -99,12 +99,12 @@ public void pushDown(QueryBuilder query) { if (current == null) { source.query(query); } else { - if (isBoolMustQuery(current)) { - ((BoolQueryBuilder) current).must(query); + if (isBoolFilterQuery(current)) { + ((BoolQueryBuilder) current).filter(query); } else { source.query(QueryBuilders.boolQuery() - .must(current) - .must(query)); + .filter(current) + .filter(query)); } } @@ -120,9 +120,9 @@ public void close() { client.cleanup(request); } - private boolean isBoolMustQuery(QueryBuilder current) { + private boolean isBoolFilterQuery(QueryBuilder current) { return (current instanceof BoolQueryBuilder) - && !((BoolQueryBuilder) current).must().isEmpty(); + && !((BoolQueryBuilder) current).filter().isEmpty(); } } diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index aa4623a5e4..79fca1982a 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -81,7 +81,7 @@ public QueryBuilder visitFunction(FunctionExpression func, Object context) { FunctionName name = func.getFunctionName(); switch (name.getFunctionName()) { case "and": - return buildBoolQuery(func, context, BoolQueryBuilder::must); + return buildBoolQuery(func, context, BoolQueryBuilder::filter); case "or": return buildBoolQuery(func, context, BoolQueryBuilder::should); case "not": 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 253abe1130..a1de771832 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 @@ -99,14 +99,14 @@ void pushDownFilters() { .pushDown(QueryBuilders.termQuery("age", 30)) .shouldQuery( QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("name", "John")) - .must(QueryBuilders.termQuery("age", 30))) + .filter(QueryBuilders.termQuery("name", "John")) + .filter(QueryBuilders.termQuery("age", 30))) .pushDown(QueryBuilders.rangeQuery("balance").gte(10000)) .shouldQuery( QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("name", "John")) - .must(QueryBuilders.termQuery("age", 30)) - .must(QueryBuilders.rangeQuery("balance").gte(10000))); + .filter(QueryBuilders.termQuery("name", "John")) + .filter(QueryBuilders.termQuery("age", 30)) + .filter(QueryBuilders.rangeQuery("balance").gte(10000))); } private PushDownAssertion assertThat() { diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index 28d102f435..5a7d93ec6b 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -170,7 +170,7 @@ void should_build_script_query_for_comparison_between_fields() { @Test void should_build_bool_query_for_and_or_expression() { - String[] names = { "must", "should" }; + String[] names = { "filter", "should" }; FunctionExpression expr1 = dsl.equal(ref("name", ES_TEXT_KEYWORD), literal("John")); FunctionExpression expr2 = dsl.equal(ref("age", INTEGER), literal(30)); Expression[] exprs = { From 38e93680dac15e1ad0c0acc29980b5be9f80e20c Mon Sep 17 00:00:00 2001 From: Dai Date: Mon, 17 Aug 2020 10:53:02 -0700 Subject: [PATCH 11/13] Add limitation doc --- docs/user/index.rst | 4 + docs/user/limitations/limitations.rst | 103 ++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 docs/user/limitations/limitations.rst diff --git a/docs/user/index.rst b/docs/user/index.rst index f2c6eb899a..39d087297c 100644 --- a/docs/user/index.rst +++ b/docs/user/index.rst @@ -49,3 +49,7 @@ Open Distro for Elasticsearch SQL enables you to extract insights out of Elastic - `Troubleshooting `_ +* **Limitations** + + - `Limitations `_ + diff --git a/docs/user/limitations/limitations.rst b/docs/user/limitations/limitations.rst new file mode 100644 index 0000000000..85a29c463e --- /dev/null +++ b/docs/user/limitations/limitations.rst @@ -0,0 +1,103 @@ + +=========== +Limitations +=========== + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 2 + +Introduction +============ + +In this doc, the restrictions and limitations of SQL plugin is covered as follows. + + +Limitations on Aggregations +=========================== + +Aggregation over expression is not supported for now. You can only apply aggregation on fields, aggregations can't accept an expression as a parameter. For example, `avg(log(age))` is not supported. + +Here's a link to the Github issue - [Issue #288](https://github.com/opendistro-for-elasticsearch/sql/issues/288). + + +Limitations on Subqueries +========================= + +Subqueries in the FROM clause +----------------------------- + +Subquery in the `FROM` clause in this format: `SELECT outer FROM (SELECT inner)` is supported only when the query is merged into one query. For example, the following query is supported: + +```sql +SELECT t.f, t.d +FROM ( + SELECT FlightNum as f, DestCountry as d + FROM kibana_sample_data_flights + WHERE OriginCountry = 'US') t +``` + +But, if the outer query has `GROUP BY` or `ORDER BY`, then it's not supported. + + +Limitations on JOINs +==================== + +JOIN does not support aggregations on the joined result. The `join` query does not support aggregations on the joined result. +For example, e.g. `SELECT depo.name, avg(empo.age) FROM empo JOIN depo WHERE empo.id == depo.id GROUP BY depo.name` is not supported. + +Here's a link to the Github issue - [Issue 110](https://github.com/opendistro-for-elasticsearch/sql/issues/110). + + +Limitations on Pagination +========================= + +Pagination only supports basic queries for now. The pagination query enables you to get back paginated responses. +Currently, the pagination only supports basic queries. For example, the following query returns the data with cursor id. + +```json +POST _opendistro/_sql/ +{ + "fetch_size" : 5, + "query" : "SELECT OriginCountry, DestCountry FROM kibana_sample_data_flights ORDER BY OriginCountry ASC" +} +``` + +The response in JDBC format with cursor id. + +```json +{ + "schema": [ + { + "name": "OriginCountry", + "type": "keyword" + }, + { + "name": "DestCountry", + "type": "keyword" + } + ], + "cursor": "d:eyJhIjp7fSwicyI6IkRYRjFaWEo1UVc1a1JtVjBZMmdCQUFBQUFBQUFCSllXVTJKVU4yeExiWEJSUkhsNFVrdDVXVEZSYkVKSmR3PT0iLCJjIjpbeyJuYW1lIjoiT3JpZ2luQ291bnRyeSIsInR5cGUiOiJrZXl3b3JkIn0seyJuYW1lIjoiRGVzdENvdW50cnkiLCJ0eXBlIjoia2V5d29yZCJ9XSwiZiI6MSwiaSI6ImtpYmFuYV9zYW1wbGVfZGF0YV9mbGlnaHRzIiwibCI6MTMwNTh9", + "total": 13059, + "datarows": [[ + "AE", + "CN" + ]], + "size": 1, + "status": 200 +} +``` + +The query with `aggregation` and `join` does not support pagination for now. + + +Limitations on Query Optimizations +================================== + +Multi-fields in WHERE Conditions +-------------------------------- + +The filter expressions in ``WHERE`` clause may be pushed down to Elasticsearch DSL queries to avoid large amounts of data retrieved. In this case, for Elasticsearch multi-field (a text field with another keyword field inside), assumption is made that the keyword field name is always "keyword" which is true by default. + From 284433334cb025a5e5b5013838fad3296e0325e9 Mon Sep 17 00:00:00 2001 From: Dai Date: Mon, 17 Aug 2020 11:02:25 -0700 Subject: [PATCH 12/13] Add limitation doc --- docs/user/limitations/limitations.rst | 67 +++++++++++++-------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/docs/user/limitations/limitations.rst b/docs/user/limitations/limitations.rst index 85a29c463e..707339c58c 100644 --- a/docs/user/limitations/limitations.rst +++ b/docs/user/limitations/limitations.rst @@ -9,6 +9,7 @@ Limitations :local: :depth: 2 + Introduction ============ @@ -29,15 +30,13 @@ Limitations on Subqueries Subqueries in the FROM clause ----------------------------- -Subquery in the `FROM` clause in this format: `SELECT outer FROM (SELECT inner)` is supported only when the query is merged into one query. For example, the following query is supported: +Subquery in the `FROM` clause in this format: `SELECT outer FROM (SELECT inner)` is supported only when the query is merged into one query. For example, the following query is supported:: -```sql -SELECT t.f, t.d -FROM ( - SELECT FlightNum as f, DestCountry as d - FROM kibana_sample_data_flights - WHERE OriginCountry = 'US') t -``` + SELECT t.f, t.d + FROM ( + SELECT FlightNum as f, DestCountry as d + FROM kibana_sample_data_flights + WHERE OriginCountry = 'US') t But, if the outer query has `GROUP BY` or `ORDER BY`, then it's not supported. @@ -55,40 +54,36 @@ Limitations on Pagination ========================= Pagination only supports basic queries for now. The pagination query enables you to get back paginated responses. -Currently, the pagination only supports basic queries. For example, the following query returns the data with cursor id. +Currently, the pagination only supports basic queries. For example, the following query returns the data with cursor id:: -```json -POST _opendistro/_sql/ -{ - "fetch_size" : 5, - "query" : "SELECT OriginCountry, DestCountry FROM kibana_sample_data_flights ORDER BY OriginCountry ASC" -} -``` + POST _opendistro/_sql/ + { + "fetch_size" : 5, + "query" : "SELECT OriginCountry, DestCountry FROM kibana_sample_data_flights ORDER BY OriginCountry ASC" + } The response in JDBC format with cursor id. -```json -{ - "schema": [ - { - "name": "OriginCountry", - "type": "keyword" - }, { - "name": "DestCountry", - "type": "keyword" + "schema": [ + { + "name": "OriginCountry", + "type": "keyword" + }, + { + "name": "DestCountry", + "type": "keyword" + } + ], + "cursor": "d:eyJhIjp7fSwicyI6IkRYRjFaWEo1UVc1a1JtVjBZMmdCQUFBQUFBQUFCSllXVTJKVU4yeExiWEJSUkhsNFVrdDVXVEZSYkVKSmR3PT0iLCJjIjpbeyJuYW1lIjoiT3JpZ2luQ291bnRyeSIsInR5cGUiOiJrZXl3b3JkIn0seyJuYW1lIjoiRGVzdENvdW50cnkiLCJ0eXBlIjoia2V5d29yZCJ9XSwiZiI6MSwiaSI6ImtpYmFuYV9zYW1wbGVfZGF0YV9mbGlnaHRzIiwibCI6MTMwNTh9", + "total": 13059, + "datarows": [[ + "AE", + "CN" + ]], + "size": 1, + "status": 200 } - ], - "cursor": "d:eyJhIjp7fSwicyI6IkRYRjFaWEo1UVc1a1JtVjBZMmdCQUFBQUFBQUFCSllXVTJKVU4yeExiWEJSUkhsNFVrdDVXVEZSYkVKSmR3PT0iLCJjIjpbeyJuYW1lIjoiT3JpZ2luQ291bnRyeSIsInR5cGUiOiJrZXl3b3JkIn0seyJuYW1lIjoiRGVzdENvdW50cnkiLCJ0eXBlIjoia2V5d29yZCJ9XSwiZiI6MSwiaSI6ImtpYmFuYV9zYW1wbGVfZGF0YV9mbGlnaHRzIiwibCI6MTMwNTh9", - "total": 13059, - "datarows": [[ - "AE", - "CN" - ]], - "size": 1, - "status": 200 -} -``` The query with `aggregation` and `join` does not support pagination for now. From bdfba87ab5732f561e40c692eb31cd47a82c0943 Mon Sep 17 00:00:00 2001 From: Dai Date: Mon, 17 Aug 2020 11:03:41 -0700 Subject: [PATCH 13/13] Add limitation doc --- docs/user/limitations/limitations.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/limitations/limitations.rst b/docs/user/limitations/limitations.rst index 707339c58c..15e43a750d 100644 --- a/docs/user/limitations/limitations.rst +++ b/docs/user/limitations/limitations.rst @@ -62,7 +62,7 @@ Currently, the pagination only supports basic queries. For example, the followin "query" : "SELECT OriginCountry, DestCountry FROM kibana_sample_data_flights ORDER BY OriginCountry ASC" } -The response in JDBC format with cursor id. +The response in JDBC format with cursor id:: { "schema": [