diff --git a/docs/reference/sql/functions/index.asciidoc b/docs/reference/sql/functions/index.asciidoc index 3f9ac26976833..393266fed8e95 100644 --- a/docs/reference/sql/functions/index.asciidoc +++ b/docs/reference/sql/functions/index.asciidoc @@ -129,6 +129,7 @@ ** <> ** <> ** <> +** <> ** <> * <> ** <> diff --git a/docs/reference/sql/functions/string.asciidoc b/docs/reference/sql/functions/string.asciidoc index 167e1e69e64ba..61fbbaad15163 100644 --- a/docs/reference/sql/functions/string.asciidoc +++ b/docs/reference/sql/functions/string.asciidoc @@ -477,6 +477,26 @@ SUBSTRING( -------------------------------------------------- include-tagged::{sql-specs}/docs/docs.csv-spec[stringSubString] -------------------------------------------------- +[[sql-functions-string-trim]] +==== `TRIM` + +.Synopsis: +[source, sql] +-------------------------------------------------- +TRIM(string_exp) <1> +-------------------------------------------------- +*Input*: + +<1> string expression + +*Output*: string + +*Description*: Returns the characters of `string_exp`, with leading and trailing blanks removed. + +[source, sql] +-------------------------------------------------- +include-tagged::{sql-specs}/docs/docs.csv-spec[stringTrim] +-------------------------------------------------- [[sql-functions-string-ucase]] ==== `UCASE` diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec index 241c6c15dfe7d..710e12036e6da 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec @@ -147,7 +147,8 @@ RIGHT |SCALAR RTRIM |SCALAR SPACE |SCALAR STARTS_WITH |SCALAR -SUBSTRING |SCALAR +SUBSTRING |SCALAR +TRIM |SCALAR UCASE |SCALAR CAST |SCALAR CONVERT |SCALAR diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/docs/docs.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/docs/docs.csv-spec index abd6e70aa25ad..20be529b399c7 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/docs/docs.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/docs/docs.csv-spec @@ -343,7 +343,8 @@ RIGHT |SCALAR RTRIM |SCALAR SPACE |SCALAR STARTS_WITH |SCALAR -SUBSTRING |SCALAR +SUBSTRING |SCALAR +TRIM |SCALAR UCASE |SCALAR CAST |SCALAR CONVERT |SCALAR @@ -1842,6 +1843,16 @@ Elastic // end::stringSubString ; +stringTrim +// tag::stringTrim +SELECT TRIM(' Elastic ') AS trimmed; + +trimmed +-------------- +Elastic +// end::stringTrim +; + stringUCase // tag::stringUCase SELECT UCASE('Elastic'); diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/string-functions.sql-spec b/x-pack/plugin/sql/qa/server/src/main/resources/string-functions.sql-spec index 2ac4257d73d8f..1e4f82cedfbee 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/string-functions.sql-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/string-functions.sql-spec @@ -154,6 +154,12 @@ SELECT SUBSTRING('Elasticsearch', 1, 15) sub; substringInline3 SELECT SUBSTRING('Elasticsearch', 10, 10) sub; +trimFilter +SELECT TRIM(CONCAT(CONCAT(' ', first_name), ' ')) trimmed FROM "test_emp" WHERE TRIM(CONCAT(CONCAT(' ', first_name), ' ')) = 'Bob'; + +trimInline +SELECT TRIM(' Elastic ') trimmed1, TRIM(' ') trimmed2; + ucaseFilter SELECT UCASE(gender) uppercased, COUNT(*) count FROM "test_emp" WHERE UCASE(gender) = 'F' GROUP BY UCASE(gender); @@ -188,6 +194,17 @@ SELECT RTRIM(first_name) rt FROM "test_emp" GROUP BY RTRIM(first_name) HAVING CO ltrimGroupByAndOrderBy SELECT LTRIM(first_name) lt FROM "test_emp" GROUP BY LTRIM(first_name) HAVING COUNT(*)>1; +trimOrderBy +SELECT TRIM(CONCAT(CONCAT(' ', first_name), ' ')) trimmed FROM "test_emp" ORDER BY 1; + +trimGroupBy +SELECT TRIM(CONCAT(CONCAT(' ', first_name), ' ')) trimmed FROM "test_emp" GROUP BY TRIM(CONCAT(CONCAT(' ', first_name), ' ')) ORDER BY 1; + +// Having on MAX/MIN() not supported: https://github.com/elastic/elasticsearch/issues/37938 +trimGroupByAndHaving-Ignore +SELECT MAX(CONCAT(CONCAT(' ', first_name), ' ')) max trimmed FROM "test_emp" GROUP BY gender HAVING MAX(CONCAT(CONCAT(' ', gender), ' ')) = 'Zvonko' ORDER BY 1; + + spaceGroupByWithCharLength SELECT CAST(CHAR_LENGTH(SPACE(languages)) AS INT) cls FROM "test_emp" GROUP BY CHAR_LENGTH(SPACE(languages)) ORDER BY CHAR_LENGTH(SPACE(languages)) ASC; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/SqlFunctionRegistry.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/SqlFunctionRegistry.java index 1649b930661c2..62a49da1efba6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/SqlFunctionRegistry.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/SqlFunctionRegistry.java @@ -106,6 +106,7 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.string.Right; 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.Trim; import org.elasticsearch.xpack.sql.expression.function.scalar.string.UCase; import org.elasticsearch.xpack.sql.expression.predicate.conditional.Case; import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce; @@ -243,6 +244,7 @@ private static FunctionDefinition[][] functions() { def(Space.class, Space::new, "SPACE"), def(StartsWith.class, StartsWith::new, "STARTS_WITH"), def(Substring.class, Substring::new, "SUBSTRING"), + def(Trim.class, Trim::new, "TRIM"), def(UCase.class, UCase::new, "UCASE") }, // DataType conversion diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionUtils.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionUtils.java index d09e3ee317c2d..834b9210bb8bc 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionUtils.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionUtils.java @@ -7,7 +7,9 @@ import static org.elasticsearch.common.Strings.hasLength; -abstract class StringFunctionUtils { +final class StringFunctionUtils { + + private StringFunctionUtils() {} /** * Extract a substring from the given string, using start index and length of the extracted substring. @@ -41,7 +43,7 @@ static String substring(String s, int start, int length) { * @return the resulting String */ static String trimTrailingWhitespaces(String s) { - if (!hasLength(s)) { + if (hasLength(s) == false) { return s; } @@ -60,7 +62,7 @@ static String trimTrailingWhitespaces(String s) { * @return the resulting String */ static String trimLeadingWhitespaces(String s) { - if (!hasLength(s)) { + if (hasLength(s) == false) { return s; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java index 6767d4bc2085f..537075355f0ce 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java @@ -53,6 +53,7 @@ public enum StringOperation { LENGTH((String s) -> StringFunctionUtils.trimTrailingWhitespaces(s).length()), RTRIM((String s) -> StringFunctionUtils.trimTrailingWhitespaces(s)), LTRIM((String s) -> StringFunctionUtils.trimLeadingWhitespaces(s)), + TRIM(String::trim), SPACE((Number n) -> { int i = n.intValue(); if (i < 0) { @@ -141,4 +142,4 @@ public int hashCode() { public String toString() { return processor.toString(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Trim.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Trim.java new file mode 100644 index 0000000000000..bbba956b21179 --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Trim.java @@ -0,0 +1,43 @@ +/* + * 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.function.scalar.string; + +import org.elasticsearch.xpack.ql.expression.Expression; +import org.elasticsearch.xpack.ql.tree.NodeInfo; +import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; +import org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.StringOperation; + +/** + * Trims both leading and trailing whitespaces. + */ +public class Trim extends UnaryStringFunction { + + public Trim(Source source, Expression field) { + super(source, field); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, Trim::new, field()); + } + + @Override + protected Trim replaceChild(Expression newChild) { + return new Trim(source(), newChild); + } + + @Override + protected StringOperation operation() { + return StringOperation.TRIM; + } + + @Override + public DataType dataType() { + return DataTypes.KEYWORD; + } +} 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 5e0ea7fe5bb3c..757da33589d90 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 @@ -425,6 +425,10 @@ public static String substring(String s, Number start, Number length) { return (String) SubstringFunctionProcessor.doProcess(s, start, length); } + public static String trim(String s) { + return (String) StringOperation.TRIM.apply(s); + } + public static String ucase(String s) { return (String) StringOperation.UCASE.apply(s); } 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 a87031980f9a2..4174bfee1f6b0 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 @@ -164,6 +164,7 @@ class org.elasticsearch.xpack.sql.expression.function.scalar.whitelist.InternalS String rtrim(String) String space(Number) String substring(String, Number, Number) + String trim(String) String ucase(String) # diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java index b43f1bae51a84..cb96f71081bf8 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java @@ -148,9 +148,10 @@ public void testRTrim() { assertNull(proc.process(null)); assertEquals("foo bar", proc.process("foo bar")); assertEquals("", proc.process("")); - assertEquals("", proc.process(" ")); - assertEquals("foo bar", proc.process("foo bar ")); - assertEquals(" foo bar", proc.process(" foo bar ")); + assertEquals("", proc.process(withRandomWhitespaces(" \t \r\n \n ", true, true))); + assertEquals("foo bar", proc.process(withRandomWhitespaces("foo bar", false, true))); + assertEquals(" foo bar", proc.process(withRandomWhitespaces(" foo bar", false, true))); + assertEquals(" \t \n \r\n foo \t \r\n \n bar", proc.process(withRandomWhitespaces(" \t \n \r\n foo \t \r\n \n bar", false, true))); assertEquals("f", proc.process('f')); stringCharInputValidation(proc); @@ -161,9 +162,25 @@ public void testLTrim() { assertNull(proc.process(null)); assertEquals("foo bar", proc.process("foo bar")); assertEquals("", proc.process("")); - assertEquals("", proc.process(" ")); - assertEquals("foo bar", proc.process(" foo bar")); - assertEquals("foo bar ", proc.process(" foo bar ")); + assertEquals("", proc.process(withRandomWhitespaces(" \t \r\n \n ", true, true))); + assertEquals("foo bar", proc.process(withRandomWhitespaces("foo bar", true, false))); + assertEquals("foo bar ", proc.process(withRandomWhitespaces("foo bar ", true, false))); + assertEquals("foo \t \r\n \n bar \t \r\n \n ", proc.process(withRandomWhitespaces("foo \t \r\n \n bar \t \r\n \n ", true, false))); + assertEquals("f", proc.process('f')); + + stringCharInputValidation(proc); + } + + public void testTrim() { + StringProcessor proc = new StringProcessor(StringOperation.TRIM); + assertNull(proc.process(null)); + assertEquals("foo bar", proc.process("foo bar")); + assertEquals("", proc.process("")); + assertEquals("", proc.process(withRandomWhitespaces(" \t \r\n \n ", true, true))); + assertEquals("foo bar", proc.process(withRandomWhitespaces("foo bar", true, false))); + assertEquals("foo bar", proc.process(withRandomWhitespaces("foo bar", false, true))); + assertEquals("foo bar", proc.process(withRandomWhitespaces("foo bar",true, true))); + assertEquals("foo \t \r\n \n bar", proc.process(withRandomWhitespaces("foo \t \r\n \n bar", true, true))); assertEquals("f", proc.process('f')); stringCharInputValidation(proc); @@ -215,4 +232,22 @@ public void testOctetLength() { stringCharInputValidation(proc); } + + private static String withRandomWhitespaces(String str, boolean prefix, boolean suffix) { + if (prefix == false && suffix == false) { + return str; + } + + StringBuilder sb = new StringBuilder(str); + int noWhitespaces = randomIntBetween(1, 100); + for (int i = 0; i < noWhitespaces; i++) { + if (prefix) { + sb.insert(0, randomFrom(" ", "\t", "\n", "\r", "\r\n")); + } + if (suffix) { + sb.append(randomFrom(" ", "\t", "\n", "\r", "\r\n")); + } + } + return sb.toString(); + } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index 324d660861626..aac6de786229c 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -56,6 +56,10 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor.DateTimeExtractor; import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation; import org.elasticsearch.xpack.sql.expression.function.scalar.math.Round; +import org.elasticsearch.xpack.sql.expression.function.scalar.string.LTrim; +import org.elasticsearch.xpack.sql.expression.function.scalar.string.RTrim; +import org.elasticsearch.xpack.sql.expression.function.scalar.string.Trim; +import org.elasticsearch.xpack.sql.expression.function.scalar.string.UnaryStringFunction; import org.elasticsearch.xpack.sql.optimizer.Optimizer; import org.elasticsearch.xpack.sql.parser.SqlParser; import org.elasticsearch.xpack.sql.plan.physical.EsQueryExec; @@ -741,6 +745,52 @@ public void testStartsWithUsesPrefixQueryAndScript() { assertEquals("[{v=keyword}, {v=xyz}, {v=true}]", sq.script().params().toString()); } + @SuppressWarnings("unchecked") + public void testTrim_WhereClause_Painless() { + Class trimFunction = randomFrom(Trim.class, LTrim.class, RTrim.class); + String trimFunctionName = trimFunction.getSimpleName().toUpperCase(Locale.ROOT); + LogicalPlan p = plan("SELECT " + trimFunctionName + "(keyword) trimmed FROM test WHERE " + trimFunctionName + "(keyword) = 'foo'"); + + assertTrue(p instanceof Project); + p = ((Project) p).child(); + assertTrue(p instanceof Filter); + Expression condition = ((Filter) p).condition(); + QueryTranslation qt = translate(condition); + assertTrue(qt.query instanceof ScriptQuery); + ScriptQuery sc = (ScriptQuery) qt.query; + assertEquals("InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.eq(InternalSqlScriptUtils." + + trimFunctionName.toLowerCase(Locale.ROOT) + "(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))", + sc.script().toString()); + assertEquals("[{v=keyword}, {v=foo}]", sc.script().params().toString()); + } + + @SuppressWarnings("unchecked") + public void testTrim_GroupBy_Painless() { + Class trimFunction = randomFrom(Trim.class, LTrim.class, RTrim.class); + String trimFunctionName = trimFunction.getSimpleName().toUpperCase(Locale.ROOT); + LogicalPlan p = plan("SELECT " + trimFunctionName + "(keyword) trimmed, count(*) FROM test GROUP BY " + + trimFunctionName + "(keyword)"); + + assertEquals(Aggregate.class, p.getClass()); + Aggregate agg = (Aggregate) p; + assertEquals(1, agg.groupings().size()); + assertEquals(2, agg.aggregates().size()); + assertEquals(trimFunction, agg.groupings().get(0).getClass()); + assertEquals(trimFunction, ((Alias) agg.aggregates().get(0)).child().getClass()); + assertEquals(Count.class,((Alias) agg.aggregates().get(1)).child().getClass()); + + UnaryStringFunction trim = (UnaryStringFunction) agg.groupings().get(0); + assertEquals(1, trim.children().size()); + + GroupingContext groupingContext = QueryFolder.FoldAggregate.groupBy(agg.groupings()); + assertNotNull(groupingContext); + ScriptTemplate scriptTemplate = groupingContext.tail.script(); + assertEquals("InternalSqlScriptUtils." + trimFunctionName.toLowerCase(Locale.ROOT) + + "(InternalQlScriptUtils.docValue(doc,params.v0))", + scriptTemplate.toString()); + assertEquals("[{v=keyword}]", scriptTemplate.params().toString()); + } + public void testTranslateNotExpression_WhereClause_Painless() { LogicalPlan p = plan("SELECT * FROM test WHERE NOT(POSITION('x', keyword) = 0)"); assertTrue(p instanceof Project);