diff --git a/README.md b/README.md index a1e0ba02ae..354904d1cd 100644 --- a/README.md +++ b/README.md @@ -28,23 +28,7 @@ Please refer to the [SQL Language Reference Manual](./docs/user/index.rst), [Pip ## Experimental -Recently we have been actively improving our query engine primarily for better correctness and extensibility. The new enhanced query engine has been already supporting the new released Piped Processing Language query processing behind the scene. Meanwhile, the integration with SQL language is also under way. To try out the power of the new query engine with SQL, simply run the command to enable it by [plugin setting](https://github.com/opendistro-for-elasticsearch/sql/blob/develop/docs/user/admin/settings.rst#opendistro-sql-engine-new-enabled). In future release, this will be enabled by default and nothing required to do from your side. Please stay tuned for updates on our progress and its new exciting features. - -Here is a documentation list with features only available in this improved SQL query engine. Please follow the instruction above to enable it before trying out example queries in these docs: - -* [Identifiers](./docs/user/general/identifiers.rst): support for identifier names with special characters -* [Data types](./docs/user/general/datatypes.rst): new data types such as date time and interval -* [Expressions](./docs/user/dql/expressions.rst): new expression system that can represent and evaluate complex expressions -* [SQL functions](./docs/user/dql/functions.rst): many more string and date functions added -* [Basic queries](./docs/user/dql/basics.rst) - * Ordering by Aggregate Functions section - * NULLS FIRST/LAST in section Specifying Order for Null -* [Aggregations](./docs/user/dql/aggregations.rst): aggregation over expression and more other features -* [Complex queries](./docs/user/dql/complex.rst) - * Improvement on Subqueries in FROM clause -* [Window functions](./docs/user/dql/window.rst): ranking and aggregate window function support - -To avoid impact on your side, normally you won't see any difference in query response. If you want to check if and why your query falls back to be handled by old SQL engine, please explain your query and check Elasticsearch log for "Request is falling back to old SQL engine due to ...". +Recently we have been actively improving our query engine primarily for better correctness and extensibility. Behind the scene, the new enhanced engine has already supported the new released Piped Processing Language. However, it was experimental and disabled by default for SQL query processing. With most important features and full testing complete, now we're ready to promote it as our default SQL query engine. Please find more details in [An Introduction to the New SQL Query Engine](/docs/dev/NewSQLEngine.md). ## Setup 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 40afa7eb12..0d03ddc536 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 @@ -526,6 +526,10 @@ public FunctionExpression nullif(Expression... expressions) { return function(BuiltinFunctionName.NULLIF, expressions); } + public FunctionExpression iffunction(Expression... expressions) { + return function(BuiltinFunctionName.IF, expressions); + } + public static Expression cases(Expression defaultResult, WhenClause... whenClauses) { return new CaseClause(Arrays.asList(whenClauses), defaultResult); 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 3bf5a64602..6b29c68da1 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 @@ -139,6 +139,7 @@ public enum BuiltinFunctionName { IS_NULL(FunctionName.of("is null")), IS_NOT_NULL(FunctionName.of("is not null")), IFNULL(FunctionName.of("ifnull")), + IF(FunctionName.of("if")), NULLIF(FunctionName.of("nullif")), ISNULL(FunctionName.of("isnull")), diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index 8378bf1ad4..01e9aa99ca 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -15,8 +15,6 @@ package com.amazon.opendistroforelasticsearch.sql.expression.operator.predicate; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; @@ -59,6 +57,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(nullIf()); repository.register(isNull(BuiltinFunctionName.IS_NULL)); repository.register(isNull(BuiltinFunctionName.ISNULL)); + repository.register(ifFunction()); } private static FunctionResolver not() { @@ -100,21 +99,28 @@ private static FunctionResolver isNotNull() { Collectors.toList())); } - private static FunctionResolver ifNull() { - FunctionName functionName = BuiltinFunctionName.IFNULL.getName(); + private static FunctionResolver ifFunction() { + FunctionName functionName = BuiltinFunctionName.IF.getName(); List typeList = ExprCoreType.coreTypes(); List>> functionsOne = typeList.stream().map(v -> - impl((UnaryPredicateOperator::exprIfNull), v, v, v)) + impl((UnaryPredicateOperator::exprIf), v, BOOLEAN, v, v)) .collect(Collectors.toList()); + FunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); + return functionResolver; + } + + private static FunctionResolver ifNull() { + FunctionName functionName = BuiltinFunctionName.IFNULL.getName(); + List typeList = ExprCoreType.coreTypes(); + List>> functionsTwo = typeList.stream().map(v -> - impl((UnaryPredicateOperator::exprIfNull), v, UNKNOWN, v)) + FunctionBuilder>>> functionsOne = typeList.stream().map(v -> + impl((UnaryPredicateOperator::exprIfNull), v, v, v)) .collect(Collectors.toList()); - functionsOne.addAll(functionsTwo); FunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); return functionResolver; } @@ -149,4 +155,8 @@ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { return v1.equals(v2) ? LITERAL_NULL : v1; } + public static ExprValue exprIf(ExprValue v1, ExprValue v2, ExprValue v3) { + return !v1.isNull() && !v1.isMissing() && LITERAL_TRUE.equals(v1) ? v2 : v3; + } + } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java index 6936e2ab50..70cde0d886 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java @@ -80,9 +80,9 @@ private static Stream isNullArguments() { private static Stream ifNullArguments() { ArrayList exprValueArrayList = new ArrayList<>(); exprValueArrayList.add(DSL.literal(123)); - exprValueArrayList.add(DSL.literal("test")); + exprValueArrayList.add(DSL.literal(LITERAL_NULL)); exprValueArrayList.add(DSL.literal(321)); - exprValueArrayList.add(DSL.literal("")); + exprValueArrayList.add(DSL.literal(LITERAL_NULL)); return Lists.cartesianProduct(exprValueArrayList, exprValueArrayList).stream() .map(list -> { @@ -115,12 +115,30 @@ private static Stream nullIfArguments() { }); } + private static Stream ifArguments() { + ArrayList exprValueArrayList = new ArrayList<>(); + exprValueArrayList.add(DSL.literal(LITERAL_TRUE)); + exprValueArrayList.add(DSL.literal(LITERAL_FALSE)); + exprValueArrayList.add(DSL.literal(LITERAL_NULL)); + exprValueArrayList.add(DSL.literal(LITERAL_MISSING)); + + return Lists.cartesianProduct(exprValueArrayList, exprValueArrayList).stream() + .map(list -> { + Expression e1 = list.get(0); + if (e1.valueOf(valueEnv()).value() == LITERAL_TRUE.value()) { + return Arguments.of(e1, DSL.literal("123"), DSL.literal("321"), DSL.literal("123")); + } else { + return Arguments.of(e1, DSL.literal("123"), DSL.literal("321"), DSL.literal("321")); + } + }); + } + private static Stream exprIfNullArguments() { ArrayList exprValues = new ArrayList<>(); exprValues.add(LITERAL_NULL); exprValues.add(LITERAL_MISSING); exprValues.add(ExprValueUtils.integerValue(123)); - exprValues.add(ExprValueUtils.stringValue("test")); + exprValues.add(ExprValueUtils.integerValue(456)); return Lists.cartesianProduct(exprValues, exprValues).stream() .map(list -> { @@ -200,18 +218,24 @@ public void test_ifnull_predicate(Expression v1, Expression v2, Expression expec assertEquals(expected.valueOf(valueEnv()), dsl.ifnull(v1, v2).valueOf(valueEnv())); } - @ParameterizedTest - @MethodSource("exprIfNullArguments") - public void test_exprIfNull_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { - assertEquals(expected.value(), UnaryPredicateOperator.exprIfNull(v1, v2).value()); - } - @ParameterizedTest @MethodSource("nullIfArguments") public void test_nullif_predicate(Expression v1, Expression v2, Expression expected) { assertEquals(expected.valueOf(valueEnv()), dsl.nullif(v1, v2).valueOf(valueEnv())); } + @ParameterizedTest + @MethodSource("ifArguments") + public void test_if_predicate(Expression v1, Expression v2, Expression v3, Expression expected) { + assertEquals(expected.valueOf(valueEnv()), dsl.iffunction(v1, v2, v3).valueOf(valueEnv())); + } + + @ParameterizedTest + @MethodSource("exprIfNullArguments") + public void test_exprIfNull_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { + assertEquals(expected.value(), UnaryPredicateOperator.exprIfNull(v1, v2).value()); + } + @ParameterizedTest @MethodSource("exprNullIfArguments") public void test_exprNullIf_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { diff --git a/docs/dev/NewSQLEngine.md b/docs/dev/NewSQLEngine.md new file mode 100644 index 0000000000..a9034ef39e --- /dev/null +++ b/docs/dev/NewSQLEngine.md @@ -0,0 +1,73 @@ +# An Introduction to the New SQL Query Engine + +--- +## 1.Motivations + +The current SQL query engine provides users the basic query capability for using familiar SQL rather than complex Elasticsearch DSL. Based on NLPchina ES-SQL, many new features have been added additionally, such as semantic analyzer, semi-structured data query support, Hash Join etc. However, as we looked into more advanced SQL features, challenges started emerging especially in terms of correctness and extensibility (see [Attributions](../attributions.md)). After thoughtful consideration, we decided to develop a new query engine to address all the problems met so far. + + +--- +## 2.What's New + +With the architecture and extensibility improved significantly, the following SQL features are able to be introduced in the new query engine: + +* [Identifiers](/docs/user/general/identifiers.rst): Support for identifier names with special characters +* [Data types](/docs/user/general/datatypes.rst): New data types such as date time and interval +* [Expressions](/docs/user/dql/expressions.rst): New expression system that can represent and evaluate complex expressions +* [SQL functions](/docs/user/dql/functions.rst): Many more string and date functions added +* [Basic queries](/docs/user/dql/basics.rst) + * Ordering by Aggregate Functions section + * NULLS FIRST/LAST in section Specifying Order for Null +* [Aggregations](/docs/user/dql/aggregations.rst): + * Aggregation over expression + * Selective aggregation by FILTER function +* [Complex queries](/docs/user/dql/complex.rst) + * Improvement on Subqueries in FROM clause +* [Window functions](/docs/user/dql/window.rst) + * Ranking window functions + * Aggregate window functions + +As for correctness, besides full coverage of unit and integration test, we developed a new comparison test framework to ensure correctness by comparing with other databases. Please find more details in [Testing](./Testing.md). + + +--- +## 3.What're Changed + +### 3.1 Breaking Changes + +Because of implementation changed internally, you can expect Explain output in a different format. For query protocol, there are slightly changes on two fields' value in the default response format: + +* **Schema**: Previously the `name` and `alias` value differed for different queries. For consistency, name is always the original text now and alias is its alias defined in SELECT clause or absent if none. +* **Total**: The `total` field represented how many documents matched in total no matter how many returned (indicated by `size` field). However, this field becomes meaningless because of post processing on DSL response in the new query engine. Thus, for now the total number is always same as size field. + +### 3.2 Limitations + +You can find all the limitations in [Limitations](/docs/user/limitations/limitations.rst). For these unsupported features, the query will be forwarded to the old query engine by fallback mechanism. To avoid impact on your side, normally you won't see any difference in a query response. If you want to check if and why your query falls back to be handled by old SQL engine, please explain your query and check Elasticsearch log for "Request is falling back to old SQL engine due to ...". + +Basically, here is a list of the features common though not supported in the new query engine yet: + +* **Cursor**: request with `fetch_size` parameter +* **JSON response format**: will not be supported anymore in the new engine +* **Nested field query**: including supports for object field or nested field query +* **JOINs**: including all types of join queries +* **Elasticsearch functions**: fulltext search, metric and bucket functions + +### 3.3 What if Something Wrong + +No panic! You can roll back to old query engine easily by a plugin setting change. Simply run the command to disable it by [plugin setting](/docs/user/admin/settings.rst#opendistro-sql-engine-new-enabled). Same as other cluster setting change, no need to restart Elasticsearch and the change will take effect on next incoming query. Later on please report the issue to us. + + +--- +## 4.How it's Implemented + +If you're interested in the new query engine, please find more details in [Develop Guide](../developing.rst), [Architecture](./Architecture.md) and other docs in the dev folder. + + +--- +## 5.What's Next + +As mentioned in section 3.2 Limitations, there are still very popular SQL features unsupported yet in the new query engine yet. In particular, the following items are on our roadmap with high priority: + +1. Object/Nested field queries +2. JOIN support +3. Elasticsearch functions diff --git a/docs/experiment/ppl/functions/condition.rst b/docs/experiment/ppl/functions/condition.rst index e287854dad..3ec153065e 100644 --- a/docs/experiment/ppl/functions/condition.rst +++ b/docs/experiment/ppl/functions/condition.rst @@ -145,3 +145,39 @@ Example:: | False | Quility | Nanette | | True | null | Dale | +----------+------------+-------------+ + +IF +------ + +Description +>>>>>>>>>>> + +Usage: if(condition, expr1, expr2) return expr1 if condition is true, otherwiser return expr2. + +Argument type: all the supported data type, (NOTE : if expr1 and expr2 are different type, you will fail semantic check + +Return type: any + +Example:: + + od> source=accounts | eval result = if(true, firstname, lastname) | fields result, firstname, lastname + fetched rows / total rows = 4/4 + +----------+-------------+------------+ + | result | firstname | lastname | + |----------+-------------+------------| + | Amber | Amber | Duke | + | Hattie | Hattie | Bond | + | Nanette | Nanette | Bates | + | Dale | Dale | Adams | + +----------+-------------+------------+ + + od> source=accounts | eval result = if(false, firstname, lastname) | fields result, firstname, lastname + fetched rows / total rows = 4/4 + +----------+-------------+------------+ + | result | firstname | lastname | + |----------+-------------+------------| + | Duke | Amber | Duke | + | Bond | Hattie | Bond | + | Bates | Nanette | Bates | + | Adams | Dale | Adams | + +----------+-------------+------------+ diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index cf64e5a685..0c0b144b9e 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -518,7 +518,7 @@ Description We are migrating existing functionalities to a new query engine under development. User can choose to enable the new engine if interested or disable if any issue found. -1. The default value is false. +1. The default value is true. 2. This setting is node scope. 3. This setting can be updated dynamically. @@ -532,7 +532,7 @@ SQL query:: >> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_opendistro/_sql/settings -d '{ "transient" : { - "opendistro.sql.engine.new.enabled" : "true" + "opendistro.sql.engine.new.enabled" : "false" } }' @@ -546,7 +546,7 @@ Result set:: "sql" : { "engine" : { "new" : { - "enabled" : "true" + "enabled" : "false" } } } diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 3ab134a1ae..e07e354b97 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -1982,6 +1982,40 @@ Example:: | True | False | +---------------+---------------+ +IF +------ + +Description +>>>>>>>>>>> + +Specifications: + +1. IF(condition, ES_TYPE1, ES_TYPE2) -> ES_TYPE1 or ES_TYPE2 + +Usage: if first parameter is true, return second parameter, otherwise return third one. + +Argument type: condition as BOOLEAN, second and third can by any type + +Return type: Any (NOTE : if parameters #2 and #3 has different type, you will fail semantic check" + +Example:: + + od> SELECT IF(100 > 200, '100', '200') + fetched rows / total rows = 1/1 + +-------------------------------+ + | IF(100 > 200, '100', '200') | + |-------------------------------| + | 200 | + +-------------------------------+ + + od> SELECT IF(200 > 100, '100', '200') + fetched rows / total rows = 1/1 + +-------------------------------+ + | IF(200 > 100, '100', '200') | + |-------------------------------| + | 100 | + +-------------------------------+ + CASE ---- diff --git a/doctest/test_docs.py b/doctest/test_docs.py index d73554b615..a0f46d3658 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -203,7 +203,4 @@ def load_tests(loader, suite, ignore): # randomize order of tests to make sure they don't depend on each other random.shuffle(tests) - # prepend a temporary doc to enable new engine so new SQL docs followed can pass - tests.insert(0, doc_suite('../docs/user/dql/newsql.rst')) - return DocTests(tests) diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 9c073e7bbc..6bf88ebbc4 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -81,6 +81,9 @@ integTest { systemProperty "user", System.getProperty("user") systemProperty "password", System.getProperty("password") + // Enable new SQL engine + systemProperty 'enableNewEngine', 'false' + // Set default query size limit systemProperty 'defaultQuerySizeLimit', '10000' @@ -109,9 +112,6 @@ task integTestWithNewEngine(type: RestIntegTestTask) { systemProperty "user", System.getProperty("user") systemProperty "password", System.getProperty("password") - // Enable new SQL engine - systemProperty 'enableNewEngine', 'true' - // Set default query size limit systemProperty 'defaultQuerySizeLimit', '10000' diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/OrdinalAliasRewriterIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/OrdinalAliasRewriterIT.java index 3beb740a7e..286798b3d1 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/OrdinalAliasRewriterIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/OrdinalAliasRewriterIT.java @@ -21,7 +21,6 @@ import com.amazon.opendistroforelasticsearch.sql.legacy.utils.StringUtils; import java.io.IOException; import org.junit.Assume; -import org.junit.Ignore; import org.junit.Test; public class OrdinalAliasRewriterIT extends SQLIntegTestCase { 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 705d75d13a..fc9088a24a 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 @@ -714,12 +714,13 @@ public void right() throws IOException { @Test public void ifFuncShouldPassJDBC() { + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT IF(age > 30, 'True', 'False') AS Ages FROM " + TEST_INDEX_ACCOUNT + " WHERE age IS NOT NULL GROUP BY Ages"); - assertEquals("Ages", response.query("/schema/0/name")); + assertEquals("IF(age > 30, \'True\', \'False\')", response.query("/schema/0/name")); assertEquals("Ages", response.query("/schema/0/alias")); - assertEquals("double", response.query("/schema/0/type")); + assertEquals("keyword", response.query("/schema/0/type")); } @Test diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLIntegTestCase.java index 9f59d93da2..0732193b64 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLIntegTestCase.java @@ -87,7 +87,7 @@ public void setUpIndices() throws Exception { initClient(); } - enableNewQueryEngine(); + configureNewQueryEngine(); resetQuerySizeLimit(); init(); } @@ -147,15 +147,15 @@ public static void cleanUpIndices() throws IOException { wipeAllClusterSettings(); } - private void enableNewQueryEngine() throws IOException { + private void configureNewQueryEngine() throws IOException { boolean isEnabled = isNewQueryEngineEabled(); - if (isEnabled) { - com.amazon.opendistroforelasticsearch.sql.util.TestUtils.enableNewQueryEngine(client()); + if (!isEnabled) { + com.amazon.opendistroforelasticsearch.sql.util.TestUtils.disableNewQueryEngine(client()); } } protected boolean isNewQueryEngineEabled() { - return Boolean.parseBoolean(System.getProperty("enableNewEngine", "false")); + return Boolean.parseBoolean(System.getProperty("enableNewEngine", "true")); } protected void setQuerySizeLimit(Integer limit) throws IOException { diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/AdminIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/AdminIT.java index 2b7101c2c7..0a21506621 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/AdminIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/AdminIT.java @@ -23,7 +23,6 @@ import com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils; import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; import com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import com.google.common.io.Resources; import java.io.IOException; import java.net.URI; @@ -39,7 +38,6 @@ public class AdminIT extends SQLIntegTestCase { @Override public void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); loadIndex(Index.ACCOUNT); } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java index 9c0648990a..e6ae7606c5 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java @@ -21,13 +21,16 @@ import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES; -import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.*; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.hitAny; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.kvInt; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.rows; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.schema; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifyDataRows; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifySchema; import static org.hamcrest.Matchers.equalTo; -import java.io.IOException; - import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; +import java.io.IOException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; @@ -36,8 +39,6 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.search.SearchHits; import org.json.JSONObject; - -import org.junit.Assume; import org.junit.Test; public class ConditionalIT extends SQLIntegTestCase { @@ -45,7 +46,6 @@ public class ConditionalIT extends SQLIntegTestCase { @Override public void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); loadIndex(Index.ACCOUNT); loadIndex(Index.BANK_WITH_NULL_VALUES); } @@ -62,17 +62,17 @@ public void ifnullShouldPassJDBC() throws IOException { @Test public void ifnullWithNullInputTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( - "SELECT IFNULL(1/0, firstname) as IFNULL1 ," - + " IFNULL(firstname, 1/0) as IFNULL2 ," - + " IFNULL(1/0, 1/0) as IFNULL3 " + "SELECT IFNULL(null, firstname) as IFNULL1 ," + + " IFNULL(firstname, null) as IFNULL2 ," + + " IFNULL(null, null) as IFNULL3 " + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + " WHERE balance is null limit 2", "jdbc")); + verifySchema(response, - schema("IFNULL(1/0, firstname)", "IFNULL1", "keyword"), - schema("IFNULL(firstname, 1/0)", "IFNULL2", "integer"), - schema("IFNULL(1/0, 1/0)", "IFNULL3", "integer")); + schema("IFNULL(null, firstname)", "IFNULL1", "keyword"), + schema("IFNULL(firstname, null)", "IFNULL2", "keyword"), + schema("IFNULL(null, null)", "IFNULL3", "byte")); verifyDataRows(response, rows("Hattie", "Hattie", LITERAL_NULL.value()), rows( "Elinor", "Elinor", LITERAL_NULL.value()) @@ -81,26 +81,25 @@ public void ifnullWithNullInputTest() { @Test public void ifnullWithMissingInputTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( - "SELECT IFNULL(balance, firstname) as IFNULL1 ," - + " IFNULL(firstname, balance) as IFNULL2 ," + "SELECT IFNULL(balance, 100) as IFNULL1, " + + " IFNULL(200, balance) as IFNULL2, " + " IFNULL(balance, balance) as IFNULL3 " + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES - + " WHERE balance is null limit 2", "jdbc")); + + " WHERE balance is null limit 3", "jdbc")); verifySchema(response, - schema("IFNULL(balance, firstname)", "IFNULL1", "keyword"), - schema("IFNULL(firstname, balance)", "IFNULL2", "long"), + schema("IFNULL(balance, 100)", "IFNULL1", "long"), + schema("IFNULL(200, balance)", "IFNULL2", "long"), schema("IFNULL(balance, balance)", "IFNULL3", "long")); verifyDataRows(response, - rows("Hattie", "Hattie", LITERAL_NULL.value()), - rows( "Elinor", "Elinor", LITERAL_NULL.value()) + rows(100, 200, null), + rows(100, 200, null), + rows(100, 200, null) ); } @Test public void nullifShouldPassJDBC() throws IOException { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT NULLIF(lastname, 'unknown') AS name FROM " + TEST_INDEX_ACCOUNT); assertEquals("NULLIF(lastname, \'unknown\')", response.query("/schema/0/name")); @@ -110,11 +109,10 @@ public void nullifShouldPassJDBC() throws IOException { @Test public void nullifWithNotNullInputTestOne(){ - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( "SELECT NULLIF(firstname, 'Amber JOHnny') as testnullif " - + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES - + " limit 2 ", "jdbc")); + + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " limit 2 ", "jdbc")); verifySchema(response, schema("NULLIF(firstname, 'Amber JOHnny')", "testnullif", "keyword")); verifyDataRows(response, @@ -125,7 +123,6 @@ public void nullifWithNotNullInputTestOne(){ @Test public void nullifWithNullInputTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( "SELECT NULLIF(1/0, 123) as nullif1 ," + " NULLIF(123, 1/0) as nullif2 ," @@ -144,7 +141,6 @@ public void nullifWithNullInputTest() { @Test public void isnullShouldPassJDBC() throws IOException { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT ISNULL(lastname) AS name FROM " + TEST_INDEX_ACCOUNT); assertEquals("ISNULL(lastname)", response.query("/schema/0/name")); @@ -166,7 +162,6 @@ public void isnullWithNotNullInputTest() throws IOException { @Test public void isnullWithNullInputTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( "SELECT ISNULL(1/0) as ISNULL1 ," + " ISNULL(firstname) as ISNULL2 " @@ -193,6 +188,37 @@ public void isnullWithMathExpr() throws IOException{ ); } + @Test + public void ifShouldPassJDBC() throws IOException { + JSONObject response = executeJdbcRequest( + "SELECT IF(2 > 0, \'hello\', \'world\') AS name FROM " + TEST_INDEX_ACCOUNT); + assertEquals("IF(2 > 0, \'hello\', \'world\')", response.query("/schema/0/name")); + assertEquals("name", response.query("/schema/0/alias")); + assertEquals("keyword", response.query("/schema/0/type")); + } + + @Test + public void ifWithTrueAndFalseCondition() throws IOException { + JSONObject response = new JSONObject(executeQuery( + "SELECT IF(2 < 0, firstname, lastname) as IF0, " + + " IF(2 > 0, firstname, lastname) as IF1, " + + " firstname as IF2, " + + " lastname as IF3 " + + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " limit 2 ", "jdbc" )); + verifySchema(response, + schema("IF(2 < 0, firstname, lastname)", "IF0", "keyword"), + schema("IF(2 > 0, firstname, lastname)", "IF1", "keyword"), + schema("firstname", "IF2", "text"), + schema("lastname", "IF3", "keyword") + ); + verifyDataRows(response, + rows("Duke Willmington", "Amber JOHnny", "Amber JOHnny", "Duke Willmington"), + rows("Bond", "Hattie", "Hattie", "Bond") + ); + + } + private SearchHits query(String query) throws IOException { final String rsp = executeQueryWithStringOutput(query); diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java index e373b3c509..418fecbd35 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java @@ -25,7 +25,6 @@ import com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils; import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import java.io.IOException; import java.util.Locale; import org.elasticsearch.client.Request; @@ -39,7 +38,6 @@ public class DateTimeFunctionIT extends SQLIntegTestCase { @Override public void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); loadIndex(Index.BANK); } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ExpressionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ExpressionIT.java index 754a274f50..f99c0f2c8f 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ExpressionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ExpressionIT.java @@ -21,7 +21,6 @@ import static org.hamcrest.Matchers.is; import com.amazon.opendistroforelasticsearch.sql.legacy.RestIntegTestCase; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import java.io.IOException; import java.util.Locale; import java.util.function.Function; @@ -31,7 +30,6 @@ import org.elasticsearch.client.ResponseException; import org.junit.Ignore; import org.junit.Rule; -import org.junit.Test; import org.junit.rules.ExpectedException; /** @@ -48,7 +46,6 @@ public class ExpressionIT extends RestIntegTestCase { @Override protected void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); } public ResponseExceptionAssertion expectResponseException() { diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MathematicalFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MathematicalFunctionIT.java index e511921003..9f235dfc7a 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MathematicalFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MathematicalFunctionIT.java @@ -24,7 +24,6 @@ import static com.amazon.opendistroforelasticsearch.sql.util.TestUtils.getResponseBody; import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import java.io.IOException; import java.util.Locale; import org.elasticsearch.client.Request; @@ -38,7 +37,6 @@ public class MathematicalFunctionIT extends SQLIntegTestCase { @Override public void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); loadIndex(Index.BANK); } 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 index 473fe4402b..81ddf5549c 100644 --- 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 @@ -20,7 +20,6 @@ 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; @@ -38,7 +37,6 @@ public class MetricsIT extends SQLIntegTestCase { @Override protected void init() throws Exception { loadIndex(Index.BANK); - TestUtils.enableNewQueryEngine(client()); } @Test diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java index e069141d4b..da25c4ed28 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/SQLCorrectnessIT.java @@ -16,7 +16,6 @@ package com.amazon.opendistroforelasticsearch.sql.sql; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import com.google.common.io.Resources; import java.io.IOException; import java.nio.file.Files; @@ -37,7 +36,6 @@ public class SQLCorrectnessIT extends CorrectnessTestBase { @Override protected void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); } @Test 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 a096b5b201..37210d5d09 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 @@ -23,7 +23,6 @@ import static com.amazon.opendistroforelasticsearch.sql.util.TestUtils.getResponseBody; import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; -import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import java.io.IOException; import java.util.Locale; import org.elasticsearch.client.Request; @@ -37,7 +36,6 @@ public class TextFunctionIT extends SQLIntegTestCase { @Override public void init() throws Exception { super.init(); - TestUtils.enableNewQueryEngine(client()); } void verifyQuery(String query, String type, String output) throws IOException { diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/TestUtils.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/TestUtils.java index f68dd74456..b38f5a1efa 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/TestUtils.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/util/TestUtils.java @@ -853,11 +853,11 @@ public static List> getPermutations(final List items) { } /** - * Enable new query engine which is disabled by default for now. + * Disable new query engine which is enabled by default. */ - public static void enableNewQueryEngine(RestClient client) throws IOException { + public static void disableNewQueryEngine(RestClient client) throws IOException { Request request = new Request("PUT", SETTINGS_API_ENDPOINT); - request.setJsonEntity("{\"transient\" : {\"opendistro.sql.engine.new.enabled\" : \"true\"}}"); + request.setJsonEntity("{\"transient\" : {\"opendistro.sql.engine.new.enabled\" : \"false\"}}"); RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder(); restOptionsBuilder.addHeader("Content-Type", "application/json"); diff --git a/integ-test/src/test/resources/correctness/expressions/conditionals.txt b/integ-test/src/test/resources/correctness/expressions/conditionals.txt index 40d7cf4598..76c3562a01 100644 --- a/integ-test/src/test/resources/correctness/expressions/conditionals.txt +++ b/integ-test/src/test/resources/correctness/expressions/conditionals.txt @@ -13,6 +13,6 @@ CASE WHEN 'test' = 'hello world' THEN 'Hello' WHEN 1.0 = 2.0 THEN 'One' ELSE TRI CASE 1 WHEN 1 THEN 'One' ELSE NULL END AS cases CASE 1 WHEN 1 THEN 'One' WHEN 2 THEN 'Two' ELSE NULL END AS cases CASE 2 WHEN 1 THEN NULL WHEN 2 THEN 'Two' ELSE NULL END AS cases -IFNULL(1/0, AvgTicketPrice) from kibana_sample_data_flights -IFNULL(FlightTimeHour, AvgTicketPrice) from kibana_sample_data_flights -IFNULL(AvgTicketPrice, 1/0) from kibana_sample_data_flights \ No newline at end of file +IFNULL(null, AvgTicketPrice) from kibana_sample_data_flights +IFNULL(AvgTicketPrice, 100) from kibana_sample_data_flights +IFNULL(AvgTicketPrice, null) from kibana_sample_data_flights \ No newline at end of file diff --git a/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/SqlSettings.java b/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/SqlSettings.java index 7bb9fe7261..346b4b8423 100644 --- a/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/SqlSettings.java +++ b/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/plugin/SqlSettings.java @@ -57,7 +57,7 @@ public class SqlSettings { public SqlSettings() { Map> settings = new HashMap<>(); settings.put(SQL_ENABLED, Setting.boolSetting(SQL_ENABLED, true, NodeScope, Dynamic)); - settings.put(SQL_NEW_ENGINE_ENABLED, Setting.boolSetting(SQL_NEW_ENGINE_ENABLED, false, NodeScope, Dynamic)); + settings.put(SQL_NEW_ENGINE_ENABLED, Setting.boolSetting(SQL_NEW_ENGINE_ENABLED, true, NodeScope, Dynamic)); settings.put(QUERY_SLOWLOG, Setting.intSetting(QUERY_SLOWLOG, 2, NodeScope, Dynamic)); settings.put(QUERY_RESPONSE_FORMAT, Setting.simpleString(QUERY_RESPONSE_FORMAT, Format.JDBC.getFormatName(), NodeScope, Dynamic)); diff --git a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 index 6db0cb86d9..9866085710 100644 --- a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 @@ -238,6 +238,8 @@ ISNOTNULL: 'ISNOTNULL'; // FLOWCONTROL FUNCTIONS IFNULL: 'IFNULL'; NULLIF: 'NULLIF'; +IF: 'IF'; + // LITERALS AND VALUES //STRING_LITERAL: DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING; diff --git a/ppl/src/main/antlr/OpenDistroPPLParser.g4 b/ppl/src/main/antlr/OpenDistroPPLParser.g4 index 42d80a625a..dafc05c7e2 100644 --- a/ppl/src/main/antlr/OpenDistroPPLParser.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLParser.g4 @@ -254,7 +254,7 @@ dateAndTimeFunctionBase /** condition function return boolean value */ conditionFunctionBase : LIKE - | ISNULL | ISNOTNULL | IFNULL | NULLIF + | IF | ISNULL | ISNOTNULL | IFNULL | NULLIF ; textFunctionBase diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index 6964304bd2..c6d2b15085 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -358,7 +358,7 @@ textFunctionName ; flowControlFunctionName - : IFNULL | NULLIF | ISNULL + : IF | IFNULL | NULLIF | ISNULL ; functionArgs