From 6ae4a4aad41203f24ce1f68e800ec1d54e922099 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Tue, 5 Jan 2021 16:28:18 -0800 Subject: [PATCH 01/13] * Enable sql function ifnull and nullif --- .../sql/expression/DSL.java | 8 ++ .../function/BuiltinFunctionName.java | 3 + .../predicate/UnaryPredicateOperator.java | 74 ++++++++++++- .../predicate/UnaryPredicateOperatorTest.java | 85 +++++++++++++++ .../sql/legacy/SQLFunctionsIT.java | 5 +- .../sql/sql/FlowConrolFunctionIT.java | 100 ++++++++++++++++++ ppl/src/main/antlr/OpenDistroPPLLexer.g4 | 4 + ppl/src/main/antlr/OpenDistroPPLParser.g4 | 2 +- sql/src/main/antlr/OpenDistroSQLParser.g4 | 5 + 9 files changed, 282 insertions(+), 4 deletions(-) create mode 100644 integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java 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 2aba72f407..3de9b3cc76 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 @@ -514,6 +514,14 @@ public FunctionExpression isnotnull(Expression... expressions) { return function(BuiltinFunctionName.IS_NOT_NULL, expressions); } + public FunctionExpression ifnull(Expression... expressions) { + return function(BuiltinFunctionName.IFNULL, expressions); + } + + public FunctionExpression nullif(Expression... expressions) { + return function(BuiltinFunctionName.NULLIF, 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 3c009ea067..0e9b4e4ab1 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 @@ -137,7 +137,10 @@ public enum BuiltinFunctionName { * NULL Test. */ IS_NULL(FunctionName.of("is null")), + ISNULL(FunctionName.of("isnull")), IS_NOT_NULL(FunctionName.of("is not null")), + IFNULL(FunctionName.of("ifnull")), + NULLIF(FunctionName.of("nullif")), ROW_NUMBER(FunctionName.of("row_number")), RANK(FunctionName.of("rank")), 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 18a621acb3..4e8f9bd113 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,16 +15,25 @@ package com.amazon.opendistroforelasticsearch.sql.expression.operator.predicate; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.nullValue; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNKNOWN; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL.impl; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionRepository; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL; +import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionResolver; +import com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils; + import java.util.Arrays; +import java.util.List; import java.util.stream.Collectors; import lombok.experimental.UtilityClass; @@ -41,6 +50,8 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(not()); repository.register(isNull()); repository.register(isNotNull()); + repository.register(nullIf()); + repository.register(ifNull()); } private static FunctionResolver not() { @@ -65,7 +76,6 @@ public ExprValue not(ExprValue v) { } private static FunctionResolver isNull() { - return FunctionDSL .define(BuiltinFunctionName.IS_NULL.getName(), Arrays.stream(ExprCoreType.values()) .map(type -> FunctionDSL @@ -82,4 +92,66 @@ private static FunctionResolver isNotNull() { .collect( Collectors.toList())); } + + private static FunctionResolver ifNull() { + FunctionName functionName = BuiltinFunctionName.IFNULL.getName(); + List typeList = ExprCoreType.coreTypes(); + typeList.add(UNKNOWN); + FunctionResolver functionResolver = + FunctionDSL.define(functionName, + typeList.stream().map(v -> + impl((UnaryPredicateOperator::exprIfNull), v, v, v)) + .collect(Collectors.toList()) + ); + return functionResolver; + } + + private static FunctionResolver nullIf() { + FunctionName functionName = BuiltinFunctionName.NULLIF.getName(); + List typeList = ExprCoreType.coreTypes(); + typeList.add(UNKNOWN); + FunctionResolver functionResolver = + FunctionDSL.define(functionName, + typeList.stream().map(v -> + impl((UnaryPredicateOperator::exprNullIf), v, v, v)) + .collect(Collectors.toList())); + return functionResolver; + } + + /** v2 if v1 is null. + * @param v1 varable 1 + * @param v2 varable 2 + * @return v2 if v1 is null + */ + public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { + if (v1.isNull()) { + return v2; + } else if (v1.isMissing()) { + return v2; + } else { + return v1; + } + } + + /** null if v1 equls to v2. + * @param v1 varable 1 + * @param v2 varable 2 + * @return null if v1 equls to v2 + */ + public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { + if (v1.isNull()) { + return v1; + } else if (v1.isMissing()) { + return v1; + } else if (v2.isNull()) { + return v1; + } else if (v2.isMissing()) { + return v1; + } else if (v1.value().equals(v2.value())) { + return LITERAL_NULL; + } else { + return v1; + } + } + } 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 bfdc097080..2bbe225744 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 @@ -26,10 +26,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; +import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionTestBase; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; +import lombok.val; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -79,4 +82,86 @@ public void is_not_null_predicate() { assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); } + + @Test + public void test_if_null_predicate() { + Expression v1 = dsl.literal(100); + Expression v2 = dsl.literal(200); + + FunctionExpression result = dsl.ifnull(v1, v2); + assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); + + v1 = DSL.literal(ExprNullValue.of()); + result = dsl.ifnull(v1, v2); + assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); + + v1 = dsl.literal(100); + v2 = DSL.literal(ExprNullValue.of()); + result = dsl.ifnull(v1, v2); + assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); + + v1 = DSL.literal(ExprNullValue.of()); + v2 = DSL.literal(ExprNullValue.of()); + result = dsl.ifnull(v1, v2); + assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); + } + + @Test + public void test_null_if_predicate() { + Expression v1 = dsl.literal(100); + Expression v2 = dsl.literal(100); + + FunctionExpression result = dsl.nullif(v1, v2); + + v1 = DSL.literal(ExprNullValue.of()); + v2 = DSL.literal(ExprNullValue.of()); + result = dsl.nullif(v1, v2); + + assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); + + v1 = dsl.literal(100); + v2 = dsl.literal(200); + result = dsl.nullif(v1, v2); + assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); + } + + @Test + public void test_exprIfNull() { + ExprValue result = UnaryPredicateOperator.exprIfNull(LITERAL_NULL, + ExprValueUtils.integerValue(200)); + assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); + + result = UnaryPredicateOperator.exprIfNull(LITERAL_MISSING, + ExprValueUtils.integerValue(200)); + assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); + + result = UnaryPredicateOperator.exprIfNull(LITERAL_NULL, + LITERAL_MISSING); + assertEquals(LITERAL_MISSING.value(), result.value()); + + result = UnaryPredicateOperator.exprIfNull(LITERAL_MISSING, + LITERAL_NULL); + assertEquals(LITERAL_NULL.value(), result.value()); + } + + @Test + public void test_exprNullIf() { + ExprValue result = UnaryPredicateOperator.exprNullIf(LITERAL_NULL, + ExprValueUtils.integerValue(200)); + assertEquals(LITERAL_NULL.value(), result.value()); + + result = UnaryPredicateOperator.exprNullIf(LITERAL_MISSING, + ExprValueUtils.integerValue(200)); + assertEquals(LITERAL_MISSING.value(), result.value()); + + result = UnaryPredicateOperator.exprNullIf(ExprValueUtils.integerValue(200), LITERAL_NULL); + assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); + + result = UnaryPredicateOperator.exprNullIf(ExprValueUtils.integerValue(200), LITERAL_MISSING); + assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); + + result = UnaryPredicateOperator.exprNullIf(ExprValueUtils.integerValue(150), + ExprValueUtils.integerValue(150)); + assertEquals(LITERAL_NULL.value(), result.value()); + } } \ No newline at end of file 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 85c785a5f3..f4244bf809 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 @@ -752,12 +752,13 @@ public void ifFuncWithNullInputAsConditionTest() throws IOException { @Test public void ifnullShouldPassJDBC() throws IOException { + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT IFNULL(lastname, 'unknown') AS name FROM " + TEST_INDEX_ACCOUNT + " GROUP BY name"); - assertEquals("name", response.query("/schema/0/name")); + assertEquals("IFNULL(lastname, \'unknown\')", response.query("/schema/0/name")); assertEquals("name", 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/sql/FlowConrolFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java new file mode 100644 index 0000000000..323e699c69 --- /dev/null +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java @@ -0,0 +1,100 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + * + */ + +package com.amazon.opendistroforelasticsearch.sql.sql; + +import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; +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.kvString; +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 org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.search.SearchHits; +import org.json.JSONObject; + +import org.junit.Test; + + +/** + * Created by allwefantasy on 8/25/16. + */ +public class FlowConrolFunctionIT extends SQLIntegTestCase { + + @Override + public void init() throws Exception { + super.init(); + TestUtils.enableNewQueryEngine(client()); + loadIndex(Index.BANK); + loadIndex(Index.ACCOUNT); + loadIndex(Index.ONLINE); + loadIndex(Index.DATE); + } + + @Test + public void ifnullShouldPassJDBC() throws IOException { + JSONObject response = executeJdbcRequest( + "SELECT IFNULL(lastname, 'unknown') AS name FROM " + TEST_INDEX_ACCOUNT + + " GROUP BY name"); + assertEquals("IFNULL(lastname, \'unknown\')", response.query("/schema/0/name")); + assertEquals("name", response.query("/schema/0/alias")); + assertEquals("keyword", response.query("/schema/0/type")); + } + + @Test + public void ifnullWithNotNullInputTest() throws IOException { + assertThat( + executeQuery("SELECT IFNULL('sample', 'IsNull') AS ifnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvString("/fields/ifnull/0", equalTo("sample"))) + ); + } + + @Test + public void ifnullWithNullInputTest() throws IOException { + assertThat( + executeQuery("SELECT IFNULL(null, 10) AS ifnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvInt("/fields/ifnull/0", equalTo(10))) + ); + assertThat( + executeQuery("SELECT IFNULL('', 10) AS ifnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvString("/fields/ifnull/0", equalTo(""))) + ); + } + + private SearchHits query(String query) throws IOException { + final String rsp = executeQueryWithStringOutput(query); + + final XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + rsp); + return SearchResponse.fromXContent(parser).getHits(); + } + + private JSONObject executeJdbcRequest(String query) { + return new JSONObject(executeQuery(query, "jdbc")); + } +} diff --git a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 index 15b8830a31..c1ce01cf07 100644 --- a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 @@ -235,6 +235,10 @@ LIKE: 'LIKE'; ISNULL: 'ISNULL'; ISNOTNULL: 'ISNOTNULL'; +// FLOWCONTROL FUNCTIONS +IFNULL: 'IFNULL'; +NULLIF: 'NULLIF'; + // LITERALS AND VALUES //STRING_LITERAL: DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING; ID: ID_LITERAL; diff --git a/ppl/src/main/antlr/OpenDistroPPLParser.g4 b/ppl/src/main/antlr/OpenDistroPPLParser.g4 index d724beb0de..71668b57bf 100644 --- a/ppl/src/main/antlr/OpenDistroPPLParser.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLParser.g4 @@ -253,7 +253,7 @@ dateAndTimeFunctionBase /** condition function return boolean value */ conditionFunctionBase : LIKE - | ISNULL | ISNOTNULL + | ISNULL | ISNOTNULL | IFNULL | NULLIF ; textFunctionBase diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index cf4df2f4fe..0622836238 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -291,6 +291,7 @@ scalarFunctionName : mathematicalFunctionName | dateTimeFunctionName | textFunctionName + | flowControlFunctionName ; specificFunction @@ -348,6 +349,10 @@ textFunctionName | CONCAT | CONCAT_WS | SUBSTR | LENGTH | STRCMP | RIGHT ; +flowControlFunctionName + : IFNULL | NULLIF + ; + functionArgs : functionArg (COMMA functionArg)* ; From 2f081dbf3e162c8b00484616b26c47492416f17b Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Wed, 6 Jan 2021 16:10:45 -0800 Subject: [PATCH 02/13] * Enable function isnull() --- .../sql/expression/DSL.java | 4 +++ .../function/BuiltinFunctionName.java | 2 +- .../predicate/UnaryPredicateOperator.java | 14 ++++++-- .../predicate/UnaryPredicateOperatorTest.java | 35 +++++++++++++++++-- .../sql/legacy/SQLFunctionsIT.java | 7 ++-- .../sql/sql/FlowConrolFunctionIT.java | 21 +++++++++++ legacy/src/main/antlr/OpenDistroSqlLexer.g4 | 1 + sql/src/main/antlr/OpenDistroSQLLexer.g4 | 1 + sql/src/main/antlr/OpenDistroSQLParser.g4 | 2 +- 9 files changed, 77 insertions(+), 10 deletions(-) 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 3de9b3cc76..40afa7eb12 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 @@ -507,6 +507,10 @@ private Aggregator aggregate(BuiltinFunctionName functionName, Expression... exp } public FunctionExpression isnull(Expression... expressions) { + return function(BuiltinFunctionName.ISNULL, expressions); + } + + public FunctionExpression is_null(Expression... expressions) { return function(BuiltinFunctionName.IS_NULL, expressions); } 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 0e9b4e4ab1..3bf5a64602 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 @@ -137,10 +137,10 @@ public enum BuiltinFunctionName { * NULL Test. */ IS_NULL(FunctionName.of("is null")), - ISNULL(FunctionName.of("isnull")), IS_NOT_NULL(FunctionName.of("is not null")), IFNULL(FunctionName.of("ifnull")), NULLIF(FunctionName.of("nullif")), + ISNULL(FunctionName.of("isnull")), ROW_NUMBER(FunctionName.of("row_number")), RANK(FunctionName.of("rank")), 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 4e8f9bd113..fa7df6cc4f 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 @@ -50,8 +50,9 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(not()); repository.register(isNull()); repository.register(isNotNull()); - repository.register(nullIf()); repository.register(ifNull()); + repository.register(nullIf()); + repository.register(is_Null()); } private static FunctionResolver not() { @@ -75,7 +76,7 @@ public ExprValue not(ExprValue v) { } } - private static FunctionResolver isNull() { + private static FunctionResolver is_Null() { return FunctionDSL .define(BuiltinFunctionName.IS_NULL.getName(), Arrays.stream(ExprCoreType.values()) .map(type -> FunctionDSL @@ -84,6 +85,15 @@ private static FunctionResolver isNull() { Collectors.toList())); } + private static FunctionResolver isNull() { + return FunctionDSL + .define(BuiltinFunctionName.ISNULL.getName(), Arrays.stream(ExprCoreType.values()) + .map(type -> FunctionDSL + .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) + .collect( + Collectors.toList())); + } + private static FunctionResolver isNotNull() { return FunctionDSL .define(BuiltinFunctionName.IS_NOT_NULL.getName(), Arrays.stream(ExprCoreType.values()) 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 2bbe225744..473125d12c 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 @@ -61,8 +61,30 @@ public void test_not_missing() { assertEquals(LITERAL_MISSING, expression.valueOf(valueEnv())); } + @Test + public void isnull_predicate() { + FunctionExpression expression = dsl.is_null(DSL.literal(1)); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); + + expression = dsl.isnull(DSL.literal(ExprNullValue.of())); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + } + @Test public void is_null_predicate() { + FunctionExpression expression = dsl.is_null(DSL.literal(1)); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); + + expression = dsl.isnull(DSL.literal(ExprNullValue.of())); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + } + + @Test + public void test_isnull_predicate() { FunctionExpression expression = dsl.isnull(DSL.literal(1)); assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); @@ -73,7 +95,7 @@ public void is_null_predicate() { } @Test - public void is_not_null_predicate() { + public void test_is_not_null_predicate() { FunctionExpression expression = dsl.isnotnull(DSL.literal(1)); assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); @@ -109,9 +131,16 @@ public void test_if_null_predicate() { @Test public void test_null_if_predicate() { Expression v1 = dsl.literal(100); - Expression v2 = dsl.literal(100); - + Expression v2 = dsl.literal(200); FunctionExpression result = dsl.nullif(v1, v2); + assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); + + System.out.println("debug : " + result.valueOf(valueEnv())); + + v1 = dsl.literal(100); + v2 = dsl.literal(100); + + result = dsl.nullif(v1, v2); v1 = DSL.literal(ExprNullValue.of()); v2 = DSL.literal(ExprNullValue.of()); 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 f4244bf809..705d75d13a 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 @@ -783,12 +783,13 @@ public void ifnullWithNullInputTest() throws IOException { @Test public void isnullShouldPassJDBC() { + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( - "SELECT ISNULL(lastname) AS name FROM " + TEST_INDEX_ACCOUNT + " GROUP BY name"); - assertEquals("name", response.query("/schema/0/name")); + "SELECT ISNULL(lastname) AS name FROM " + TEST_INDEX_ACCOUNT); + assertEquals("ISNULL(lastname)", response.query("/schema/0/name")); assertEquals("name", response.query("/schema/0/alias")); - assertEquals("integer", response.query("/schema/0/type")); + assertEquals("boolean", response.query("/schema/0/type")); } @Test diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java index 323e699c69..cc62e7db76 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java @@ -36,6 +36,7 @@ import org.elasticsearch.search.SearchHits; import org.json.JSONObject; +import org.junit.Assume; import org.junit.Test; @@ -84,6 +85,26 @@ public void ifnullWithNullInputTest() throws IOException { ); } + @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")); + assertEquals("name", response.query("/schema/0/alias")); + assertEquals("keyword", response.query("/schema/0/type")); + } + + @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")); + assertEquals("name", response.query("/schema/0/alias")); + assertEquals("boolean", response.query("/schema/0/type")); + } + private SearchHits query(String query) throws IOException { final String rsp = executeQueryWithStringOutput(query); diff --git a/legacy/src/main/antlr/OpenDistroSqlLexer.g4 b/legacy/src/main/antlr/OpenDistroSqlLexer.g4 index 772f3f51d1..884659fce8 100644 --- a/legacy/src/main/antlr/OpenDistroSqlLexer.g4 +++ b/legacy/src/main/antlr/OpenDistroSqlLexer.g4 @@ -171,6 +171,7 @@ MONTH: 'MONTH'; MONTHNAME: 'MONTHNAME'; MULTIPLY: 'MULTIPLY'; NOW: 'NOW'; +NULLIF: 'NULLIF'; PI: 'PI'; POW: 'POW'; POWER: 'POWER'; diff --git a/sql/src/main/antlr/OpenDistroSQLLexer.g4 b/sql/src/main/antlr/OpenDistroSQLLexer.g4 index 8f094179d9..17eb7565d2 100644 --- a/sql/src/main/antlr/OpenDistroSQLLexer.g4 +++ b/sql/src/main/antlr/OpenDistroSQLLexer.g4 @@ -208,6 +208,7 @@ MODULUS: 'MODULUS'; MONTHNAME: 'MONTHNAME'; MULTIPLY: 'MULTIPLY'; NOW: 'NOW'; +NULLIF: 'NULLIF'; PI: 'PI'; POW: 'POW'; POWER: 'POWER'; diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index 0622836238..1c95b8d9e6 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -350,7 +350,7 @@ textFunctionName ; flowControlFunctionName - : IFNULL | NULLIF + : IFNULL | NULLIF | ISNULL ; functionArgs From 60d665927b5441428805efae2d3844ac0b552530 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Wed, 6 Jan 2021 21:53:34 -0800 Subject: [PATCH 03/13] * UT and IT --- .../predicate/UnaryPredicateOperator.java | 5 ++++- .../predicate/UnaryPredicateOperatorTest.java | 19 ++++--------------- .../correctness/queries/flowcontrol.txt | 8 ++++++++ 3 files changed, 16 insertions(+), 16 deletions(-) create mode 100644 integ-test/src/test/resources/correctness/queries/flowcontrol.txt 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 fa7df6cc4f..efcca71406 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 @@ -86,8 +86,11 @@ private static FunctionResolver is_Null() { } private static FunctionResolver isNull() { + FunctionName functionName = BuiltinFunctionName.ISNULL.getName(); + List typeList = ExprCoreType.coreTypes(); + typeList.add(UNKNOWN); return FunctionDSL - .define(BuiltinFunctionName.ISNULL.getName(), Arrays.stream(ExprCoreType.values()) + .define(functionName, typeList.stream() .map(type -> FunctionDSL .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) .collect( 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 473125d12c..6e759bfbd9 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 @@ -62,23 +62,12 @@ public void test_not_missing() { } @Test - public void isnull_predicate() { + public void test_is_null_predicate() { FunctionExpression expression = dsl.is_null(DSL.literal(1)); assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); - expression = dsl.isnull(DSL.literal(ExprNullValue.of())); - assertEquals(BOOLEAN, expression.type()); - assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); - } - - @Test - public void is_null_predicate() { - FunctionExpression expression = dsl.is_null(DSL.literal(1)); - assertEquals(BOOLEAN, expression.type()); - assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); - - expression = dsl.isnull(DSL.literal(ExprNullValue.of())); + expression = dsl.is_null(DSL.literal(ExprNullValue.of())); assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); } @@ -106,7 +95,7 @@ public void test_is_not_null_predicate() { } @Test - public void test_if_null_predicate() { + public void test_ifnull_predicate() { Expression v1 = dsl.literal(100); Expression v2 = dsl.literal(200); @@ -129,7 +118,7 @@ public void test_if_null_predicate() { } @Test - public void test_null_if_predicate() { + public void test_nullif_predicate() { Expression v1 = dsl.literal(100); Expression v2 = dsl.literal(200); FunctionExpression result = dsl.nullif(v1, v2); diff --git a/integ-test/src/test/resources/correctness/queries/flowcontrol.txt b/integ-test/src/test/resources/correctness/queries/flowcontrol.txt new file mode 100644 index 0000000000..0932330a57 --- /dev/null +++ b/integ-test/src/test/resources/correctness/queries/flowcontrol.txt @@ -0,0 +1,8 @@ +SELECT IFNULL(AvgTicketPrice, AvgTicketPrice * 2.0) FROM kibana_sample_data_flights +SELECT IFNULL(1/0, AvgTicketPrice) FROM kibana_sample_data_flights +SELECT IFNULL(AvgTicketPrice, 1/0) FROM kibana_sample_data_flights +SELECT NULLIF(AvgTicketPrice, AvgTicketPrice) FROM kibana_sample_data_flights +SELECT NULLIF(AvgTicketPrice, 1/0) FROM kibana_sample_data_flights limit 2 +SELECT ISNULL(AvgTicketPrice) FROM kibana_sample_data_flights +SELECT ISNULL(null) FROM kibana_sample_data_flights +SELECT ISNULL(1/0) FROM kibana_sample_data_flights \ No newline at end of file From 06604597a3c2f8c63aeb0ec38a5b17f2e16cb899 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Thu, 7 Jan 2021 07:41:52 -0800 Subject: [PATCH 04/13] * Update IT&UT, remove debug info --- .../predicate/UnaryPredicateOperator.java | 26 ++++--------- .../predicate/UnaryPredicateOperatorTest.java | 14 +++---- .../sql/correctness/CorrectnessIT.java | 3 +- ...tionIT.java => FlowControlFunctionIT.java} | 37 +++++++++++++++++-- .../correctness/queries/flowcontrol.txt | 4 +- 5 files changed, 49 insertions(+), 35 deletions(-) rename integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/{FlowConrolFunctionIT.java => FlowControlFunctionIT.java} (79%) 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 efcca71406..021a50cb30 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 @@ -90,11 +90,11 @@ private static FunctionResolver isNull() { List typeList = ExprCoreType.coreTypes(); typeList.add(UNKNOWN); return FunctionDSL - .define(functionName, typeList.stream() - .map(type -> FunctionDSL - .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) - .collect( - Collectors.toList())); + .define(functionName, typeList.stream() + .map(type -> FunctionDSL + .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) + .collect( + Collectors.toList())); } private static FunctionResolver isNotNull() { @@ -137,13 +137,7 @@ private static FunctionResolver nullIf() { * @return v2 if v1 is null */ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { - if (v1.isNull()) { - return v2; - } else if (v1.isMissing()) { - return v2; - } else { - return v1; - } + return (v1.isNull() || v1.isMissing()) ? v2 : v1; } /** null if v1 equls to v2. @@ -152,13 +146,7 @@ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { * @return null if v1 equls to v2 */ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { - if (v1.isNull()) { - return v1; - } else if (v1.isMissing()) { - return v1; - } else if (v2.isNull()) { - return v1; - } else if (v2.isMissing()) { + if (v1.isNull() || v1.isMissing() || v2.isNull() || v2.isMissing()) { return v1; } else if (v1.value().equals(v2.value())) { return LITERAL_NULL; 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 6e759bfbd9..4dad07ba94 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 @@ -81,6 +81,10 @@ public void test_isnull_predicate() { expression = dsl.isnull(DSL.literal(ExprNullValue.of())); assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + + expression = dsl.isnull(DSL.literal("test")); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); } @Test @@ -124,23 +128,15 @@ public void test_nullif_predicate() { FunctionExpression result = dsl.nullif(v1, v2); assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); - System.out.println("debug : " + result.valueOf(valueEnv())); - v1 = dsl.literal(100); v2 = dsl.literal(100); - result = dsl.nullif(v1, v2); + assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); v1 = DSL.literal(ExprNullValue.of()); v2 = DSL.literal(ExprNullValue.of()); result = dsl.nullif(v1, v2); - assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); - - v1 = dsl.literal(100); - v2 = dsl.literal(200); - result = dsl.nullif(v1, v2); - assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); } @Test diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java index 8ab9f28e84..649a30ba0d 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java @@ -52,7 +52,8 @@ * Correctness integration test by performing comparison test with other databases. */ @ESIntegTestCase.SuiteScopeTestCase -@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, numDataNodes = 3, supportsDedicatedMasters = false, transportClientRatio = 1) +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, numDataNodes = 3, + supportsDedicatedMasters = false, transportClientRatio = 1) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class CorrectnessIT extends ESIntegTestCase { diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java similarity index 79% rename from integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java rename to integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java index cc62e7db76..a96eb28c72 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowConrolFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java @@ -43,16 +43,13 @@ /** * Created by allwefantasy on 8/25/16. */ -public class FlowConrolFunctionIT extends SQLIntegTestCase { +public class FlowControlFunctionIT extends SQLIntegTestCase { @Override public void init() throws Exception { super.init(); TestUtils.enableNewQueryEngine(client()); - loadIndex(Index.BANK); loadIndex(Index.ACCOUNT); - loadIndex(Index.ONLINE); - loadIndex(Index.DATE); } @Test @@ -105,6 +102,38 @@ public void isnullShouldPassJDBC() throws IOException { assertEquals("boolean", response.query("/schema/0/type")); } + @Test + public void isnullWithNotNullInputTest() throws IOException { + assertThat( + executeQuery("SELECT ISNULL('elastic') AS isnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvInt("/fields/isnull/0", equalTo(0))) + ); + assertThat( + executeQuery("SELECT ISNULL('') AS isnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvInt("/fields/isnull/0", equalTo(0))) + ); + } + + @Test + public void isnullWithNullInputTest() throws IOException { + assertThat( + executeQuery("SELECT ISNULL(null) AS isnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvInt("/fields/isnull/0", equalTo(1))) + ); + } + + @Test + public void isnullWithMathExpr() throws IOException{ + assertThat( + executeQuery("SELECT ISNULL(1+1) AS isnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvInt("/fields/isnull/0", equalTo(0))) + ); + assertThat( + executeQuery("SELECT ISNULL(1+1*1/0) AS isnull FROM " + TEST_INDEX_ACCOUNT), + hitAny(kvInt("/fields/isnull/0", equalTo(1))) + ); + } + private SearchHits query(String query) throws IOException { final String rsp = executeQueryWithStringOutput(query); diff --git a/integ-test/src/test/resources/correctness/queries/flowcontrol.txt b/integ-test/src/test/resources/correctness/queries/flowcontrol.txt index 0932330a57..2e253424c7 100644 --- a/integ-test/src/test/resources/correctness/queries/flowcontrol.txt +++ b/integ-test/src/test/resources/correctness/queries/flowcontrol.txt @@ -2,7 +2,7 @@ SELECT IFNULL(AvgTicketPrice, AvgTicketPrice * 2.0) FROM kibana_sample_data_flig SELECT IFNULL(1/0, AvgTicketPrice) FROM kibana_sample_data_flights SELECT IFNULL(AvgTicketPrice, 1/0) FROM kibana_sample_data_flights SELECT NULLIF(AvgTicketPrice, AvgTicketPrice) FROM kibana_sample_data_flights -SELECT NULLIF(AvgTicketPrice, 1/0) FROM kibana_sample_data_flights limit 2 +SELECT NULLIF(AvgTicketPrice, 1.0/0) FROM kibana_sample_data_flights limit 2 +SELECT NULLIF(1/0, AvgTicketPrice) FROM kibana_sample_data_flights limit 2 SELECT ISNULL(AvgTicketPrice) FROM kibana_sample_data_flights -SELECT ISNULL(null) FROM kibana_sample_data_flights SELECT ISNULL(1/0) FROM kibana_sample_data_flights \ No newline at end of file From f488951c63f48466e83fae598e2597fe0ade544e Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Thu, 7 Jan 2021 18:13:00 -0800 Subject: [PATCH 05/13] * Update IT and UT --- .../predicate/UnaryPredicateOperator.java | 25 +++++++++++++------ .../sql/sql/FlowControlFunctionIT.java | 15 ++++++++--- .../correctness/expressions/conditionals.txt | 6 +++++ .../correctness/queries/flowcontrol.txt | 8 ------ legacy/src/main/antlr/OpenDistroSqlLexer.g4 | 1 - 5 files changed, 34 insertions(+), 21 deletions(-) delete mode 100644 integ-test/src/test/resources/correctness/queries/flowcontrol.txt 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 021a50cb30..0082c8313b 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 @@ -16,7 +16,6 @@ package com.amazon.opendistroforelasticsearch.sql.expression.operator.predicate; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.nullValue; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNKNOWN; import static com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL.impl; @@ -27,10 +26,12 @@ import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionRepository; +import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionBuilder; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionResolver; -import com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils; +import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionSignature; +import com.amazon.opendistroforelasticsearch.sql.expression.function.SerializableFunction; import java.util.Arrays; import java.util.List; @@ -110,12 +111,19 @@ private static FunctionResolver ifNull() { FunctionName functionName = BuiltinFunctionName.IFNULL.getName(); List typeList = ExprCoreType.coreTypes(); typeList.add(UNKNOWN); - FunctionResolver functionResolver = - FunctionDSL.define(functionName, - typeList.stream().map(v -> - impl((UnaryPredicateOperator::exprIfNull), v, v, v)) - .collect(Collectors.toList()) - ); + + List>> functionsOne = typeList.stream().map(v -> + impl((UnaryPredicateOperator::exprIfNull), v, v, v)) + .collect(Collectors.toList()); + + List>> functionsTwo = typeList.stream().map(v -> + impl((UnaryPredicateOperator::exprIfNull), v, UNKNOWN, v)) + .collect(Collectors.toList()); + + functionsOne.addAll(functionsTwo); + FunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); return functionResolver; } @@ -123,6 +131,7 @@ private static FunctionResolver nullIf() { FunctionName functionName = BuiltinFunctionName.NULLIF.getName(); List typeList = ExprCoreType.coreTypes(); typeList.add(UNKNOWN); + FunctionResolver functionResolver = FunctionDSL.define(functionName, typeList.stream().map(v -> diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java index a96eb28c72..01efdeec8d 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java @@ -23,9 +23,11 @@ import static org.hamcrest.Matchers.equalTo; import java.io.IOException; +import java.util.Locale; import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; +import com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants; import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -39,10 +41,6 @@ import org.junit.Assume; import org.junit.Test; - -/** - * Created by allwefantasy on 8/25/16. - */ public class FlowControlFunctionIT extends SQLIntegTestCase { @Override @@ -92,6 +90,15 @@ public void nullifShouldPassJDBC() throws IOException { assertEquals("keyword", response.query("/schema/0/type")); } + @Test + public void nullifWithNotNullInputTest() throws IOException { + System.out.println("TEST_INDEX_ACCOUNT :" + TEST_INDEX_ACCOUNT); + assertThat( + executeQuery("SELECT NULLIF(lastname, lastname) as nullif from " + TEST_INDEX_ACCOUNT), + hitAny(kvString("/fields/nullif/0", equalTo("sample"))) + ); + } + @Test public void isnullShouldPassJDBC() throws IOException { Assume.assumeTrue(isNewQueryEngineEabled()); diff --git a/integ-test/src/test/resources/correctness/expressions/conditionals.txt b/integ-test/src/test/resources/correctness/expressions/conditionals.txt index 18ded14bfa..afdf8178b6 100644 --- a/integ-test/src/test/resources/correctness/expressions/conditionals.txt +++ b/integ-test/src/test/resources/correctness/expressions/conditionals.txt @@ -13,3 +13,9 @@ 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 +NULLIF(AvgTicketPrice, AvgTicketPrice) from kibana_sample_data_flights +NULLIF(AvgTicketPrice, FlightTimeHour) from kibana_sample_data_flights +NULLIF(FlightTimeHour, AvgTicketPrice) from kibana_sample_data_flights diff --git a/integ-test/src/test/resources/correctness/queries/flowcontrol.txt b/integ-test/src/test/resources/correctness/queries/flowcontrol.txt deleted file mode 100644 index 2e253424c7..0000000000 --- a/integ-test/src/test/resources/correctness/queries/flowcontrol.txt +++ /dev/null @@ -1,8 +0,0 @@ -SELECT IFNULL(AvgTicketPrice, AvgTicketPrice * 2.0) FROM kibana_sample_data_flights -SELECT IFNULL(1/0, AvgTicketPrice) FROM kibana_sample_data_flights -SELECT IFNULL(AvgTicketPrice, 1/0) FROM kibana_sample_data_flights -SELECT NULLIF(AvgTicketPrice, AvgTicketPrice) FROM kibana_sample_data_flights -SELECT NULLIF(AvgTicketPrice, 1.0/0) FROM kibana_sample_data_flights limit 2 -SELECT NULLIF(1/0, AvgTicketPrice) FROM kibana_sample_data_flights limit 2 -SELECT ISNULL(AvgTicketPrice) FROM kibana_sample_data_flights -SELECT ISNULL(1/0) FROM kibana_sample_data_flights \ No newline at end of file diff --git a/legacy/src/main/antlr/OpenDistroSqlLexer.g4 b/legacy/src/main/antlr/OpenDistroSqlLexer.g4 index 884659fce8..772f3f51d1 100644 --- a/legacy/src/main/antlr/OpenDistroSqlLexer.g4 +++ b/legacy/src/main/antlr/OpenDistroSqlLexer.g4 @@ -171,7 +171,6 @@ MONTH: 'MONTH'; MONTHNAME: 'MONTHNAME'; MULTIPLY: 'MULTIPLY'; NOW: 'NOW'; -NULLIF: 'NULLIF'; PI: 'PI'; POW: 'POW'; POWER: 'POWER'; From b6cd6d2cab03355b1c922718c002ee1558815820 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Thu, 7 Jan 2021 21:21:07 -0800 Subject: [PATCH 06/13] * Remove a debug message --- .../sql/sql/FlowControlFunctionIT.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java index 01efdeec8d..b54b4587cf 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java @@ -92,10 +92,10 @@ public void nullifShouldPassJDBC() throws IOException { @Test public void nullifWithNotNullInputTest() throws IOException { - System.out.println("TEST_INDEX_ACCOUNT :" + TEST_INDEX_ACCOUNT); + Assume.assumeTrue(isNewQueryEngineEabled()); assertThat( - executeQuery("SELECT NULLIF(lastname, lastname) as nullif from " + TEST_INDEX_ACCOUNT), - hitAny(kvString("/fields/nullif/0", equalTo("sample"))) + executeQuery("SELECT NULLIF(lastname, lastname) as test from " + TEST_INDEX_ACCOUNT), + hitAny(kvString("/fields/test/0", equalTo("sample"))) ); } From 7339d6fb3b8cc5eb680d27e4fb1fd28002b49023 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Fri, 8 Jan 2021 08:22:57 -0800 Subject: [PATCH 07/13] * Move some comparsion test to IT --- .../sql/sql/FlowControlFunctionIT.java | 73 ++++++++++++++++--- .../correctness/expressions/conditionals.txt | 5 +- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java index b54b4587cf..73b8f86159 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java @@ -17,9 +17,8 @@ package com.amazon.opendistroforelasticsearch.sql.sql; import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; -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.kvString; +import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.*; import static org.hamcrest.Matchers.equalTo; import java.io.IOException; @@ -48,6 +47,7 @@ public void init() throws Exception { super.init(); TestUtils.enableNewQueryEngine(client()); loadIndex(Index.ACCOUNT); + loadIndex(Index.BANK_WITH_NULL_VALUES); } @Test @@ -91,14 +91,69 @@ public void nullifShouldPassJDBC() throws IOException { } @Test - public void nullifWithNotNullInputTest() throws IOException { - Assume.assumeTrue(isNewQueryEngineEabled()); - assertThat( - executeQuery("SELECT NULLIF(lastname, lastname) as test from " + TEST_INDEX_ACCOUNT), - hitAny(kvString("/fields/test/0", equalTo("sample"))) - ); + public void nullifWithNotNullInputTest() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(lastname, lastname) as testnullif " + + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); + verifySchema(response, + schema("NULLIF(lastname, lastname)", "testnullif", "keyword")); + } + + @Test + public void nullifWithNullInputTestOne() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(1/0, lastname) as testnullif " + + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); + verifySchema(response, + schema("NULLIF(1/0, lastname)", "testnullif", "unknown")); + } + + @Test + public void nullifWithNullInputTestTwo() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(lastname, 1/0) as testnullif " + + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); + verifySchema(response, + schema("NULLIF(lastname, 1/0)", "testnullif", "unknown")); + } + + @Test + public void nullifWithNullInputTestThree() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(1/0, 1/0) as testnullif " + + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); + verifySchema(response, + schema("NULLIF(1/0, 1/0)", "testnullif", "integer")); + } + + @Test + public void nullifWithMissingInputTestOne() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(balance, balance) as testnullif " + + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES, "jdbc")); + verifySchema(response, + schema("NULLIF(balance, balance)", "testnullif", "long")); } + @Test + public void nullifWithMissingInputTestTwo() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(balance, age) as testnullif " + + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES, "jdbc")); + verifySchema(response, + schema("NULLIF(balance, age)", "testnullif", "long")); + } + + @Test + public void nullifWithMissingInputTestThree() { + JSONObject response = new JSONObject(executeQuery( + "SELECT NULLIF(age, balance) as testnullif " + + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES, "jdbc")); + verifySchema(response, + schema("NULLIF(age, balance)", "testnullif", "long")); + } + + @Test public void isnullShouldPassJDBC() throws IOException { Assume.assumeTrue(isNewQueryEngineEabled()); diff --git a/integ-test/src/test/resources/correctness/expressions/conditionals.txt b/integ-test/src/test/resources/correctness/expressions/conditionals.txt index afdf8178b6..40d7cf4598 100644 --- a/integ-test/src/test/resources/correctness/expressions/conditionals.txt +++ b/integ-test/src/test/resources/correctness/expressions/conditionals.txt @@ -15,7 +15,4 @@ 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 -NULLIF(AvgTicketPrice, AvgTicketPrice) from kibana_sample_data_flights -NULLIF(AvgTicketPrice, FlightTimeHour) from kibana_sample_data_flights -NULLIF(FlightTimeHour, AvgTicketPrice) from kibana_sample_data_flights +IFNULL(AvgTicketPrice, 1/0) from kibana_sample_data_flights \ No newline at end of file From 83cf2df3cedd84e1854875e8c6fa5184184f9581 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Fri, 8 Jan 2021 16:42:52 -0800 Subject: [PATCH 08/13] * Update document --- docs/experiment/ppl/functions/condition.rst | 75 ++++++++++++++++++++ docs/user/dql/functions.rst | 78 +++++++++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/docs/experiment/ppl/functions/condition.rst b/docs/experiment/ppl/functions/condition.rst index c367287df6..436c0272d4 100644 --- a/docs/experiment/ppl/functions/condition.rst +++ b/docs/experiment/ppl/functions/condition.rst @@ -67,3 +67,78 @@ Example, the account 13 doesn't have email field:: | 13 | null | +------------------+---------+ +IFNULL +------ + +Description +>>>>>>>>>>> + +Usage: ifnull(field1, field2) return field2 if field1 is null. + +Argument type: all the supported data type, (NOTE : if two parametershas different type, return result is correct, but return type could be wrong) + +Return type: any + +Example:: + + od> source=accounts | eval result = ifnull(employer, 'default') | fields result, employer, firstname + fetched rows / total rows = 4/4 + +----------+------------+-------------+ + | result | employer | firstname | + |----------+------------+-------------| + | Pyrami | Pyrami | Amber | + | Netagy | Netagy | Hattie | + | Quility | Quility | Nanette | + | default | null | Dale | + +----------+------------+-------------+ + +NULLIF +------ + +Description +>>>>>>>>>>> + +Usage: nullif(field1, field2) return null if two parameters are same, otherwiser return field1. + +Argument type: all the supported data type, (NOTE : if two parametershas different type, return result is correct, but return type could be wrong) + +Return type: any + +Example:: + + od> source=accounts | eval result = nullif(employer, 'Pyrami') | fields result, employer, firstname + fetched rows / total rows = 4/4 + +----------+------------+-------------+ + | result | employer | firstname | + |----------+------------+-------------| + | null | Pyrami | Amber | + | Netagy | Netagy | Hattie | + | Quility | Quility | Nanette | + | null | null | Dale | + +----------+------------+-------------+ + + +ISNULL +------ + +Description +>>>>>>>>>>> + +Usage: nullif(field1, field2) return null if two parameters are same, otherwiser return field1. + +Argument type: all the supported data type + +Return type: any + +Example:: + + od> source=accounts | eval result = isnull(employer) | fields result, employer, firstname + fetched rows / total rows = 4/4 + +----------+------------+-------------+ + | result | employer | firstname | + |----------+------------+-------------| + | False | Pyrami | Amber | + | False | Netagy | Hattie | + | False | Quility | Nanette | + | True | null | Dale | + +----------+------------+-------------+ diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 9233084d4d..4b738ed9dc 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -1892,6 +1892,69 @@ Specifications: 1. IFNULL(ES_TYPE, ES_TYPE) -> ES_TYPE +Usage: return parameter2 if parameter1 is null, otherwise return parameter1 + +Argument type: Any + +Return type: Any (NOTE : if two parameters has different type, return result is correct, but return type could be wrong) + +Example One:: + + od> SELECT IFNULL(123, 321), IFNULL(321, 123) + fetched rows / total rows = 1/1 + +--------------------+--------------------+ + | IFNULL(123, 321) | IFNULL(321, 123) | + |--------------------+--------------------| + | 123 | 321 | + +--------------------+--------------------+ + +Example Two:: + + od> SELECT IFNULL(321, 1/0), IFNULL(1/0, 123) + fetched rows / total rows = 1/1 + +--------------------+--------------------+ + | IFNULL(321, 1/0) | IFNULL(1/0, 123) | + |--------------------+--------------------| + | 321 | 123 | + +--------------------+--------------------+ + +Example Three:: + + od> SELECT IFNULL(1/0, 1/0) + fetched rows / total rows = 1/1 + +--------------------+ + | IFNULL(1/0, 1/0) | + |--------------------| + | null | + +--------------------+ + + +NULLIF +------ + +Description +>>>>>>>>>>> + +Specifications: + +1. NULLIF(ES_TYPE, ES_TYPE) -> ES_TYPE + +Usage: return null if two parameters are same, otherwise return parameer1 + +Argument type: Any + +Return type: Any (NOTE : if two parametershas different type, return result is correct, but return type could be wrong) + +Example:: + + od> SELECT NULLIF(123, 123), NULLIF(321, 123), NULLIF(1/0, 321), NULLIF(321, 1/0), NULLIF(1/0, 1/0) + fetched rows / total rows = 1/1 + +--------------------+--------------------+--------------------+--------------------+--------------------+ + | NULLIF(123, 123) | NULLIF(321, 123) | NULLIF(1/0, 321) | NULLIF(321, 1/0) | NULLIF(1/0, 1/0) | + |--------------------+--------------------+--------------------+--------------------+--------------------| + | null | 321 | null | 321 | null | + +--------------------+--------------------+--------------------+--------------------+--------------------+ + ISNULL ------ @@ -1903,6 +1966,21 @@ Specifications: 1. ISNULL(ES_TYPE) -> INTEGER +Usage: return true if parameter is null, otherwise return false + +Argument type: Any + +Return type: boolean + +Example:: + + od> SELECT ISNULL(1/0), ISNULL(123) + fetched rows / total rows = 1/1 + +---------------+---------------+ + | ISNULL(1/0) | ISNULL(123) | + |---------------+---------------| + | True | False | + +---------------+---------------+ CASE ---- From 12cc6f713eec774fa5b42f6ab6bf929faf261c46 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Fri, 8 Jan 2021 20:03:47 -0800 Subject: [PATCH 09/13] * Update IT test --- .../operator/predicate/UnaryPredicateOperator.java | 8 +------- .../sql/correctness/CorrectnessIT.java | 3 +-- 2 files changed, 2 insertions(+), 9 deletions(-) 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 0082c8313b..fd503ed202 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 @@ -155,13 +155,7 @@ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { * @return null if v1 equls to v2 */ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { - if (v1.isNull() || v1.isMissing() || v2.isNull() || v2.isMissing()) { - return v1; - } else if (v1.value().equals(v2.value())) { - return LITERAL_NULL; - } else { - return v1; - } + return v1.isNull() || v1.equals(v2) ? LITERAL_NULL : v1; } } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java index 649a30ba0d..8ab9f28e84 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/correctness/CorrectnessIT.java @@ -52,8 +52,7 @@ * Correctness integration test by performing comparison test with other databases. */ @ESIntegTestCase.SuiteScopeTestCase -@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, numDataNodes = 3, - supportsDedicatedMasters = false, transportClientRatio = 1) +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, numDataNodes = 3, supportsDedicatedMasters = false, transportClientRatio = 1) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class CorrectnessIT extends ESIntegTestCase { From 588fe006d2196117afa90f2e8db483360952c03b Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Sat, 9 Jan 2021 07:53:26 -0800 Subject: [PATCH 10/13] * Update IT & Fix a bug in isnull --- .../predicate/UnaryPredicateOperator.java | 28 ++- .../predicate/UnaryPredicateOperatorTest.java | 17 ++ .../sql/sql/FlowControlFunctionIT.java | 166 +++++++++++------- 3 files changed, 140 insertions(+), 71 deletions(-) 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 fd503ed202..a8edeef18e 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,9 +15,13 @@ 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_NULL; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; + import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNKNOWN; + import static com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL.impl; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; @@ -90,12 +94,14 @@ private static FunctionResolver isNull() { FunctionName functionName = BuiltinFunctionName.ISNULL.getName(); List typeList = ExprCoreType.coreTypes(); typeList.add(UNKNOWN); - return FunctionDSL - .define(functionName, typeList.stream() - .map(type -> FunctionDSL - .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) - .collect( - Collectors.toList())); + + List>> functionsOne = typeList.stream().map(v -> + impl((UnaryPredicateOperator::exprIsNull), BOOLEAN, v)) + .collect(Collectors.toList()); + + FunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); + return functionResolver; } private static FunctionResolver isNotNull() { @@ -149,7 +155,7 @@ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { return (v1.isNull() || v1.isMissing()) ? v2 : v1; } - /** null if v1 equls to v2. + /** return null if v1 equls to v2. * @param v1 varable 1 * @param v2 varable 2 * @return null if v1 equls to v2 @@ -158,4 +164,12 @@ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { return v1.isNull() || v1.equals(v2) ? LITERAL_NULL : v1; } + /*** return true if input is null or missing. + * @param v varable + * @return + */ + public static ExprValue exprIsNull(ExprValue v) { + return v.isNull() || v.isMissing() ? LITERAL_TRUE : LITERAL_FALSE; + } + } 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 4dad07ba94..b78bbb488a 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 @@ -25,6 +25,7 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.junit.jupiter.api.Assertions.assertEquals; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprMissingValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; @@ -82,6 +83,10 @@ public void test_isnull_predicate() { assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + expression = dsl.isnull(DSL.literal(ExprMissingValue.of())); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + expression = dsl.isnull(DSL.literal("test")); assertEquals(BOOLEAN, expression.type()); assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); @@ -119,6 +124,12 @@ public void test_ifnull_predicate() { v2 = DSL.literal(ExprNullValue.of()); result = dsl.ifnull(v1, v2); assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); + + v1 = DSL.literal(ExprMissingValue.of()); + v2 = v2 = dsl.literal(200); + result = dsl.ifnull(v1, v2); + assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); + } @Test @@ -137,6 +148,12 @@ public void test_nullif_predicate() { v2 = DSL.literal(ExprNullValue.of()); result = dsl.nullif(v1, v2); assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); + + v1 = DSL.literal(ExprMissingValue.of()); + v2 = DSL.literal(ExprMissingValue.of()); + result = dsl.nullif(v1, v2); + assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); + } @Test diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java index 73b8f86159..fe53d545d0 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java @@ -16,6 +16,9 @@ package com.amazon.opendistroforelasticsearch.sql.sql; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; +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.*; @@ -61,22 +64,40 @@ public void ifnullShouldPassJDBC() throws IOException { } @Test - public void ifnullWithNotNullInputTest() throws IOException { - assertThat( - executeQuery("SELECT IFNULL('sample', 'IsNull') AS ifnull FROM " + TEST_INDEX_ACCOUNT), - hitAny(kvString("/fields/ifnull/0", equalTo("sample"))) + 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 " + + " 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")); + verifyDataRows(response, + rows("Hattie", "Hattie", LITERAL_NULL.value()), + rows( "Elinor", "Elinor", LITERAL_NULL.value()) ); } @Test - public void ifnullWithNullInputTest() throws IOException { - assertThat( - executeQuery("SELECT IFNULL(null, 10) AS ifnull FROM " + TEST_INDEX_ACCOUNT), - hitAny(kvInt("/fields/ifnull/0", equalTo(10))) - ); - assertThat( - executeQuery("SELECT IFNULL('', 10) AS ifnull FROM " + TEST_INDEX_ACCOUNT), - hitAny(kvString("/fields/ifnull/0", equalTo(""))) + public void ifnullWithMissingInputTest() { + Assume.assumeTrue(isNewQueryEngineEabled()); + JSONObject response = new JSONObject(executeQuery( + "SELECT IFNULL(balance, firstname) as IFNULL1 ," + + " IFNULL(firstname, balance) as IFNULL2 ," + + " IFNULL(balance, balance) as IFNULL3 " + + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " WHERE balance is null limit 2", "jdbc")); + verifySchema(response, + schema("IFNULL(balance, firstname)", "IFNULL1", "keyword"), + schema("IFNULL(firstname, balance)", "IFNULL2", "long"), + schema("IFNULL(balance, balance)", "IFNULL3", "long")); + verifyDataRows(response, + rows("Hattie", "Hattie", LITERAL_NULL.value()), + rows( "Elinor", "Elinor", LITERAL_NULL.value()) ); } @@ -91,69 +112,59 @@ public void nullifShouldPassJDBC() throws IOException { } @Test - public void nullifWithNotNullInputTest() { - JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(lastname, lastname) as testnullif " - + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); - verifySchema(response, - schema("NULLIF(lastname, lastname)", "testnullif", "keyword")); - } - - @Test - public void nullifWithNullInputTestOne() { - JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(1/0, lastname) as testnullif " - + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); - verifySchema(response, - schema("NULLIF(1/0, lastname)", "testnullif", "unknown")); - } - - @Test - public void nullifWithNullInputTestTwo() { - JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(lastname, 1/0) as testnullif " - + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); - verifySchema(response, - schema("NULLIF(lastname, 1/0)", "testnullif", "unknown")); - } - - @Test - public void nullifWithNullInputTestThree() { + public void nullifWithNotNullInputTestOne(){ + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(1/0, 1/0) as testnullif " - + "FROM " + TEST_INDEX_ACCOUNT, "jdbc")); + "SELECT NULLIF(firstname, 'Amber JOHnny') as testnullif " + + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " limit 2 ", "jdbc")); verifySchema(response, - schema("NULLIF(1/0, 1/0)", "testnullif", "integer")); + schema("NULLIF(firstname, 'Amber JOHnny')", "testnullif", "keyword")); + verifyDataRows(response, + rows(LITERAL_NULL.value()), + rows("Hattie") + ); } @Test - public void nullifWithMissingInputTestOne() { + public void nullifWithNullInputTest() { + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(balance, balance) as testnullif " - + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES, "jdbc")); + "SELECT NULLIF(1/0, firstname) as nullif1 ," + + " NULLIF(firstname, 1/0) as nullif2 ," + + " NULLIF(1/0, 1/0) as nullif3 " + + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " WHERE balance is null limit 2", "jdbc")); verifySchema(response, - schema("NULLIF(balance, balance)", "testnullif", "long")); + schema("NULLIF(1/0, firstname)", "nullif1", "unknown"), + schema("NULLIF(firstname, 1/0)", "nullif2", "unknown"), + schema("NULLIF(1/0, 1/0)", "nullif3", "integer")); + verifyDataRows(response, + rows(LITERAL_NULL.value(), "Hattie", LITERAL_NULL.value()), + rows(LITERAL_NULL.value(), "Elinor", LITERAL_NULL.value()) + ); } @Test - public void nullifWithMissingInputTestTwo() { + public void nullifWithMissingInputTest(){ + Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(balance, age) as testnullif " - + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES, "jdbc")); + "SELECT NULLIF(balance, firstname) as nullif1 ," + + " NULLIF(firstname, balance) as nullif2 ," + + " NULLIF(balance, balance) as nullif3 " + + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " WHERE balance is null limit 2", "jdbc")); verifySchema(response, - schema("NULLIF(balance, age)", "testnullif", "long")); - } + schema("NULLIF(balance, firstname)", "nullif1", "unknown"), + schema("NULLIF(firstname, balance)", "nullif2", "unknown"), + schema("NULLIF(balance, balance)", "nullif3", "long")); + verifyDataRows(response, + rows(LITERAL_NULL.value(), "Hattie", LITERAL_NULL.value()), + rows(LITERAL_NULL.value(), "Elinor", LITERAL_NULL.value()) + ); - @Test - public void nullifWithMissingInputTestThree() { - JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(age, balance) as testnullif " - + "FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES, "jdbc")); - verifySchema(response, - schema("NULLIF(age, balance)", "testnullif", "long")); } - @Test public void isnullShouldPassJDBC() throws IOException { Assume.assumeTrue(isNewQueryEngineEabled()); @@ -177,11 +188,38 @@ public void isnullWithNotNullInputTest() throws IOException { } @Test - public void isnullWithNullInputTest() throws IOException { - assertThat( - executeQuery("SELECT ISNULL(null) AS isnull FROM " + TEST_INDEX_ACCOUNT), - hitAny(kvInt("/fields/isnull/0", equalTo(1))) + public void isnullWithNullInputTest() { + Assume.assumeTrue(isNewQueryEngineEabled()); + JSONObject response = new JSONObject(executeQuery( + "SELECT ISNULL(1/0) as ISNULL1 ," + + " ISNULL(firstname) as ISNULL2 " + + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " WHERE balance is null limit 2", "jdbc")); + verifySchema(response, + schema("ISNULL(1/0)", "ISNULL1", "boolean"), + schema("ISNULL(firstname)", "ISNULL2", "boolean")); + verifyDataRows(response, + rows(LITERAL_TRUE.value(), LITERAL_FALSE.value()), + rows(LITERAL_TRUE.value(), LITERAL_FALSE.value()) + ); + } + + @Test + public void isnullWithMissingInputTest() { + Assume.assumeTrue(isNewQueryEngineEabled()); + JSONObject response = new JSONObject(executeQuery( + "SELECT ISNULL(balance) as ISNULL1 ," + + " ISNULL(firstname) as ISNULL2 " + + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES + + " WHERE balance is null limit 2", "jdbc")); + verifySchema(response, + schema("ISNULL(balance)", "ISNULL1", "boolean"), + schema("ISNULL(firstname)", "ISNULL2", "boolean")); + verifyDataRows(response, + rows(LITERAL_TRUE.value(), LITERAL_FALSE.value()), + rows(LITERAL_TRUE.value(), LITERAL_FALSE.value()) ); + } @Test From c2eede77f26bc93f28b500faee25e955d5b1fef7 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Tue, 12 Jan 2021 13:42:07 -0800 Subject: [PATCH 11/13] * remove UNKNOWN type, consolidate isnull with is_null --- .../predicate/UnaryPredicateOperator.java | 35 +--- .../predicate/UnaryPredicateOperatorTest.java | 173 ++++++++---------- .../sql/sql/FlowControlFunctionIT.java | 55 +----- 3 files changed, 93 insertions(+), 170 deletions(-) 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 a8edeef18e..6edfede281 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 @@ -16,6 +16,7 @@ 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; @@ -53,11 +54,11 @@ public class UnaryPredicateOperator { */ public static void register(BuiltinFunctionRepository repository) { repository.register(not()); - repository.register(isNull()); repository.register(isNotNull()); repository.register(ifNull()); repository.register(nullIf()); - repository.register(is_Null()); + repository.register(is_Null(BuiltinFunctionName.IS_NULL)); + repository.register(is_Null(BuiltinFunctionName.ISNULL)); } private static FunctionResolver not() { @@ -81,29 +82,15 @@ public ExprValue not(ExprValue v) { } } - private static FunctionResolver is_Null() { + private static FunctionResolver is_Null(BuiltinFunctionName funcName) { return FunctionDSL - .define(BuiltinFunctionName.IS_NULL.getName(), Arrays.stream(ExprCoreType.values()) + .define(funcName.getName(), Arrays.stream(ExprCoreType.values()) .map(type -> FunctionDSL .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) .collect( Collectors.toList())); } - private static FunctionResolver isNull() { - FunctionName functionName = BuiltinFunctionName.ISNULL.getName(); - List typeList = ExprCoreType.coreTypes(); - typeList.add(UNKNOWN); - - List>> functionsOne = typeList.stream().map(v -> - impl((UnaryPredicateOperator::exprIsNull), BOOLEAN, v)) - .collect(Collectors.toList()); - - FunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); - return functionResolver; - } - private static FunctionResolver isNotNull() { return FunctionDSL .define(BuiltinFunctionName.IS_NOT_NULL.getName(), Arrays.stream(ExprCoreType.values()) @@ -116,7 +103,6 @@ private static FunctionResolver isNotNull() { private static FunctionResolver ifNull() { FunctionName functionName = BuiltinFunctionName.IFNULL.getName(); List typeList = ExprCoreType.coreTypes(); - typeList.add(UNKNOWN); List>> functionsOne = typeList.stream().map(v -> @@ -136,7 +122,6 @@ private static FunctionResolver ifNull() { private static FunctionResolver nullIf() { FunctionName functionName = BuiltinFunctionName.NULLIF.getName(); List typeList = ExprCoreType.coreTypes(); - typeList.add(UNKNOWN); FunctionResolver functionResolver = FunctionDSL.define(functionName, @@ -161,15 +146,7 @@ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { * @return null if v1 equls to v2 */ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { - return v1.isNull() || v1.equals(v2) ? LITERAL_NULL : v1; - } - - /*** return true if input is null or missing. - * @param v varable - * @return - */ - public static ExprValue exprIsNull(ExprValue v) { - return v.isNull() || v.isMissing() ? LITERAL_TRUE : LITERAL_FALSE; + return v1.equals(v2) ? LITERAL_NULL : v1; } } 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 b78bbb488a..c00ee5d9ad 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 @@ -25,7 +25,6 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.junit.jupiter.api.Assertions.assertEquals; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprMissingValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; @@ -33,7 +32,10 @@ import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionTestBase; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; -import lombok.val; + +import java.lang.reflect.Array; +import java.util.ArrayList; + import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -75,21 +77,21 @@ public void test_is_null_predicate() { @Test public void test_isnull_predicate() { - FunctionExpression expression = dsl.isnull(DSL.literal(1)); - assertEquals(BOOLEAN, expression.type()); - assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); - - expression = dsl.isnull(DSL.literal(ExprNullValue.of())); - assertEquals(BOOLEAN, expression.type()); - assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); - - expression = dsl.isnull(DSL.literal(ExprMissingValue.of())); - assertEquals(BOOLEAN, expression.type()); - assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); - - expression = dsl.isnull(DSL.literal("test")); - assertEquals(BOOLEAN, expression.type()); - assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); + ArrayList exprValueArrayList = new ArrayList<>(); + exprValueArrayList.add(dsl.literal("test")); + exprValueArrayList.add(dsl.literal(100)); + exprValueArrayList.add(dsl.literal("")); + + for (Expression expression : exprValueArrayList) { + FunctionExpression functionExpression = dsl.isnull(expression); + assertEquals(BOOLEAN, functionExpression.type()); + if (expression.valueOf(valueEnv()) == LITERAL_NULL + || expression.valueOf(valueEnv()) == LITERAL_MISSING) { + assertEquals(LITERAL_TRUE, functionExpression.valueOf(valueEnv())); + } else { + assertEquals(LITERAL_FALSE, functionExpression.valueOf(valueEnv())); + } + } } @Test @@ -105,94 +107,79 @@ public void test_is_not_null_predicate() { @Test public void test_ifnull_predicate() { - Expression v1 = dsl.literal(100); - Expression v2 = dsl.literal(200); - - FunctionExpression result = dsl.ifnull(v1, v2); - assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); - - v1 = DSL.literal(ExprNullValue.of()); - result = dsl.ifnull(v1, v2); - assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); - - v1 = dsl.literal(100); - v2 = DSL.literal(ExprNullValue.of()); - result = dsl.ifnull(v1, v2); - assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); - - v1 = DSL.literal(ExprNullValue.of()); - v2 = DSL.literal(ExprNullValue.of()); - result = dsl.ifnull(v1, v2); - assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); - - v1 = DSL.literal(ExprMissingValue.of()); - v2 = v2 = dsl.literal(200); - result = dsl.ifnull(v1, v2); - assertEquals(v2.valueOf(valueEnv()), result.valueOf(valueEnv())); + ArrayList exprValueArrayList = new ArrayList<>(); + exprValueArrayList.add(dsl.literal(100)); + exprValueArrayList.add(dsl.literal(200)); + + for (Expression expressionOne : exprValueArrayList) { + for (Expression expressionTwo : exprValueArrayList) { + FunctionExpression functionExpression = dsl.ifnull(expressionOne, expressionTwo); + if (expressionOne.valueOf(valueEnv()) == LITERAL_NULL + || expressionOne.valueOf(valueEnv()) == LITERAL_MISSING) { + assertEquals(expressionTwo.valueOf(valueEnv()), functionExpression.valueOf(valueEnv())); + } else { + assertEquals(expressionOne.valueOf(valueEnv()), functionExpression.valueOf(valueEnv())); + } + } + } } @Test public void test_nullif_predicate() { - Expression v1 = dsl.literal(100); - Expression v2 = dsl.literal(200); - FunctionExpression result = dsl.nullif(v1, v2); - assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); - - v1 = dsl.literal(100); - v2 = dsl.literal(100); - result = dsl.nullif(v1, v2); - assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); - - v1 = DSL.literal(ExprNullValue.of()); - v2 = DSL.literal(ExprNullValue.of()); - result = dsl.nullif(v1, v2); - assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); - - v1 = DSL.literal(ExprMissingValue.of()); - v2 = DSL.literal(ExprMissingValue.of()); - result = dsl.nullif(v1, v2); - assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); - + ArrayList exprValueArrayList = new ArrayList<>(); + exprValueArrayList.add(DSL.literal(123)); + exprValueArrayList.add(DSL.literal(321)); + + for (Expression v1 : exprValueArrayList) { + for (Expression v2 : exprValueArrayList) { + FunctionExpression result = dsl.nullif(v1, v2); + if (v1.valueOf(valueEnv()) == v2.valueOf(valueEnv())) { + assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); + } else { + assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); + } + } + } } @Test public void test_exprIfNull() { - ExprValue result = UnaryPredicateOperator.exprIfNull(LITERAL_NULL, - ExprValueUtils.integerValue(200)); - assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); - - result = UnaryPredicateOperator.exprIfNull(LITERAL_MISSING, - ExprValueUtils.integerValue(200)); - assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); - - result = UnaryPredicateOperator.exprIfNull(LITERAL_NULL, - LITERAL_MISSING); - assertEquals(LITERAL_MISSING.value(), result.value()); - - result = UnaryPredicateOperator.exprIfNull(LITERAL_MISSING, - LITERAL_NULL); - assertEquals(LITERAL_NULL.value(), result.value()); + ArrayList exprValues = new ArrayList<>(); + exprValues.add(LITERAL_NULL); + exprValues.add(LITERAL_MISSING); + exprValues.add(ExprValueUtils.integerValue(123)); + exprValues.add(ExprValueUtils.stringValue("test")); + + for (ExprValue exprValueOne : exprValues) { + for (ExprValue exprValueTwo : exprValues) { + ExprValue result = UnaryPredicateOperator.exprIfNull(exprValueOne, exprValueTwo); + if (exprValueOne.isNull() || exprValueOne.isMissing()) { + assertEquals(exprValueTwo.value(), result.value()); + } else { + assertEquals(exprValueOne.value(), result.value()); + } + } + } } @Test public void test_exprNullIf() { - ExprValue result = UnaryPredicateOperator.exprNullIf(LITERAL_NULL, - ExprValueUtils.integerValue(200)); - assertEquals(LITERAL_NULL.value(), result.value()); - - result = UnaryPredicateOperator.exprNullIf(LITERAL_MISSING, - ExprValueUtils.integerValue(200)); - assertEquals(LITERAL_MISSING.value(), result.value()); - - result = UnaryPredicateOperator.exprNullIf(ExprValueUtils.integerValue(200), LITERAL_NULL); - assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); - - result = UnaryPredicateOperator.exprNullIf(ExprValueUtils.integerValue(200), LITERAL_MISSING); - assertEquals(ExprValueUtils.integerValue(200).value(), result.value()); - - result = UnaryPredicateOperator.exprNullIf(ExprValueUtils.integerValue(150), - ExprValueUtils.integerValue(150)); - assertEquals(LITERAL_NULL.value(), result.value()); + ArrayList exprValues = new ArrayList<>(); + exprValues.add(LITERAL_NULL); + exprValues.add(LITERAL_MISSING); + exprValues.add(ExprValueUtils.integerValue(123)); + exprValues.add(ExprValueUtils.integerValue(456)); + + for (ExprValue exprValueOne : exprValues) { + for (ExprValue exprValueTwo : exprValues) { + ExprValue result = UnaryPredicateOperator.exprNullIf(exprValueOne, exprValueTwo); + if (exprValueOne.equals(exprValueTwo)) { + assertEquals(LITERAL_NULL.value(), result.value()); + } else { + assertEquals(exprValueOne.value(), result.value()); + } + } + } } } \ No newline at end of file diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java index fe53d545d0..9d8b814acd 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java @@ -25,11 +25,8 @@ import static org.hamcrest.Matchers.equalTo; import java.io.IOException; -import java.util.Locale; - import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; -import com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants; import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -130,39 +127,19 @@ public void nullifWithNotNullInputTestOne(){ public void nullifWithNullInputTest() { Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(1/0, firstname) as nullif1 ," - + " NULLIF(firstname, 1/0) as nullif2 ," + "SELECT NULLIF(1/0, 123) as nullif1 ," + + " NULLIF(123, 1/0) as nullif2 ," + " NULLIF(1/0, 1/0) as nullif3 " + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES - + " WHERE balance is null limit 2", "jdbc")); + + " WHERE balance is null limit 1", "jdbc")); verifySchema(response, - schema("NULLIF(1/0, firstname)", "nullif1", "unknown"), - schema("NULLIF(firstname, 1/0)", "nullif2", "unknown"), + schema("NULLIF(1/0, 123)", "nullif1", "integer"), + schema("NULLIF(123, 1/0)", "nullif2", "integer"), schema("NULLIF(1/0, 1/0)", "nullif3", "integer")); verifyDataRows(response, - rows(LITERAL_NULL.value(), "Hattie", LITERAL_NULL.value()), - rows(LITERAL_NULL.value(), "Elinor", LITERAL_NULL.value()) - ); - } - - @Test - public void nullifWithMissingInputTest(){ - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = new JSONObject(executeQuery( - "SELECT NULLIF(balance, firstname) as nullif1 ," - + " NULLIF(firstname, balance) as nullif2 ," - + " NULLIF(balance, balance) as nullif3 " - + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES - + " WHERE balance is null limit 2", "jdbc")); - verifySchema(response, - schema("NULLIF(balance, firstname)", "nullif1", "unknown"), - schema("NULLIF(firstname, balance)", "nullif2", "unknown"), - schema("NULLIF(balance, balance)", "nullif3", "long")); - verifyDataRows(response, - rows(LITERAL_NULL.value(), "Hattie", LITERAL_NULL.value()), - rows(LITERAL_NULL.value(), "Elinor", LITERAL_NULL.value()) + rows(LITERAL_NULL.value(), 123, LITERAL_NULL.value() + ) ); - } @Test @@ -204,24 +181,6 @@ public void isnullWithNullInputTest() { ); } - @Test - public void isnullWithMissingInputTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = new JSONObject(executeQuery( - "SELECT ISNULL(balance) as ISNULL1 ," - + " ISNULL(firstname) as ISNULL2 " - + " FROM " + TEST_INDEX_BANK_WITH_NULL_VALUES - + " WHERE balance is null limit 2", "jdbc")); - verifySchema(response, - schema("ISNULL(balance)", "ISNULL1", "boolean"), - schema("ISNULL(firstname)", "ISNULL2", "boolean")); - verifyDataRows(response, - rows(LITERAL_TRUE.value(), LITERAL_FALSE.value()), - rows(LITERAL_TRUE.value(), LITERAL_FALSE.value()) - ); - - } - @Test public void isnullWithMathExpr() throws IOException{ assertThat( From 87049c14f559e02ac07bac823910fb336e3a24d2 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Wed, 13 Jan 2021 11:54:23 -0800 Subject: [PATCH 12/13] * Update Docs * rename Is_Null back to isNull --- .../predicate/UnaryPredicateOperator.java | 6 ++--- docs/experiment/ppl/functions/condition.rst | 23 +++++++++++-------- docs/user/dql/functions.rst | 4 ++-- ...trolFunctionIT.java => ConditionalIT.java} | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) rename integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/{FlowControlFunctionIT.java => ConditionalIT.java} (99%) 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 6edfede281..2770bfec9f 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 @@ -57,8 +57,8 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(isNotNull()); repository.register(ifNull()); repository.register(nullIf()); - repository.register(is_Null(BuiltinFunctionName.IS_NULL)); - repository.register(is_Null(BuiltinFunctionName.ISNULL)); + repository.register(isNull(BuiltinFunctionName.IS_NULL)); + repository.register(isNull(BuiltinFunctionName.ISNULL)); } private static FunctionResolver not() { @@ -82,7 +82,7 @@ public ExprValue not(ExprValue v) { } } - private static FunctionResolver is_Null(BuiltinFunctionName funcName) { + private static FunctionResolver isNull(BuiltinFunctionName funcName) { return FunctionDSL .define(funcName.getName(), Arrays.stream(ExprCoreType.values()) .map(type -> FunctionDSL diff --git a/docs/experiment/ppl/functions/condition.rst b/docs/experiment/ppl/functions/condition.rst index 436c0272d4..e287854dad 100644 --- a/docs/experiment/ppl/functions/condition.rst +++ b/docs/experiment/ppl/functions/condition.rst @@ -22,13 +22,16 @@ Return type: BOOLEAN Example:: - od> source=accounts | where isnull(employer) | fields account_number, employer - fetched rows / total rows = 1/1 - +------------------+------------+ - | account_number | employer | - |------------------+------------| - | 18 | null | - +------------------+------------+ + od> source=accounts | eval result = isnull(employer) | fields result, employer, firstname + fetched rows / total rows = 4/4 + +----------+------------+-------------+ + | result | employer | firstname | + |----------+------------+-------------| + | False | Pyrami | Amber | + | False | Netagy | Hattie | + | False | Quility | Nanette | + | True | null | Dale | + +----------+------------+-------------+ ISNOTNULL --------- @@ -75,7 +78,7 @@ Description Usage: ifnull(field1, field2) return field2 if field1 is null. -Argument type: all the supported data type, (NOTE : if two parametershas different type, return result is correct, but return type could be wrong) +Argument type: all the supported data type, (NOTE : if two parameters has different type, you will fail semantic check.) Return type: any @@ -100,7 +103,7 @@ Description Usage: nullif(field1, field2) return null if two parameters are same, otherwiser return field1. -Argument type: all the supported data type, (NOTE : if two parametershas different type, return result is correct, but return type could be wrong) +Argument type: all the supported data type, (NOTE : if two parameters has different type, if two parameters has different type, you will fail semantic check) Return type: any @@ -124,7 +127,7 @@ ISNULL Description >>>>>>>>>>> -Usage: nullif(field1, field2) return null if two parameters are same, otherwiser return field1. +Usage: isnull(field1, field2) return null if two parameters are same, otherwise return field1. Argument type: all the supported data type diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 4b738ed9dc..3ab134a1ae 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -1896,7 +1896,7 @@ Usage: return parameter2 if parameter1 is null, otherwise return parameter1 Argument type: Any -Return type: Any (NOTE : if two parameters has different type, return result is correct, but return type could be wrong) +Return type: Any (NOTE : if two parameters has different type, you will fail semantic check" Example One:: @@ -1943,7 +1943,7 @@ Usage: return null if two parameters are same, otherwise return parameer1 Argument type: Any -Return type: Any (NOTE : if two parametershas different type, return result is correct, but return type could be wrong) +Return type: Any (NOTE : if two parametershas different type, you will fail semantic check") Example:: diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java similarity index 99% rename from integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java rename to integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java index 9d8b814acd..9c0648990a 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/FlowControlFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/ConditionalIT.java @@ -40,7 +40,7 @@ import org.junit.Assume; import org.junit.Test; -public class FlowControlFunctionIT extends SQLIntegTestCase { +public class ConditionalIT extends SQLIntegTestCase { @Override public void init() throws Exception { From e2f159b69c587030e7f1e6b09e23aece1c72ae48 Mon Sep 17 00:00:00 2001 From: Harold Wang Date: Wed, 13 Jan 2021 16:25:50 -0800 Subject: [PATCH 13/13] * Use ParameterizedTest for UT --- .../sql/expression/ExpressionTestBase.java | 2 +- .../predicate/UnaryPredicateOperatorTest.java | 214 ++++++++++-------- 2 files changed, 126 insertions(+), 90 deletions(-) diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionTestBase.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionTestBase.java index 1caeba23cc..9a18e00297 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionTestBase.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/ExpressionTestBase.java @@ -63,7 +63,7 @@ public class ExpressionTestBase { protected Environment typeEnv; @Bean - protected Environment valueEnv() { + protected static Environment valueEnv() { return var -> { if (var instanceof ReferenceExpression) { switch (((ReferenceExpression) var).getAttr()) { 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 c00ee5d9ad..6936e2ab50 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 @@ -23,21 +23,29 @@ import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.booleanValue; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; +import static java.lang.Enum.valueOf; import static org.junit.jupiter.api.Assertions.assertEquals; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionTestBase; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; - +import com.google.common.collect.Lists; import java.lang.reflect.Array; import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; class UnaryPredicateOperatorTest extends ExpressionTestBase { @@ -50,6 +58,100 @@ public void test_not(Boolean v) { assertEquals(String.format("not(%s)", v.toString()), not.toString()); } + private static Stream isNullArguments() { + ArrayList expressions = new ArrayList<>(); + expressions.add(DSL.literal("test")); + expressions.add(DSL.literal(100)); + expressions.add(DSL.literal("")); + expressions.add(DSL.literal(LITERAL_NULL)); + + return Lists.cartesianProduct(expressions, expressions).stream() + .map(list -> { + Expression e1 = list.get(0); + if (e1.valueOf(valueEnv()).isNull() + || e1.valueOf(valueEnv()).isMissing()) { + return Arguments.of(e1, DSL.literal(LITERAL_TRUE)); + } else { + return Arguments.of(e1, DSL.literal(LITERAL_FALSE)); + } + }); + } + + private static Stream ifNullArguments() { + ArrayList exprValueArrayList = new ArrayList<>(); + exprValueArrayList.add(DSL.literal(123)); + exprValueArrayList.add(DSL.literal("test")); + exprValueArrayList.add(DSL.literal(321)); + exprValueArrayList.add(DSL.literal("")); + + return Lists.cartesianProduct(exprValueArrayList, exprValueArrayList).stream() + .map(list -> { + Expression e1 = list.get(0); + Expression e2 = list.get(1); + if (e1.valueOf(valueEnv()).value() == LITERAL_NULL.value() + || e1.valueOf(valueEnv()).value() == LITERAL_MISSING) { + return Arguments.of(e1, e2, e2); + } else { + return Arguments.of(e1, e2, e1); + } + }); + } + + private static Stream nullIfArguments() { + ArrayList exprValueArrayList = new ArrayList<>(); + exprValueArrayList.add(DSL.literal(123)); + exprValueArrayList.add(DSL.literal(321)); + + return Lists.cartesianProduct(exprValueArrayList, exprValueArrayList).stream() + .map(list -> { + Expression e1 = list.get(0); + Expression e2 = list.get(1); + + if (e1.equals(e2)) { + return Arguments.of(e1, e2, DSL.literal(LITERAL_NULL)); + } else { + return Arguments.of(e1, e2, e1); + } + }); + } + + 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")); + + return Lists.cartesianProduct(exprValues, exprValues).stream() + .map(list -> { + ExprValue e1 = list.get(0); + ExprValue e2 = list.get(1); + if (e1.isNull() || e1.isMissing()) { + return Arguments.of(e1, e2, e2); + } else { + return Arguments.of(e1, e2, e1); + } + }); + } + + private static Stream exprNullIfArguments() { + ArrayList exprValues = new ArrayList<>(); + exprValues.add(LITERAL_NULL); + exprValues.add(LITERAL_MISSING); + exprValues.add(ExprValueUtils.integerValue(123)); + + return Lists.cartesianProduct(exprValues, exprValues).stream() + .map(list -> { + ExprValue e1 = list.get(0); + ExprValue e2 = list.get(1); + if (e1.equals(e2)) { + return Arguments.of(e1, e2, LITERAL_NULL); + } else { + return Arguments.of(e1, e2, e1); + } + }); + } + @Test public void test_not_null() { FunctionExpression expression = dsl.not(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); @@ -75,25 +177,6 @@ public void test_is_null_predicate() { assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); } - @Test - public void test_isnull_predicate() { - ArrayList exprValueArrayList = new ArrayList<>(); - exprValueArrayList.add(dsl.literal("test")); - exprValueArrayList.add(dsl.literal(100)); - exprValueArrayList.add(dsl.literal("")); - - for (Expression expression : exprValueArrayList) { - FunctionExpression functionExpression = dsl.isnull(expression); - assertEquals(BOOLEAN, functionExpression.type()); - if (expression.valueOf(valueEnv()) == LITERAL_NULL - || expression.valueOf(valueEnv()) == LITERAL_MISSING) { - assertEquals(LITERAL_TRUE, functionExpression.valueOf(valueEnv())); - } else { - assertEquals(LITERAL_FALSE, functionExpression.valueOf(valueEnv())); - } - } - } - @Test public void test_is_not_null_predicate() { FunctionExpression expression = dsl.isnotnull(DSL.literal(1)); @@ -105,81 +188,34 @@ public void test_is_not_null_predicate() { assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); } - @Test - public void test_ifnull_predicate() { - ArrayList exprValueArrayList = new ArrayList<>(); - exprValueArrayList.add(dsl.literal(100)); - exprValueArrayList.add(dsl.literal(200)); - - for (Expression expressionOne : exprValueArrayList) { - for (Expression expressionTwo : exprValueArrayList) { - FunctionExpression functionExpression = dsl.ifnull(expressionOne, expressionTwo); - if (expressionOne.valueOf(valueEnv()) == LITERAL_NULL - || expressionOne.valueOf(valueEnv()) == LITERAL_MISSING) { - assertEquals(expressionTwo.valueOf(valueEnv()), functionExpression.valueOf(valueEnv())); - } else { - assertEquals(expressionOne.valueOf(valueEnv()), functionExpression.valueOf(valueEnv())); - } - } - } - + @ParameterizedTest + @MethodSource("isNullArguments") + public void test_isnull_predicate(Expression v1, Expression expected) { + assertEquals(expected.valueOf(valueEnv()), dsl.isnull(v1).valueOf(valueEnv())); } - @Test - public void test_nullif_predicate() { - ArrayList exprValueArrayList = new ArrayList<>(); - exprValueArrayList.add(DSL.literal(123)); - exprValueArrayList.add(DSL.literal(321)); - - for (Expression v1 : exprValueArrayList) { - for (Expression v2 : exprValueArrayList) { - FunctionExpression result = dsl.nullif(v1, v2); - if (v1.valueOf(valueEnv()) == v2.valueOf(valueEnv())) { - assertEquals(LITERAL_NULL, result.valueOf(valueEnv())); - } else { - assertEquals(v1.valueOf(valueEnv()), result.valueOf(valueEnv())); - } - } - } + @ParameterizedTest + @MethodSource("ifNullArguments") + public void test_ifnull_predicate(Expression v1, Expression v2, Expression expected) { + assertEquals(expected.valueOf(valueEnv()), dsl.ifnull(v1, v2).valueOf(valueEnv())); } - @Test - public void test_exprIfNull() { - ArrayList exprValues = new ArrayList<>(); - exprValues.add(LITERAL_NULL); - exprValues.add(LITERAL_MISSING); - exprValues.add(ExprValueUtils.integerValue(123)); - exprValues.add(ExprValueUtils.stringValue("test")); + @ParameterizedTest + @MethodSource("exprIfNullArguments") + public void test_exprIfNull_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { + assertEquals(expected.value(), UnaryPredicateOperator.exprIfNull(v1, v2).value()); + } - for (ExprValue exprValueOne : exprValues) { - for (ExprValue exprValueTwo : exprValues) { - ExprValue result = UnaryPredicateOperator.exprIfNull(exprValueOne, exprValueTwo); - if (exprValueOne.isNull() || exprValueOne.isMissing()) { - assertEquals(exprValueTwo.value(), result.value()); - } else { - assertEquals(exprValueOne.value(), result.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())); } - @Test - public void test_exprNullIf() { - ArrayList exprValues = new ArrayList<>(); - exprValues.add(LITERAL_NULL); - exprValues.add(LITERAL_MISSING); - exprValues.add(ExprValueUtils.integerValue(123)); - exprValues.add(ExprValueUtils.integerValue(456)); - - for (ExprValue exprValueOne : exprValues) { - for (ExprValue exprValueTwo : exprValues) { - ExprValue result = UnaryPredicateOperator.exprNullIf(exprValueOne, exprValueTwo); - if (exprValueOne.equals(exprValueTwo)) { - assertEquals(LITERAL_NULL.value(), result.value()); - } else { - assertEquals(exprValueOne.value(), result.value()); - } - } - } + @ParameterizedTest + @MethodSource("exprNullIfArguments") + public void test_exprNullIf_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { + assertEquals(expected.value(), UnaryPredicateOperator.exprNullIf(v1, v2).value()); } + } \ No newline at end of file