SQL: Introduce Coalesce function (#35253)

Add Coalesce conditional for replacing null values Fix #35060
elastic · Nov 6, 2018 · 75e9a63 · 75e9a63
1 parent a5e1f4d
commit 75e9a63
Show file tree

Hide file tree

Showing 31 changed files with 560 additions and 71 deletions.
diff --git a/...ugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlIT.java b/...ugin/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
@@ -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
@@ -36,7 +36,7 @@ public static List<Object[]> 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
@@ -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
@@ -41,6 +41,7 @@ public static List<Object[]> 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
@@ -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
@@ -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
@@ -0,0 +1,75 @@
+//
+// 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
+;
+
+
+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;
+
+c
+null
+;
+
+coalesceFirstNotNull
+SELECT COALESCE(123) AS c;
+
+c
+123
+;
+
+
+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
@@ -0,0 +1,12 @@
+//
+// Null expressions
+//
+
+coalesceField
+SELECT COALESCE(null, ABS(emp_no) + 1) AS c FROM test_emp ORDER BY emp_no LIMIT 5;
+
+coalesceHaving
+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/qa/src/main/resources/nulls.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/nulls.csv-spec
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
@@ -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<String> names(Collection<? extends Expression> e) {
         List<String> 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> pipe(List<Expression> expressions) {
+        List<Pipe> 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<DataType> 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));
+    }
+}
diff --git a/...n/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java b/...n/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 <T extends Function> FunctionDefinition def(Class<T> 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 <T extends Function> FunctionDefinition def(Class<T> function,
+            MultiFunctionBuilder<T> 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> {
+        T build(Location location, List<Expression> 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 <T extends Function> FunctionDefinition def(Class<T> function,
         };
         return def(function, builder, false, aliases);
     }
+
     interface DistinctAwareUnaryFunctionBuilder<T> {
         T build(Location location, Expression target, boolean distinct);
     }
@@ -347,6 +371,7 @@ static <T extends Function> FunctionDefinition def(Class<T> function,
         };
         return def(function, builder, true, aliases);
     }
+
     interface DatetimeUnaryFunctionBuilder<T> {
         T build(Location location, Expression target, TimeZone tz);
     }
@@ -373,6 +398,7 @@ static <T extends Function> FunctionDefinition def(Class<T> function,
         };
         return def(function, builder, false, aliases);
     }
+
     interface BinaryFunctionBuilder<T> {
         T build(Location location, Expression lhs, Expression rhs);
     }
@@ -391,6 +417,7 @@ private static FunctionDefinition def(Class<? extends Function> function, Functi
         };
         return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), function, datetime, realBuilder);
     }
+
     private interface FunctionBuilder {
         Function build(Location location, List<Expression> children, boolean distinct, TimeZone tz);
     }
@@ -450,6 +477,7 @@ private static <T extends Function> FunctionDefinition def(Class<T> function,
             ctorRef.build(location, children.get(0), children.get(0).dataType());
         return def(function, builder, false, aliases);
     }
+
     private interface CastFunctionBuilder<T> {
         T build(Location location, Expression expression, DataType dataType);
     }

diff --git a/...lugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionType.java b/...lugin/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/.../sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Processors.java b/.../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<NamedWriteableRegistry.Entry> 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