From 5f8442d1f406c78c04e055506efd85d569fa4d00 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Thu, 4 Jun 2020 13:43:49 +0200 Subject: [PATCH] SQL: Improve performances of LTRIM/RTRIM (#57603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change custom stripping leading and trailing whitespaces implementation to substantially improves performance: ``` Benchmark Mode Cnt Score Error Units StringTrim.testWithStringBuilder avgt 25 82547.575 ± 66.244 ns/op (existing impl) StringTrim.testWithSubstring avgt 25 1398.762 ± 101.152 ns/op (new impl) StringTrim.testWithJavaStrip avgt 25 1186.120 ± 10.374 ns/op (for reference) ``` Java's string stripLeading()/stripTrailing() not available to all supported JDKs. Enhanced LENGTH unit tests and compine a couple of LTRIM/RTRIM integ tests. Relates to: #57594 (partially cherry picked from commit ee7868d68733f195dc46926a7eab3d9dd7033ef4) Co-authored-by: Bogdan Pintea --- .../main/resources/string-functions.sql-spec | 14 ++++-------- .../scalar/string/StringFunctionUtils.java | 22 +++++++++---------- .../scalar/string/StringProcessor.java | 4 ++-- .../string/StringFunctionProcessorTests.java | 8 +++---- 4 files changed, 21 insertions(+), 27 deletions(-) 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 1e4f82cedfb..3033c9a5091 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 @@ -70,11 +70,8 @@ SELECT LEFT('Elasticsearch', LENGTH('abcdefghijklmnop')) leftchars; ltrimFilter SELECT LTRIM(first_name) lt FROM "test_emp" WHERE LTRIM(first_name) = 'Bob'; -ltrimInline1 -SELECT LTRIM(' Elastic ') trimmed; - -ltrimInline2 -SELECT LTRIM(' ') trimmed; +ltrimInline +SELECT LTRIM(' Elastic ') lt1, LTRIM(' ') lt2; locateInline1 SELECT LOCATE('a', 'Elasticsearch', 8) location; @@ -127,11 +124,8 @@ SELECT LTRIM("first_name") lt FROM "test_emp" WHERE LTRIM("first_name") LIKE '%a rtrimFilter SELECT RTRIM(first_name) rt FROM "test_emp" WHERE RTRIM(first_name) = 'Johnny'; -rtrimInline1 -SELECT RTRIM(' Elastic ') trimmed; - -rtrimInline2 -SELECT RTRIM(' ') trimmed; +rtrimInline +SELECT RTRIM(' Elastic ') rt1, RTRIM(' ') rt2; spaceFilter SELECT SPACE(languages) spaces, languages FROM "test_emp" WHERE SPACE(languages) = ' '; 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 834b9210bb8..c272084875b 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 @@ -23,15 +23,15 @@ final class StringFunctionUtils { if (!hasLength(s)) { return s; } - + if (start < 0) { start = 0; } - + if (start + 1 > s.length() || length < 0) { return ""; } - + return (start + length > s.length()) ? s.substring(start) : s.substring(start, start + length); } @@ -47,11 +47,11 @@ final class StringFunctionUtils { return s; } - StringBuilder sb = new StringBuilder(s); - while (sb.length() > 0 && Character.isWhitespace(sb.charAt(sb.length() - 1))) { - sb.deleteCharAt(sb.length() - 1); + int i = s.length() - 1; + while (i >= 0 && Character.isWhitespace(s.charAt(i))) { + i--; } - return sb.toString(); + return s.substring(0, i + 1); } /** @@ -66,10 +66,10 @@ final class StringFunctionUtils { return s; } - StringBuilder sb = new StringBuilder(s); - while (sb.length() > 0 && Character.isWhitespace(sb.charAt(0))) { - sb.deleteCharAt(0); + int i = 0; + while (i < s.length() && Character.isWhitespace(s.charAt(i))) { + i++; } - return sb.toString(); + return s.substring(i); } } 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 537075355f0..402c08a0e8a 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 @@ -51,8 +51,8 @@ public class StringProcessor implements Processor { LCASE((String s) -> s.toLowerCase(Locale.ROOT)), UCASE((String s) -> s.toUpperCase(Locale.ROOT)), LENGTH((String s) -> StringFunctionUtils.trimTrailingWhitespaces(s).length()), - RTRIM((String s) -> StringFunctionUtils.trimTrailingWhitespaces(s)), - LTRIM((String s) -> StringFunctionUtils.trimLeadingWhitespaces(s)), + RTRIM(StringFunctionUtils::trimTrailingWhitespaces), + LTRIM(StringFunctionUtils::trimLeadingWhitespaces), TRIM(String::trim), SPACE((Number n) -> { int i = n.intValue(); 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 cb96f71081b..7c8bbaf6c91 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 @@ -134,10 +134,10 @@ public class StringFunctionProcessorTests extends AbstractWireSerializingTestCas StringProcessor proc = new StringProcessor(StringOperation.LENGTH); assertNull(proc.process(null)); assertEquals(7, proc.process("foo bar")); - assertEquals(0, proc.process("")); - assertEquals(0, proc.process(" ")); - assertEquals(7, proc.process("foo bar ")); - assertEquals(10, proc.process(" foo bar ")); + assertEquals(0, proc.process(withRandomWhitespaces(" \t \r\n \n ", true, true))); + assertEquals(0, proc.process(withRandomWhitespaces(" ", true, true))); + assertEquals(7, proc.process(withRandomWhitespaces("foo bar", false, true))); + assertEquals(10, proc.process(withRandomWhitespaces(" foo bar ", false, true))); assertEquals(1, proc.process('f')); stringCharInputValidation(proc);