From 1d50355d9ac6888509a625b53d9751b908748c07 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sat, 27 Oct 2018 01:23:48 +0200 Subject: [PATCH 1/3] SQL: Add `CAST` and `CONVERT` to `SHOW FUNCTIONS` Fixes: #34939 --- .../function/FunctionDefinition.java | 7 ++++ .../expression/function/FunctionRegistry.java | 42 +++++++++++++++---- .../sql/expression/function/FunctionType.java | 3 ++ .../sql/expression/function/scalar/Cast.java | 3 +- .../xpack/qa/sql/cli/ShowTestCase.java | 5 ++- .../sql/src/main/resources/command.csv-spec | 4 +- .../qa/sql/src/main/resources/docs.csv-spec | 4 +- 7 files changed, 55 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java index d513ca07df4ae..5ba13792fe20c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java @@ -5,6 +5,8 @@ */ package org.elasticsearch.xpack.sql.expression.function; +import org.elasticsearch.xpack.sql.expression.function.scalar.Cast; + import java.util.List; import java.util.Locale; import java.util.TimeZone; @@ -19,6 +21,11 @@ public class FunctionDefinition { public interface Builder { Function build(UnresolvedFunction uf, boolean distinct, TimeZone tz); } + + public interface CastBuilder extends Builder { + Function build(Cast cast); + } + private final String name; private final List aliases; private final Class clazz; 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 4da4cf4d02301..8b8ecc44d529a 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 @@ -20,6 +20,7 @@ import org.elasticsearch.xpack.sql.expression.function.aggregate.Sum; import org.elasticsearch.xpack.sql.expression.function.aggregate.SumOfSquares; import org.elasticsearch.xpack.sql.expression.function.aggregate.VarPop; +import org.elasticsearch.xpack.sql.expression.function.scalar.Cast; import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayName; import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfMonth; import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfWeek; @@ -116,14 +117,14 @@ public class FunctionRegistry { public FunctionRegistry() { defineDefaultFunctions(); } - + /** * Constructor specifying alternate functions for testing. */ FunctionRegistry(FunctionDefinition... functions) { addToMap(functions); } - + private void defineDefaultFunctions() { // Aggregate functions addToMap(def(Avg.class, Avg::new), @@ -206,11 +207,13 @@ private void defineDefaultFunctions() { def(Space.class, Space::new), def(Substring.class, Substring::new), def(UCase.class, UCase::new)); + // DataType conversion + addToMap(defCast("CONVERT")); // Special addToMap(def(Score.class, Score::new)); } - - protected void addToMap(FunctionDefinition...functions) { + + void addToMap(FunctionDefinition...functions) { // temporary map to hold [function_name/alias_name : function instance] Map batchMap = new HashMap<>(); for (FunctionDefinition f : functions) { @@ -227,7 +230,7 @@ protected void addToMap(FunctionDefinition...functions) { // sort the temporary map by key name and add it to the global map of functions defs.putAll(batchMap.entrySet().stream() .sorted(Map.Entry.comparingByKey()) - .collect(Collectors., String, + .collect(Collectors., String, FunctionDefinition, LinkedHashMap> toMap(Map.Entry::getKey, Map.Entry::getValue, (oldValue, newValue) -> oldValue, LinkedHashMap::new))); } @@ -390,7 +393,7 @@ private static FunctionDefinition def(Class function, Functi private interface FunctionBuilder { Function build(Location location, List children, boolean distinct, TimeZone tz); } - + @SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do static FunctionDefinition def(Class function, ThreeParametersFunctionBuilder ctorRef, String... aliases) { @@ -408,11 +411,11 @@ static FunctionDefinition def(Class function, }; return def(function, builder, false, aliases); } - + interface ThreeParametersFunctionBuilder { T build(Location location, Expression source, Expression exp1, Expression exp2); } - + @SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do static FunctionDefinition def(Class function, FourParametersFunctionBuilder ctorRef, String... aliases) { @@ -427,11 +430,32 @@ static FunctionDefinition def(Class function, }; return def(function, builder, false, aliases); } - + interface FourParametersFunctionBuilder { T build(Location location, Expression source, Expression exp1, Expression exp2, Expression exp3); } + /** + * Special method to create function definition for {@link Cast} as its + * signature is not compatible with {@link UnresolvedFunction} + * @param aliases aliases for Cast + * @return Cast function definition + */ + private static FunctionDefinition defCast(String... aliases) { + String primaryName = normalize(Cast.class.getSimpleName()); + FunctionDefinition.CastBuilder builder = new FunctionDefinition.CastBuilder() { + @Override + public Function build(Cast cast) { + return cast; + } + @Override + public Function build(UnresolvedFunction uf, boolean distinct, TimeZone tz) { + return null; + } + }; + return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), Cast.class, false, builder); + } + private static String normalize(String name) { // translate CamelCase to camel_case return StringUtils.camelCaseToUnderscore(name); 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 dc75f0f5be37a..c68c206a0a9f2 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 @@ -7,11 +7,14 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction; +import org.elasticsearch.xpack.sql.expression.function.scalar.Cast; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.sql.expression.function.Score; public enum FunctionType { + + DATA_TYPE_CONVERSION(Cast.class), AGGREGATE(AggregateFunction.class), SCALAR(ScalarFunction.class), SCORE(Score.class); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java index 298039640446e..bc74abf4ada34 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java @@ -16,6 +16,7 @@ import java.util.Objects; public class Cast extends UnaryScalarFunction { + private final DataType dataType; public Cast(Location location, Expression field, DataType dataType) { @@ -102,4 +103,4 @@ public String name() { sb.insert(sb.length() - 1, " AS " + to().sqlName()); return sb.toString(); } -} \ No newline at end of file +} diff --git a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java index b4e87d3e20711..640ceb7e4c25c 100644 --- a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java +++ b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java @@ -39,7 +39,10 @@ public void testShowFunctions() throws IOException { while (scalarFunction.matcher(line).matches()) { line = readLine(); } - assertThat(line, RegexMatcher.matches("\\s*SCORE\\s*\\|\\s*SCORE\\s*")); + + assertThat(line, RegexMatcher.matches("\\s*CAST\\s*\\|\\s*DATA_TYPE_CONVERSION\\s*")); + assertThat(readLine(), RegexMatcher.matches("\\s*CONVERT\\s*\\|\\s*DATA_TYPE_CONVERSION\\s*")); + assertThat(readLine(), RegexMatcher.matches("\\s*SCORE\\s*\\|\\s*SCORE\\s*")); assertEquals("", readLine()); } diff --git a/x-pack/qa/sql/src/main/resources/command.csv-spec b/x-pack/qa/sql/src/main/resources/command.csv-spec index cc71dd947129a..e60b557c4bd9b 100644 --- a/x-pack/qa/sql/src/main/resources/command.csv-spec +++ b/x-pack/qa/sql/src/main/resources/command.csv-spec @@ -99,7 +99,9 @@ RTRIM |SCALAR SPACE |SCALAR SUBSTRING |SCALAR UCASE |SCALAR -SCORE |SCORE +CAST |DATA_TYPE_CONVERSION +CONVERT |DATA_TYPE_CONVERSION +SCORE |SCORE ; showFunctionsWithExactMatch diff --git a/x-pack/qa/sql/src/main/resources/docs.csv-spec b/x-pack/qa/sql/src/main/resources/docs.csv-spec index 4d5c8c26b8cd0..b69b391cef8c1 100644 --- a/x-pack/qa/sql/src/main/resources/docs.csv-spec +++ b/x-pack/qa/sql/src/main/resources/docs.csv-spec @@ -276,7 +276,9 @@ RTRIM |SCALAR SPACE |SCALAR SUBSTRING |SCALAR UCASE |SCALAR -SCORE |SCORE +CAST |DATA_TYPE_CONVERSION +CONVERT |DATA_TYPE_CONVERSION +SCORE |SCORE // end::showFunctions ; From afee48c743e17abb003e8ab25546287c552d0d73 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sat, 27 Oct 2018 12:00:34 +0200 Subject: [PATCH 2/3] simplify registration --- .../function/FunctionDefinition.java | 6 ---- .../expression/function/FunctionRegistry.java | 28 +++++++++---------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java index 5ba13792fe20c..a284ba83a972b 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java @@ -5,8 +5,6 @@ */ package org.elasticsearch.xpack.sql.expression.function; -import org.elasticsearch.xpack.sql.expression.function.scalar.Cast; - import java.util.List; import java.util.Locale; import java.util.TimeZone; @@ -22,10 +20,6 @@ public interface Builder { Function build(UnresolvedFunction uf, boolean distinct, TimeZone tz); } - public interface CastBuilder extends Builder { - Function build(Cast cast); - } - private final String name; private final List aliases; private final Class clazz; 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 8b8ecc44d529a..b36789bfbc6be 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 @@ -85,6 +85,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Mod; import org.elasticsearch.xpack.sql.parser.ParsingException; import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.util.StringUtils; import java.util.Arrays; @@ -208,7 +209,7 @@ private void defineDefaultFunctions() { def(Substring.class, Substring::new), def(UCase.class, UCase::new)); // DataType conversion - addToMap(defCast("CONVERT")); + addToMap(def(Cast.class, Cast::new, "CONVERT")); // Special addToMap(def(Score.class, Score::new)); } @@ -438,22 +439,19 @@ interface FourParametersFunctionBuilder { /** * Special method to create function definition for {@link Cast} as its * signature is not compatible with {@link UnresolvedFunction} - * @param aliases aliases for Cast + * * @return Cast function definition */ - private static FunctionDefinition defCast(String... aliases) { - String primaryName = normalize(Cast.class.getSimpleName()); - FunctionDefinition.CastBuilder builder = new FunctionDefinition.CastBuilder() { - @Override - public Function build(Cast cast) { - return cast; - } - @Override - public Function build(UnresolvedFunction uf, boolean distinct, TimeZone tz) { - return null; - } - }; - return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), Cast.class, false, builder); + @SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do + private static FunctionDefinition def(Class function, + CastFunctionBuilder ctorRef, + String... aliases) { + FunctionBuilder builder = (location, children, distinct, tz) -> + 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); } private static String normalize(String name) { From 8a591ccc30946c041a84dac1116e219189a86881 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Mon, 29 Oct 2018 13:33:43 +0100 Subject: [PATCH 3/3] Change type to SCALAR --- .../xpack/sql/expression/function/FunctionType.java | 3 --- .../java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java | 4 +--- x-pack/qa/sql/src/main/resources/command.csv-spec | 4 ++-- x-pack/qa/sql/src/main/resources/docs.csv-spec | 4 ++-- 4 files changed, 5 insertions(+), 10 deletions(-) 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 c68c206a0a9f2..5ed81e354fce2 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 @@ -7,14 +7,11 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction; -import org.elasticsearch.xpack.sql.expression.function.scalar.Cast; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; -import org.elasticsearch.xpack.sql.expression.function.Score; public enum FunctionType { - DATA_TYPE_CONVERSION(Cast.class), AGGREGATE(AggregateFunction.class), SCALAR(ScalarFunction.class), SCORE(Score.class); diff --git a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java index 640ceb7e4c25c..723ca8efb94d1 100644 --- a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java +++ b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ShowTestCase.java @@ -40,9 +40,7 @@ public void testShowFunctions() throws IOException { line = readLine(); } - assertThat(line, RegexMatcher.matches("\\s*CAST\\s*\\|\\s*DATA_TYPE_CONVERSION\\s*")); - assertThat(readLine(), RegexMatcher.matches("\\s*CONVERT\\s*\\|\\s*DATA_TYPE_CONVERSION\\s*")); - assertThat(readLine(), RegexMatcher.matches("\\s*SCORE\\s*\\|\\s*SCORE\\s*")); + assertThat(line, RegexMatcher.matches("\\s*SCORE\\s*\\|\\s*SCORE\\s*")); assertEquals("", readLine()); } diff --git a/x-pack/qa/sql/src/main/resources/command.csv-spec b/x-pack/qa/sql/src/main/resources/command.csv-spec index e60b557c4bd9b..0514b2a4982bb 100644 --- a/x-pack/qa/sql/src/main/resources/command.csv-spec +++ b/x-pack/qa/sql/src/main/resources/command.csv-spec @@ -99,8 +99,8 @@ RTRIM |SCALAR SPACE |SCALAR SUBSTRING |SCALAR UCASE |SCALAR -CAST |DATA_TYPE_CONVERSION -CONVERT |DATA_TYPE_CONVERSION +CAST |SCALAR +CONVERT |SCALAR SCORE |SCORE ; diff --git a/x-pack/qa/sql/src/main/resources/docs.csv-spec b/x-pack/qa/sql/src/main/resources/docs.csv-spec index b69b391cef8c1..8bccdc8d2fb26 100644 --- a/x-pack/qa/sql/src/main/resources/docs.csv-spec +++ b/x-pack/qa/sql/src/main/resources/docs.csv-spec @@ -276,8 +276,8 @@ RTRIM |SCALAR SPACE |SCALAR SUBSTRING |SCALAR UCASE |SCALAR -CAST |DATA_TYPE_CONVERSION -CONVERT |DATA_TYPE_CONVERSION +CAST |SCALAR +CONVERT |SCALAR SCORE |SCORE // end::showFunctions ;