From 7225f38fea62fba50956ae6e2f5ace2967980fa7 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Tue, 30 Oct 2018 20:10:26 +0200 Subject: [PATCH 1/2] SQL: Introduce Coalesce function Add Coalesce conditional for replacing null values Fix #35060 --- .../xpack/sql/qa/security/RestSqlIT.java | 6 +- .../xpack/sql/qa/cli/ShowTestCase.java | 4 + .../xpack/sql/qa/jdbc/CsvSpecTestCase.java | 2 +- .../xpack/sql/qa/jdbc/JdbcAssert.java | 5 + .../xpack/sql/qa/jdbc/SqlSpecTestCase.java | 1 + .../qa/src/main/resources/command.csv-spec | 1 + .../sql/qa/src/main/resources/docs.csv-spec | 1 + .../sql/qa/src/main/resources/null.csv-spec | 60 ++++++++++++ .../sql/qa/src/main/resources/null.sql-spec | 9 ++ .../sql/qa/src/main/resources/nulls.csv-spec | 25 ----- .../xpack/sql/expression/Expressions.java | 42 +++++---- .../expression/function/FunctionRegistry.java | 24 +++++ .../sql/expression/function/FunctionType.java | 2 + .../function/scalar/Processors.java | 4 +- .../whitelist/InternalSqlScriptUtils.java | 10 +- .../expression/gen/pipeline/MultiPipe.java | 33 +++++++ .../sql/expression/gen/pipeline/Pipe.java | 35 ++++++- .../expression/gen/pipeline/UnaryPipe.java | 2 +- .../predicate/conditional/Coalesce.java | 93 +++++++++++++++++++ .../predicate/conditional/CoalescePipe.java | 38 ++++++++ .../conditional/CoalesceProcessor.java | 81 ++++++++++++++++ .../conditional/ConditionalFunction.java | 23 +++++ .../predicate/logical/NotProcessor.java | 2 +- .../predicate/{ => nulls}/IsNotNull.java | 2 +- .../{ => nulls}/IsNotNullProcessor.java | 4 +- .../xpack/sql/optimizer/Optimizer.java | 48 ++++++++-- .../xpack/sql/parser/ExpressionBuilder.java | 2 +- .../xpack/sql/planner/QueryTranslator.java | 2 +- .../xpack/sql/type/DataTypeConversion.java | 2 +- .../xpack/sql/plugin/sql_whitelist.txt | 5 + .../xpack/sql/optimizer/OptimizerTests.java | 19 +++- 31 files changed, 517 insertions(+), 70 deletions(-) create mode 100644 x-pack/plugin/sql/qa/src/main/resources/null.csv-spec create mode 100644 x-pack/plugin/sql/qa/src/main/resources/null.sql-spec delete mode 100644 x-pack/plugin/sql/qa/src/main/resources/nulls.csv-spec create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/MultiPipe.java create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalescePipe.java create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalesceProcessor.java create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java rename x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/{ => nulls}/IsNotNull.java (96%) rename x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/{ => nulls}/IsNotNullProcessor.java (92%) diff --git a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlIT.java b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlIT.java index bfcb2f95bcb0d..362b3a93211ea 100644 --- a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlIT.java +++ b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlIT.java @@ -13,18 +13,18 @@ import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase; -import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; - import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; +import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; + /** * Integration test for the rest sql action. The one that speaks json directly to a * user rather than to the JDBC driver or CLI. */ public class RestSqlIT extends RestSqlTestCase { - static final boolean SSL_ENABLED = Booleans.parseBoolean(System.getProperty("tests.ssl.enabled")); + static final boolean SSL_ENABLED = Booleans.parseBoolean(System.getProperty("tests.ssl.enabled"), false); static Settings securitySettings() { String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray())); diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/cli/ShowTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/cli/ShowTestCase.java index 42b87d69b1de1..00fb96d01855c 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/cli/ShowTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/cli/ShowTestCase.java @@ -35,6 +35,10 @@ public void testShowFunctions() throws IOException { while (aggregateFunction.matcher(line).matches()) { line = readLine(); } + Pattern conditionalFunction = Pattern.compile("\\s*[A-Z0-9_~]+\\s*\\|\\s*CONDITIONAL\\s*"); + while (conditionalFunction.matcher(line).matches()) { + line = readLine(); + } Pattern scalarFunction = Pattern.compile("\\s*[A-Z0-9_~]+\\s*\\|\\s*SCALAR\\s*"); while (scalarFunction.matcher(line).matches()) { line = readLine(); diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/CsvSpecTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/CsvSpecTestCase.java index 07fcc0681e39a..f20dafff2a793 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/CsvSpecTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/CsvSpecTestCase.java @@ -35,7 +35,7 @@ public static List readScriptSpec() throws Exception { tests.addAll(readScriptSpec("/columns.csv-spec", parser)); tests.addAll(readScriptSpec("/datetime.csv-spec", parser)); tests.addAll(readScriptSpec("/alias.csv-spec", parser)); - tests.addAll(readScriptSpec("/nulls.csv-spec", parser)); + tests.addAll(readScriptSpec("/null.csv-spec", parser)); tests.addAll(readScriptSpec("/nested.csv-spec", parser)); tests.addAll(readScriptSpec("/functions.csv-spec", parser)); tests.addAll(readScriptSpec("/math.csv-spec", parser)); diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java index dc4ab4f2c1c0e..5b0a2e226abe8 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java @@ -114,6 +114,11 @@ public static void assertResultSetMetadata(ResultSet expected, ResultSet actual, if (expectedType == Types.FLOAT && expected instanceof CsvResultSet) { expectedType = Types.REAL; } + // csv doesn't support NULL type so skip type checking + if (actualType == Types.NULL && expected instanceof CsvResultSet) { + expectedType = Types.NULL; + } + // when lenient is used, an int is equivalent to a short, etc... assertEquals("Different column type for column [" + expectedName + "] (" + JDBCType.valueOf(expectedType) + " != " + JDBCType.valueOf(actualType) + ")", expectedType, actualType); diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/SqlSpecTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/SqlSpecTestCase.java index 672d6eef576f5..d5e720cae8614 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/SqlSpecTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/SqlSpecTestCase.java @@ -41,6 +41,7 @@ public static List readScriptSpec() throws Exception { tests.addAll(readScriptSpec("/arithmetic.sql-spec", parser)); tests.addAll(readScriptSpec("/string-functions.sql-spec", parser)); tests.addAll(readScriptSpec("/case-functions.sql-spec", parser)); + tests.addAll(readScriptSpec("/null.sql-spec", parser)); return tests; } diff --git a/x-pack/plugin/sql/qa/src/main/resources/command.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/command.csv-spec index 0514b2a4982bb..d15e849921592 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/command.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/command.csv-spec @@ -19,6 +19,7 @@ SKEWNESS |AGGREGATE STDDEV_POP |AGGREGATE SUM_OF_SQUARES |AGGREGATE VAR_POP |AGGREGATE +COALESCE |CONDITIONAL DAY |SCALAR DAYNAME |SCALAR DAYOFMONTH |SCALAR diff --git a/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec index bba77664ceda8..ccb9498f3590a 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec @@ -196,6 +196,7 @@ SKEWNESS |AGGREGATE STDDEV_POP |AGGREGATE SUM_OF_SQUARES |AGGREGATE VAR_POP |AGGREGATE +COALESCE |CONDITIONAL DAY |SCALAR DAYNAME |SCALAR DAYOFMONTH |SCALAR diff --git a/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec new file mode 100644 index 0000000000000..377b356ed29e7 --- /dev/null +++ b/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec @@ -0,0 +1,60 @@ +// +// Null expressions +// + +dateTimeOverNull +SELECT YEAR(CAST(NULL AS DATE)) d; + +d:i +null +; + +addOfNull +SELECT CAST(NULL AS INT) + CAST(NULL AS FLOAT) AS n; + +n:d +null +; + + +divOfNull +SELECT 5 / CAST(NULL AS FLOAT) + 10 AS n; + +n:d +null +; + +coalesceJustWithNull +SELECT COALESCE(null, null, null) AS c; + +c +null +; + +coalesceWithFirstNullOfString +SELECT COALESCE(null, 'first') AS c; + +c:s +first +; + +coalesceWithFirstNullOfNumber +SELECT COALESCE(null, 123) AS c; + +c:i +123 +; + +coalesceMixed +SELECT COALESCE(null, 123, null, 321) AS c; + +c:i +123 +; + +coalesceScalar +SELECT COALESCE(null, ABS(123) + 1) AS c; + +c:i +124 +; diff --git a/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec b/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec new file mode 100644 index 0000000000000..95c5052f2837a --- /dev/null +++ b/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec @@ -0,0 +1,9 @@ +// +// Null expressions +// + +coalesceField +SELECT COALESCE(null, ABS(emp_no) + 1) AS c FROM test_emp ORDER BY emp_no LIMIT 5; + +coalesceHaving +SELECT COALESCE(languages, 0) c FROM test_emp GROUP BY languages ORDER BY languages; diff --git a/x-pack/plugin/sql/qa/src/main/resources/nulls.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/nulls.csv-spec deleted file mode 100644 index 417fb459ee32e..0000000000000 --- a/x-pack/plugin/sql/qa/src/main/resources/nulls.csv-spec +++ /dev/null @@ -1,25 +0,0 @@ -// -// Null expressions -// - -nullDate -SELECT YEAR(CAST(NULL AS DATE)) AS d; - -d:i -null -; - -nullAdd -SELECT CAST(NULL AS INT) + CAST(NULL AS FLOAT) AS n; - -n:d -null -; - - -nullDiv -SELECT 5 / CAST(NULL AS FLOAT) + 10 AS n; - -n:d -null -; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java index 5a35e00ab9d6c..92f94140fa0f8 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java @@ -18,6 +18,7 @@ import java.util.Locale; import java.util.function.Predicate; +import static java.lang.String.format; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -103,6 +104,10 @@ public static String name(Expression e) { return e instanceof NamedExpression ? ((NamedExpression) e).name() : e.nodeName(); } + public static boolean isNull(Expression e) { + return e.dataType() == DataType.NULL || (e.foldable() && e.fold() == null); + } + public static List names(Collection e) { List names = new ArrayList<>(e.size()); for (Expression ex : e) { @@ -137,6 +142,14 @@ public static Pipe pipe(Expression e) { throw new SqlIllegalArgumentException("Cannot create pipe for {}", e); } + public static List pipe(List expressions) { + List pipes = new ArrayList<>(expressions.size()); + for (Expression e : expressions) { + pipes.add(pipe(e)); + } + return pipes; + } + public static TypeResolution typeMustBeBoolean(Expression e, String operationName, ParamOrdinal paramOrd) { return typeMustBe(e, dt -> dt == DataType.BOOLEAN, operationName, paramOrd, "boolean"); } @@ -161,27 +174,18 @@ public static TypeResolution typeMustBeNumericOrDate(Expression e, String operat return typeMustBe(e, dt -> dt.isNumeric() || dt == DataType.DATE, operationName, paramOrd, "numeric", "date"); } - private static TypeResolution typeMustBe(Expression e, + public static TypeResolution typeMustBe(Expression e, Predicate predicate, String operationName, - ParamOrdinal pOrd, + ParamOrdinal paramOrd, String... acceptedTypes) { - return predicate.test(e.dataType()) || DataTypes.isNull(e.dataType())? TypeResolution.TYPE_RESOLVED : - new TypeResolution(incorrectTypeErrorMessage(e, operationName, pOrd, acceptedTypes)); - - } - - private static String incorrectTypeErrorMessage(Expression e, - String operationName, - ParamOrdinal paramOrd, - String... acceptedTypes) { - return String.format(Locale.ROOT, "[%s]%s argument must be [%s], found value [%s] type [%s]", - operationName, - paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : " " + paramOrd.name().toLowerCase(Locale.ROOT), - Strings.arrayToDelimitedString(acceptedTypes, " or "), - Expressions.name(e), - e.dataType().esType); - } -} + new TypeResolution(format(Locale.ROOT, "[%s]%s argument must be [%s], found value [%s] type [%s]", + operationName, + paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : " " + paramOrd.name().toLowerCase(Locale.ROOT), + Strings.arrayToDelimitedString(acceptedTypes, " or "), + Expressions.name(e), + e.dataType().esType)); + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java index b36789bfbc6be..c11a565f0aa54 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java @@ -82,6 +82,7 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.string.Space; import org.elasticsearch.xpack.sql.expression.function.scalar.string.Substring; import org.elasticsearch.xpack.sql.expression.function.scalar.string.UCase; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Mod; import org.elasticsearch.xpack.sql.parser.ParsingException; import org.elasticsearch.xpack.sql.tree.Location; @@ -142,6 +143,8 @@ private void defineDefaultFunctions() { def(Skewness.class, Skewness::new), def(Kurtosis.class, Kurtosis::new)); // Scalar functions + // conditional + addToMap(def(Coalesce.class, Coalesce::new)); // Date addToMap(def(DayName.class, DayName::new, "DAYNAME"), def(DayOfMonth.class, DayOfMonth::new, "DAYOFMONTH", "DAY", "DOM"), @@ -310,6 +313,26 @@ static FunctionDefinition def(Class function, return def(function, builder, false, aliases); } + /** + * Build a {@linkplain FunctionDefinition} for multi-arg function that + * is not aware of time zone and does not support {@code DISTINCT}. + */ + @SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do + static FunctionDefinition def(Class function, + MultiFunctionBuilder ctorRef, String... aliases) { + FunctionBuilder builder = (location, children, distinct, tz) -> { + if (distinct) { + throw new IllegalArgumentException("does not support DISTINCT yet it was specified"); + } + return ctorRef.build(location, children); + }; + return def(function, builder, false, aliases); + } + + interface MultiFunctionBuilder { + T build(Location location, List children); + } + /** * Build a {@linkplain FunctionDefinition} for a unary function that is not * aware of time zone but does support {@code DISTINCT}. @@ -325,6 +348,7 @@ static FunctionDefinition def(Class function, }; return def(function, builder, false, aliases); } + interface DistinctAwareUnaryFunctionBuilder { T build(Location location, Expression target, boolean distinct); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionType.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionType.java index 5ed81e354fce2..b2f4ab8ef2ca2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionType.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionType.java @@ -8,11 +8,13 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.ConditionalFunction; public enum FunctionType { AGGREGATE(AggregateFunction.class), + CONDITIONAL(ConditionalFunction.class), SCALAR(ScalarFunction.class), SCORE(Score.class); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Processors.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Processors.java index 44970e76911e5..6954abbbaa7dc 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Processors.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Processors.java @@ -25,9 +25,10 @@ import org.elasticsearch.xpack.sql.expression.gen.processor.ConstantProcessor; import org.elasticsearch.xpack.sql.expression.gen.processor.HitExtractorProcessor; import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; -import org.elasticsearch.xpack.sql.expression.predicate.IsNotNullProcessor; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.CoalesceProcessor; import org.elasticsearch.xpack.sql.expression.predicate.logical.BinaryLogicProcessor; import org.elasticsearch.xpack.sql.expression.predicate.logical.NotProcessor; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNullProcessor; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.UnaryArithmeticProcessor; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparisonProcessor; @@ -58,6 +59,7 @@ public static List getNamedWriteables() { entries.add(new Entry(Processor.class, BinaryLogicProcessor.NAME, BinaryLogicProcessor::new)); entries.add(new Entry(Processor.class, NotProcessor.NAME, NotProcessor::new)); // null + entries.add(new Entry(Processor.class, CoalesceProcessor.NAME, CoalesceProcessor::new)); entries.add(new Entry(Processor.class, IsNotNullProcessor.NAME, IsNotNullProcessor::new)); // arithmetic diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java index 1c2ccfeeb29cb..964bbf9441fd6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java @@ -21,9 +21,10 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.string.ReplaceFunctionProcessor; import org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.StringOperation; import org.elasticsearch.xpack.sql.expression.function.scalar.string.SubstringFunctionProcessor; -import org.elasticsearch.xpack.sql.expression.predicate.IsNotNullProcessor; import org.elasticsearch.xpack.sql.expression.predicate.logical.BinaryLogicProcessor.BinaryLogicOperation; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.CoalesceProcessor; import org.elasticsearch.xpack.sql.expression.predicate.logical.NotProcessor; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNullProcessor; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor.BinaryArithmeticOperation; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.UnaryArithmeticProcessor.UnaryArithmeticOperation; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparisonProcessor.BinaryComparisonOperation; @@ -119,6 +120,13 @@ public static Boolean in(Object value, List values) { return InProcessor.apply(value, values); } + // + // Null + // + public static Object coalesce(List expressions) { + return CoalesceProcessor.apply(expressions); + } + // // Regex // diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/MultiPipe.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/MultiPipe.java new file mode 100644 index 0000000000000..d25e7a2e660a2 --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/MultiPipe.java @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.expression.gen.pipeline; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; +import org.elasticsearch.xpack.sql.tree.Location; + +import java.util.ArrayList; +import java.util.List; + +public abstract class MultiPipe extends Pipe { + + protected MultiPipe(Location location, Expression expression, List children) { + super(location, expression, children); + } + + @Override + public Processor asProcessor() { + List procs = new ArrayList<>(); + for (Pipe pipe : children()) { + procs.add(pipe.asProcessor()); + } + + return asProcessor(procs); + } + + public abstract Processor asProcessor(List procs); +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/Pipe.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/Pipe.java index 5c96d2c9244ab..b92cb9a15eae4 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/Pipe.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/Pipe.java @@ -6,13 +6,16 @@ package org.elasticsearch.xpack.sql.expression.gen.pipeline; import org.elasticsearch.xpack.sql.capabilities.Resolvable; +import org.elasticsearch.xpack.sql.capabilities.Resolvables; import org.elasticsearch.xpack.sql.execution.search.FieldExtraction; +import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.tree.Node; +import java.util.ArrayList; import java.util.List; /** @@ -38,6 +41,27 @@ public Expression expression() { return expression; } + @Override + public boolean resolved() { + return Resolvables.resolved(children()); + } + + @Override + public void collectFields(SqlSourceBuilder sourceBuilder) { + children().forEach(c -> c.collectFields(sourceBuilder)); + } + + @Override + public boolean supportedByAggsOnlyQuery() { + for (Pipe pipe : children()) { + if (pipe.supportedByAggsOnlyQuery()) { + return true; + } + } + + return false; + } + public abstract Processor asProcessor(); /** @@ -47,9 +71,16 @@ public Expression expression() { * @return {@code this} if the resolution doesn't change the * definition, a new {@link Pipe} otherwise */ - public abstract Pipe resolveAttributes(AttributeResolver resolver); + public Pipe resolveAttributes(AttributeResolver resolver) { + List newPipes = new ArrayList<>(children().size()); + for (Pipe p : children()) { + newPipes.add(p.resolveAttributes(resolver)); + } + + return children().equals(newPipes) ? this : replaceChildren(newPipes); + } public interface AttributeResolver { FieldExtraction resolve(Attribute attribute); } -} +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/UnaryPipe.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/UnaryPipe.java index 8e36f448929ab..8e2c87dc75cda 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/UnaryPipe.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/UnaryPipe.java @@ -98,4 +98,4 @@ public boolean equals(Object obj) { && Objects.equals(child, other.child) && Objects.equals(expression(), other.expression()); } -} +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java new file mode 100644 index 0000000000000..9f93866d0b70b --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.expression.predicate.conditional; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Expressions; +import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; +import org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder; +import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.NodeInfo; +import org.elasticsearch.xpack.sql.type.DataType; +import org.elasticsearch.xpack.sql.type.DataTypeConversion; + +import java.util.ArrayList; +import java.util.List; +import java.util.StringJoiner; + +import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; + +public class Coalesce extends ConditionalFunction { + + private DataType dataType = DataType.NULL; + + public Coalesce(Location location, List fields) { + super(location, fields); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, Coalesce::new, children()); + } + + @Override + public Expression replaceChildren(List newChildren) { + return new Coalesce(location(), newChildren); + } + + @Override + protected TypeResolution resolveType() { + for (Expression e : children()) { + dataType = DataTypeConversion.commonType(dataType, e.dataType()); + } + return TypeResolution.TYPE_RESOLVED; + } + + @Override + public DataType dataType() { + return dataType; + } + + @Override + public boolean foldable() { + // if the first entry is foldable, so is coalesce + // that's because the nulls are eliminated by the optimizer + // and if the first expression is folded (and not null), the rest do not matter + List children = children(); + return (children.isEmpty() || (children.get(0).foldable() && children.get(0).fold() != null)); + } + + @Override + public Object fold() { + List children = children(); + return children.isEmpty() ? null : children.get(0).fold(); + } + + @Override + public ScriptTemplate asScript() { + List templates = new ArrayList<>(); + for (Expression ex : children()) { + templates.add(asScript(ex)); + } + + StringJoiner template = new StringJoiner(",", "{sql}.coalesce(", ")"); + ParamsBuilder params = paramsBuilder(); + + for (ScriptTemplate scriptTemplate : templates) { + template.add(scriptTemplate.template()); + params.script(scriptTemplate.params()); + } + + return new ScriptTemplate(template.toString(), params.build(), dataType); + } + + @Override + protected Pipe makePipe() { + return new CoalescePipe(location(), this, Expressions.pipe(children())); + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalescePipe.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalescePipe.java new file mode 100644 index 0000000000000..a19d13e02df92 --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalescePipe.java @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.expression.predicate.conditional; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.gen.pipeline.MultiPipe; +import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; +import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.NodeInfo; + +import java.util.List; + +public class CoalescePipe extends MultiPipe { + + public CoalescePipe(Location location, Expression expression, List children) { + super(location, expression, children); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, CoalescePipe::new, expression(), children()); + } + + @Override + public Pipe replaceChildren(List newChildren) { + return new CoalescePipe(location(), expression(), newChildren); + } + + @Override + public Processor asProcessor(List procs) { + return new CoalesceProcessor(procs); + } +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalesceProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalesceProcessor.java new file mode 100644 index 0000000000000..e23b43634238d --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CoalesceProcessor.java @@ -0,0 +1,81 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.expression.predicate.conditional; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; + +import java.io.IOException; +import java.util.List; +import java.util.Objects; + +public class CoalesceProcessor implements Processor { + + public static final String NAME = "nco"; + + private final List processsors; + + public CoalesceProcessor(List processors) { + this.processsors = processors; + } + + public CoalesceProcessor(StreamInput in) throws IOException { + processsors = in.readNamedWriteableList(Processor.class); + } + + @Override + public String getWriteableName() { + return NAME; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeNamedWriteableList(processsors); + } + + @Override + public Object process(Object input) { + for (Processor proc : processsors) { + Object result = proc.process(input); + if (result != null) { + return result; + } + } + return null; + } + + public static Object apply(List values) { + if (values == null || values.isEmpty()) { + return null; + } + + for (Object object : values) { + if (object != null) { + return object; + } + } + + return null; + } + + @Override + public int hashCode() { + return Objects.hash(processsors); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + CoalesceProcessor that = (CoalesceProcessor) o; + return Objects.equals(processsors, that.processsors); + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java new file mode 100644 index 0000000000000..2feff40643335 --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java @@ -0,0 +1,23 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.expression.predicate.conditional; + +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; +import org.elasticsearch.xpack.sql.tree.Location; + +import java.util.List; + +/** + * Base class for conditional predicates. + */ +public abstract class ConditionalFunction extends ScalarFunction { + + protected ConditionalFunction(Location location, List fields) { + super(location, fields); + } +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/NotProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/NotProcessor.java index 8c82f2ab3ccda..3480854d5015e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/NotProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/NotProcessor.java @@ -14,7 +14,7 @@ public class NotProcessor implements Processor { - static final NotProcessor INSTANCE = new NotProcessor(); + public static final NotProcessor INSTANCE = new NotProcessor(); public static final String NAME = "ln"; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/IsNotNull.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java similarity index 96% rename from x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/IsNotNull.java rename to x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java index bd3fd5bf0811b..c4c848aecff32 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/IsNotNull.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -package org.elasticsearch.xpack.sql.expression.predicate; +package org.elasticsearch.xpack.sql.expression.predicate.nulls; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/IsNotNullProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNullProcessor.java similarity index 92% rename from x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/IsNotNullProcessor.java rename to x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNullProcessor.java index a9bec52a85926..ddc5f11975165 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/IsNotNullProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNullProcessor.java @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -package org.elasticsearch.xpack.sql.expression.predicate; +package org.elasticsearch.xpack.sql.expression.predicate.nulls; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -15,7 +15,7 @@ public class IsNotNullProcessor implements Processor { static final IsNotNullProcessor INSTANCE = new IsNotNullProcessor(); - public static final String NAME = "inn"; + public static final String NAME = "ninn"; private IsNotNullProcessor() {} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java index e2b8d02589ac0..c913626599d99 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java @@ -39,12 +39,13 @@ import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator; import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator.Negateable; import org.elasticsearch.xpack.sql.expression.predicate.BinaryPredicate; -import org.elasticsearch.xpack.sql.expression.predicate.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.Predicates; import org.elasticsearch.xpack.sql.expression.predicate.Range; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce; import org.elasticsearch.xpack.sql.expression.predicate.logical.And; import org.elasticsearch.xpack.sql.expression.predicate.logical.Not; import org.elasticsearch.xpack.sql.expression.predicate.logical.Or; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThan; @@ -127,6 +128,7 @@ protected Iterable.Batch> batches() { new ReplaceFoldableAttributes(), new FoldNull(), new ConstantFolding(), + new SimplifyCoalesce(), // boolean new BooleanSimplification(), new BooleanLiteralsOnTheRight(), @@ -686,7 +688,7 @@ protected LogicalPlan rule(Filter filter) { if (TRUE.equals(filter.condition())) { return filter.child(); } - if (FALSE.equals(filter.condition()) || FoldNull.isNull(filter.condition())) { + if (FALSE.equals(filter.condition()) || Expressions.isNull(filter.condition())) { return new LocalRelation(filter.location(), new EmptyExecutable(filter.output())); } } @@ -1121,10 +1123,6 @@ static class FoldNull extends OptimizerExpressionRule { super(TransformDirection.UP); } - private static boolean isNull(Expression ex) { - return DataType.NULL == ex.dataType() || (ex.foldable() && ex.fold() == null); - } - @Override protected Expression rule(Expression e) { if (e instanceof IsNotNull) { @@ -1137,12 +1135,12 @@ protected Expression rule(Expression e) { if (e instanceof In) { In in = (In) e; - if (isNull(in.value())) { + if (Expressions.isNull(in.value())) { return Literal.of(in, null); } } - if (e.nullable() && Expressions.anyMatch(e.children(), FoldNull::isNull)) { + if (e.nullable() && Expressions.anyMatch(e.children(), Expressions::isNull)) { return Literal.of(e, null); } @@ -1166,6 +1164,38 @@ protected Expression rule(Expression e) { return e.foldable() ? Literal.of(e) : e; } } + + static class SimplifyCoalesce extends OptimizerExpressionRule { + + SimplifyCoalesce() { + super(TransformDirection.DOWN); + } + + @Override + protected Expression rule(Expression e) { + if (e instanceof Coalesce) { + Coalesce c = (Coalesce) e; + + // find the first non-null foldable child (if any) and remove the rest + // while at it, exclude any nulls found + List newChildren = new ArrayList<>(); + + for (Expression child : c.children()) { + if (Expressions.isNull(child) == false) { + newChildren.add(child); + if (child.foldable()) { + break; + } + } + } + + if (newChildren.size() < c.children().size()) { + return new Coalesce(c.location(), newChildren); + } + } + return e; + } + } static class BooleanSimplification extends OptimizerExpressionRule { @@ -1930,4 +1960,4 @@ protected LogicalPlan rule(LogicalPlan plan) { enum TransformDirection { UP, DOWN }; -} +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java index 893f71c8bcb1b..b2f83fd834282 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java @@ -25,7 +25,6 @@ import org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction; import org.elasticsearch.xpack.sql.expression.function.scalar.Cast; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In; -import org.elasticsearch.xpack.sql.expression.predicate.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.Range; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MultiMatchQueryPredicate; @@ -33,6 +32,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.logical.And; import org.elasticsearch.xpack.sql.expression.predicate.logical.Not; import org.elasticsearch.xpack.sql.expression.predicate.logical.Or; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Add; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Div; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Mod; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java index 4e0bdea88b80b..bcd4a4cac2ad3 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java @@ -32,7 +32,6 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeHistogramFunction; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In; -import org.elasticsearch.xpack.sql.expression.predicate.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.Range; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MultiMatchQueryPredicate; @@ -40,6 +39,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.logical.And; import org.elasticsearch.xpack.sql.expression.predicate.logical.Not; import org.elasticsearch.xpack.sql.expression.predicate.logical.Or; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThan; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java index 53f7e6b1ab16d..26436c614f565 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java @@ -102,7 +102,7 @@ public static Conversion conversionFor(DataType from, DataType to) { if (from == to) { return Conversion.IDENTITY; } - if (to == DataType.NULL) { + if (to == DataType.NULL || from == DataType.NULL) { return Conversion.NULL; } if (from == DataType.NULL) { diff --git a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt index 827947424b08e..756a86b2fe7db 100644 --- a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt +++ b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt @@ -34,6 +34,11 @@ class org.elasticsearch.xpack.sql.expression.function.scalar.whitelist.InternalS Boolean not(Boolean) Boolean notNull(Object) +# +# Null +# + Object coalesce(java.util.List) + # # Regex # diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java index 68a0ea9e9490a..4786cf0367b32 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java @@ -36,11 +36,12 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.string.Repeat; import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In; -import org.elasticsearch.xpack.sql.expression.predicate.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.Range; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce; import org.elasticsearch.xpack.sql.expression.predicate.logical.And; import org.elasticsearch.xpack.sql.expression.predicate.logical.Not; import org.elasticsearch.xpack.sql.expression.predicate.logical.Or; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNull; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Add; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Div; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Mod; @@ -65,6 +66,7 @@ import org.elasticsearch.xpack.sql.optimizer.Optimizer.PruneDuplicateFunctions; import org.elasticsearch.xpack.sql.optimizer.Optimizer.PruneSubqueryAliases; import org.elasticsearch.xpack.sql.optimizer.Optimizer.ReplaceFoldableAttributes; +import org.elasticsearch.xpack.sql.optimizer.Optimizer.SimplifyCoalesce; import org.elasticsearch.xpack.sql.plan.logical.Filter; import org.elasticsearch.xpack.sql.plan.logical.LocalRelation; import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan; @@ -400,6 +402,21 @@ public void testGenericNullableExpression() { assertNullLiteral(rule.rule(new RLike(EMPTY, getFieldAttribute(), Literal.NULL))); } + public void testSimplifyCoalesceNulls() { + Expression e = new SimplifyCoalesce().rule(new Coalesce(EMPTY, asList(Literal.NULL, Literal.NULL))); + assertEquals(Coalesce.class, e.getClass()); + assertEquals(0, e.children().size()); + } + + public void testSimplifyCoalesceFirstLiteral() { + Expression e = new SimplifyCoalesce() + .rule(new Coalesce(EMPTY, + Arrays.asList(Literal.NULL, Literal.TRUE, Literal.FALSE, new Abs(EMPTY, getFieldAttribute())))); + assertEquals(Coalesce.class, e.getClass()); + assertEquals(1, e.children().size()); + assertEquals(Literal.TRUE, e.children().get(0)); + } + // // Logical simplifications // From 65605e815683a79b535bc2da82a9a9e74694f2b8 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Mon, 5 Nov 2018 23:05:15 +0200 Subject: [PATCH 2/2] Address feedback --- .../sql/qa/src/main/resources/null.csv-spec | 17 +++++++++++++- .../sql/qa/src/main/resources/null.sql-spec | 5 ++++- .../expression/function/FunctionRegistry.java | 4 ++++ .../predicate/conditional/Coalesce.java | 2 +- .../xpack/sql/optimizer/OptimizerTests.java | 22 ++++++++++++++++++- 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec index 377b356ed29e7..474fceaed4612 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/null.csv-spec @@ -17,13 +17,20 @@ null ; -divOfNull +divOfCastedNull SELECT 5 / CAST(NULL AS FLOAT) + 10 AS n; n:d null ; +divNoNull +SELECT 5 / null + 1 AS n; + +n:i +null +; + coalesceJustWithNull SELECT COALESCE(null, null, null) AS c; @@ -31,6 +38,14 @@ c null ; +coalesceFirstNotNull +SELECT COALESCE(123) AS c; + +c +123 +; + + coalesceWithFirstNullOfString SELECT COALESCE(null, 'first') AS c; diff --git a/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec b/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec index 95c5052f2837a..8976c8d8a5ea9 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/null.sql-spec @@ -6,4 +6,7 @@ coalesceField SELECT COALESCE(null, ABS(emp_no) + 1) AS c FROM test_emp ORDER BY emp_no LIMIT 5; coalesceHaving -SELECT COALESCE(languages, 0) c FROM test_emp GROUP BY languages ORDER BY languages; +SELECT COALESCE(null, ABS(MAX(emp_no)) + 1, 123) AS c FROM test_emp GROUP BY languages HAVING c > 100 ORDER BY languages LIMIT 5; + +coalesceWhere +SELECT COALESCE(null, ABS(emp_no) + 1, 123) AS c FROM test_emp WHERE COALESCE(null, ABS(emp_no) + 1, 123, 321) > 100 ORDER BY emp_no NULLS FIRST LIMIT 5; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java index c11a565f0aa54..70c9c18a5a4c0 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java @@ -371,6 +371,7 @@ static FunctionDefinition def(Class function, }; return def(function, builder, true, aliases); } + interface DatetimeUnaryFunctionBuilder { T build(Location location, Expression target, TimeZone tz); } @@ -397,6 +398,7 @@ static FunctionDefinition def(Class function, }; return def(function, builder, false, aliases); } + interface BinaryFunctionBuilder { T build(Location location, Expression lhs, Expression rhs); } @@ -415,6 +417,7 @@ private static FunctionDefinition def(Class function, Functi }; return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), function, datetime, realBuilder); } + private interface FunctionBuilder { Function build(Location location, List children, boolean distinct, TimeZone tz); } @@ -474,6 +477,7 @@ private static FunctionDefinition def(Class function, ctorRef.build(location, children.get(0), children.get(0).dataType()); return def(function, builder, false, aliases); } + private interface CastFunctionBuilder { T build(Location location, Expression expression, DataType dataType); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java index 9f93866d0b70b..8a2ddc4d10564 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Coalesce.java @@ -75,7 +75,7 @@ public ScriptTemplate asScript() { templates.add(asScript(ex)); } - StringJoiner template = new StringJoiner(",", "{sql}.coalesce(", ")"); + StringJoiner template = new StringJoiner(",", "{sql}.coalesce([", "])"); ParamsBuilder params = paramsBuilder(); for (ScriptTemplate scriptTemplate : templates) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java index 4786cf0367b32..e78115639d64d 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java @@ -35,7 +35,6 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.string.Ascii; import org.elasticsearch.xpack.sql.expression.function.scalar.string.Repeat; import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator; -import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.sql.expression.predicate.Range; import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce; import org.elasticsearch.xpack.sql.expression.predicate.logical.And; @@ -50,6 +49,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThan; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThanOrEqual; +import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.sql.expression.predicate.regex.Like; @@ -79,6 +79,7 @@ import org.elasticsearch.xpack.sql.tree.NodeInfo; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.EsField; +import org.elasticsearch.xpack.sql.util.CollectionUtils; import java.util.Arrays; import java.util.Collections; @@ -408,6 +409,25 @@ public void testSimplifyCoalesceNulls() { assertEquals(0, e.children().size()); } + public void testSimplifyCoalesceRandomNulls() { + Expression e = new SimplifyCoalesce().rule(new Coalesce(EMPTY, randomListOfNulls())); + assertEquals(Coalesce.class, e.getClass()); + assertEquals(0, e.children().size()); + } + + public void testSimplifyCoalesceRandomNullsWithValue() { + Expression e = new SimplifyCoalesce().rule(new Coalesce(EMPTY, + CollectionUtils.combine( + CollectionUtils.combine(randomListOfNulls(), Literal.TRUE, Literal.FALSE, Literal.TRUE), + randomListOfNulls()))); + assertEquals(1, e.children().size()); + assertEquals(Literal.TRUE, e.children().get(0)); + } + + private List randomListOfNulls() { + return asList(randomArray(1, 10, i -> new Literal[i], () -> Literal.NULL)); + } + public void testSimplifyCoalesceFirstLiteral() { Expression e = new SimplifyCoalesce() .rule(new Coalesce(EMPTY,