From 1f924f57316f8762a9fba1728e5d62d89bae84ec Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Fri, 13 Jan 2023 09:04:20 -0800 Subject: [PATCH] Enable `concat()` string function to support multiple string arguments (#200) * Refactor concat() to support multiple string arguments Signed-off-by: Margarit Hakobyan --- .../function/BuiltinFunctionRepository.java | 7 ++ .../sql/expression/function/FunctionDSL.java | 103 +++++++++++++++++- .../function/SerializableVarargsFunction.java | 22 ++++ .../function/VarargsFunctionResolver.java | 69 ++++++++++++ .../sql/expression/text/TextFunction.java | 19 ++-- .../BuiltinFunctionRepositoryTest.java | 31 ++++++ .../function/FunctionDSLTestBase.java | 2 + .../function/FunctionDSLimplVarargsTest.java | 32 ++++++ .../function/VarargsFunctionResolverTest.java | 81 ++++++++++++++ .../sql/expression/text/TextFunctionTest.java | 20 ++++ docs/user/dql/functions.rst | 16 +-- docs/user/ppl/functions/string.rst | 16 +-- .../opensearch/sql/ppl/TextFunctionIT.java | 4 +- .../opensearch/sql/sql/TextFunctionIT.java | 1 + .../expressions/text_functions.txt | 4 +- 15 files changed, 399 insertions(+), 28 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/expression/function/SerializableVarargsFunction.java create mode 100644 core/src/main/java/org/opensearch/sql/expression/function/VarargsFunctionResolver.java create mode 100644 core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLimplVarargsTest.java create mode 100644 core/src/test/java/org/opensearch/sql/expression/function/VarargsFunctionResolverTest.java diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java index 71fd19991e..f3a5590b98 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java @@ -182,6 +182,13 @@ private FunctionBuilder getFunctionBuilder( if (isCastFunction(functionName) || sourceTypes.equals(targetTypes)) { return funcBuilder; } + // For functions with variable number of args (ex: concat()) + // targetTypes will always be empty (as the function signature is not fixed), + // and failure will occur. + // So, in this case sourceTypes are passed instead of targetTypes to address that. + if (functionResolverMap.get(functionName) instanceof VarargsFunctionResolver) { + return castArguments(sourceTypes, sourceTypes, funcBuilder); + } return castArguments(sourceTypes, targetTypes, funcBuilder); } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java b/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java index d94d7cdf60..c2814e9f4e 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java @@ -9,12 +9,14 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; import lombok.experimental.UtilityClass; import org.apache.commons.lang3.tuple.Pair; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; @@ -58,6 +60,39 @@ public static DefaultFunctionResolver define(FunctionName functionName, List< return builder.build(); } + /** + * Define varargs function with implementation. + * + * @param functionName function name. + * @param functions a list of function implementation. + * @return VarargsFunctionResolver. + */ + public static VarargsFunctionResolver defineVarargsFunction(FunctionName functionName, + SerializableFunction>... functions) { + return defineVarargsFunction(functionName, List.of(functions)); + } + + /** + * Define varargs function with implementation. + * + * @param functionName function name. + * @param functions a list of function implementation. + * @return VarargsFunctionResolver. + */ + public static VarargsFunctionResolver defineVarargsFunction(FunctionName functionName, List< + SerializableFunction>> functions) { + + VarargsFunctionResolver.VarargsFunctionResolverBuilder builder = + VarargsFunctionResolver.builder(); + builder.functionName(functionName); + for (SerializableFunction> func + : functions) { + Pair functionBuilder = func.apply(functionName); + builder.functionBundle(functionBuilder.getKey(), functionBuilder.getValue()); + } + return builder.build(); + } /** * Implementation of no args function that uses FunctionProperties. @@ -212,6 +247,56 @@ public static SerializableFunction function.apply(arg), returnType, argsType); } + /** + * Varargs Function Implementation. + * This implementation considers 1...n args of the same type. + * + * @param function {@link ExprValue} based varargs function. + * @param returnType return type. + * @param argsType argument type. + * @return Varargs Function Implementation. + */ + public static SerializableFunction> impl( + SerializableVarargsFunction function, + ExprType returnType, + ExprType argsType, + boolean withVarargs) { + + return functionName -> { + AtomicInteger argsCount = new AtomicInteger(0); + FunctionBuilder functionBuilder = + (functionProperties, arguments) -> new FunctionExpression(functionName, arguments) { + @Override + public ExprValue valueOf(Environment valueEnv) { + argsCount.set(arguments.size()); + ExprValue[] args = arguments.stream() + .map(arg -> arg.valueOf(valueEnv)) + .collect(Collectors.toList()) + .toArray(new ExprValue[arguments.size()]); + + return function.apply(args); + } + + @Override + public ExprType type() { + return returnType; + } + + @Override + public String toString() { + return String.format("%s(%s)", functionName, arguments.stream() + .map(Object::toString) + .collect(Collectors.joining(", "))); + } + }; + ExprCoreType[] argsTypes = new ExprCoreType[argsCount.get()]; + Arrays.fill(argsTypes, argsType); + FunctionSignature functionSignature = + new FunctionSignature(functionName, List.of(argsTypes)); + return Pair.of(functionSignature, functionBuilder); + }; + } + /** * Binary Function Implementation. * @@ -323,13 +408,29 @@ public SerializableTriFunction nullM }; } + /** + * Wrapper the varargs ExprValue function with default NULL and MISSING handling. + */ + public SerializableVarargsFunction nullMissingHandling( + SerializableVarargsFunction function, boolean withVarargs) { + return (args) -> { + if (Arrays.stream(args).anyMatch(ExprValue::isMissing)) { + return ExprValueUtils.missingValue(); + } else if (Arrays.stream(args).anyMatch(ExprValue::isNull)) { + return ExprValueUtils.nullValue(); + } else { + return function.apply(args); + } + }; + } + /** * Wrapper the unary ExprValue function that is aware of FunctionProperties, * with default NULL and MISSING handling. */ public static SerializableBiFunction nullMissingHandlingWithProperties( - SerializableBiFunction implementation) { + SerializableBiFunction implementation) { return (functionProperties, v1) -> { if (v1.isMissing()) { return ExprValueUtils.missingValue(); diff --git a/core/src/main/java/org/opensearch/sql/expression/function/SerializableVarargsFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/SerializableVarargsFunction.java new file mode 100644 index 0000000000..3c0c07fa79 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/expression/function/SerializableVarargsFunction.java @@ -0,0 +1,22 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.expression.function; + +import java.io.Serializable; + +/** + * Serializable Varargs Function. + */ +public interface SerializableVarargsFunction extends Serializable { + /** + * Applies this function to the given arguments. + * + * @param t the function argument + * @return the function result + */ + R apply(T... t); +} diff --git a/core/src/main/java/org/opensearch/sql/expression/function/VarargsFunctionResolver.java b/core/src/main/java/org/opensearch/sql/expression/function/VarargsFunctionResolver.java new file mode 100644 index 0000000000..5af054fb77 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/expression/function/VarargsFunctionResolver.java @@ -0,0 +1,69 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.function; + +import java.util.AbstractMap; +import java.util.Map; +import java.util.PriorityQueue; +import java.util.Set; +import java.util.stream.Collectors; +import lombok.Builder; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.Singular; +import org.apache.commons.lang3.tuple.Pair; +import org.opensearch.sql.exception.ExpressionEvaluationException; + +/** + * The Function Resolver hold the overload {@link FunctionBuilder} implementation. + * is composed by {@link FunctionName} which identified the function name + * and a map of {@link FunctionSignature} and {@link FunctionBuilder} + * to represent the overloaded implementation + */ +@Builder +@RequiredArgsConstructor +public class VarargsFunctionResolver implements FunctionResolver { + @Getter + private final FunctionName functionName; + @Singular("functionBundle") + private final Map functionBundle; + + /** + * Resolve the {@link FunctionBuilder} by using input {@link FunctionSignature}. + * If the {@link FunctionBuilder} exactly match the input {@link FunctionSignature}, return it. + * If applying the widening rule, found the most match one, return it. + * If nothing found, throw {@link ExpressionEvaluationException} + * + * @return function signature and its builder + */ + @Override + public Pair resolve(FunctionSignature unresolvedSignature) { + PriorityQueue> functionMatchQueue = new PriorityQueue<>( + Map.Entry.comparingByKey()); + + for (FunctionSignature functionSignature : functionBundle.keySet()) { + functionMatchQueue.add( + new AbstractMap.SimpleEntry<>(unresolvedSignature.match(functionSignature), + functionSignature)); + } + Map.Entry bestMatchEntry = functionMatchQueue.peek(); + if (unresolvedSignature.getParamTypeList().isEmpty()) { + throw new ExpressionEvaluationException( + String.format("%s function expected %s, but get %s", functionName, + formatFunctions(functionBundle.keySet()), + unresolvedSignature.formatTypes() + )); + } else { + FunctionSignature resolvedSignature = bestMatchEntry.getValue(); + return Pair.of(resolvedSignature, functionBundle.get(resolvedSignature)); + } + } + + private String formatFunctions(Set functionSignatures) { + return functionSignatures.stream().map(FunctionSignature::formatTypes) + .collect(Collectors.joining(",", "{", "}")); + } +} diff --git a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java b/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java index 25eb25489c..e57c696785 100644 --- a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java @@ -9,9 +9,12 @@ import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.expression.function.FunctionDSL.define; +import static org.opensearch.sql.expression.function.FunctionDSL.defineVarargsFunction; import static org.opensearch.sql.expression.function.FunctionDSL.impl; import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling; +import java.util.Arrays; +import java.util.stream.Collectors; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprIntegerValue; import org.opensearch.sql.data.model.ExprStringValue; @@ -22,7 +25,7 @@ import org.opensearch.sql.expression.function.FunctionName; import org.opensearch.sql.expression.function.SerializableBiFunction; import org.opensearch.sql.expression.function.SerializableTriFunction; - +import org.opensearch.sql.expression.function.VarargsFunctionResolver; /** * The definition of text functions. @@ -141,16 +144,16 @@ private DefaultFunctionResolver upper() { } /** - * TODO: https://github.com/opendistro-for-elasticsearch/sql/issues/710 - * Extend to accept variable argument amounts. * Concatenates a list of Strings. * Supports following signatures: - * (STRING, STRING) -> STRING + * (STRING, STRING, ...., STRING) -> STRING */ - private DefaultFunctionResolver concat() { - return define(BuiltinFunctionName.CONCAT.getName(), - impl(nullMissingHandling((str1, str2) -> - new ExprStringValue(str1.stringValue() + str2.stringValue())), STRING, STRING, STRING)); + private VarargsFunctionResolver concat() { + return defineVarargsFunction(BuiltinFunctionName.CONCAT.getName(), + impl(nullMissingHandling(strings -> + new ExprStringValue(Arrays.stream(strings) + .map(ExprValue::stringValue) + .collect(Collectors.joining())), true), STRING, STRING, true)); } /** diff --git a/core/src/test/java/org/opensearch/sql/expression/function/BuiltinFunctionRepositoryTest.java b/core/src/test/java/org/opensearch/sql/expression/function/BuiltinFunctionRepositoryTest.java index 8bba3bd9b9..28fbb7bd97 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/BuiltinFunctionRepositoryTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/BuiltinFunctionRepositoryTest.java @@ -238,6 +238,37 @@ void resolve_unregistered() { assertEquals("unsupported function name: unknown", exception.getMessage()); } + @Test + void resolve_should_cast_arguments_for_varargs_function() { + FunctionSignature unresolvedSignature = new FunctionSignature( + mockFunctionName, ImmutableList.of(STRING, STRING, STRING)); + FunctionSignature resolvedSignature = new FunctionSignature( + mockFunctionName, Collections.emptyList()); + + VarargsFunctionResolver varargsFunctionResolver = mock(VarargsFunctionResolver.class); + FunctionBuilder funcBuilder = mock(FunctionBuilder.class); + + when(mockFunctionName.getFunctionName()).thenReturn("mockFunction"); + when(mockExpression.toString()).thenReturn("string"); + when(mockNamespaceMap.get(DEFAULT_NAMESPACE)).thenReturn(mockMap); + when(mockNamespaceMap.containsKey(DEFAULT_NAMESPACE)).thenReturn(true); + when(mockMap.containsKey(eq(mockFunctionName))).thenReturn(true); + when(mockMap.get(eq(mockFunctionName))).thenReturn(varargsFunctionResolver); + when(varargsFunctionResolver.resolve(eq(unresolvedSignature))).thenReturn( + Pair.of(resolvedSignature, funcBuilder)); + repo.register(varargsFunctionResolver); + // Relax unnecessary stubbing check because error case test doesn't call this + lenient().doAnswer(invocation -> + new FakeFunctionExpression(mockFunctionName, invocation.getArgument(1)) + ).when(funcBuilder).apply(eq(functionProperties), any()); + + FunctionImplementation function = + repo.resolve(Collections.singletonList(DEFAULT_NAMESPACE), unresolvedSignature) + .apply(functionProperties, + ImmutableList.of(mockExpression, mockExpression, mockExpression)); + assertEquals("mockFunction(string, string, string)", function.toString()); + } + private FunctionSignature registerFunctionResolver(FunctionName funcName, ExprType sourceType, ExprType targetType) { diff --git a/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLTestBase.java b/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLTestBase.java index 63c6ea3329..5bcc9f9e89 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLTestBase.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLTestBase.java @@ -59,6 +59,8 @@ public int compareTo(ExprValue o) { twoArgs = (v1, v2) -> ANY; static final SerializableTriFunction threeArgs = (v1, v2, v3) -> ANY; + static final SerializableVarargsFunction + varrgs = (v1) -> ANY; @Mock FunctionProperties mockProperties; } diff --git a/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLimplVarargsTest.java b/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLimplVarargsTest.java new file mode 100644 index 0000000000..cee8889359 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/function/FunctionDSLimplVarargsTest.java @@ -0,0 +1,32 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.function; + +import static org.opensearch.sql.expression.function.FunctionDSL.impl; + +import java.util.List; +import org.apache.commons.lang3.tuple.Pair; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; + +class FunctionDSLimplVarargsTest extends FunctionDSLimplTestBase { + + @Override + SerializableFunction> + getImplementationGenerator() { + return impl(varrgs, ANY_TYPE, ANY_TYPE, true); + } + + @Override + List getSampleArguments() { + return List.of(DSL.literal(ANY)); + } + + @Override + String getExpected_toString() { + return "sample(ANY)"; + } +} diff --git a/core/src/test/java/org/opensearch/sql/expression/function/VarargsFunctionResolverTest.java b/core/src/test/java/org/opensearch/sql/expression/function/VarargsFunctionResolverTest.java new file mode 100644 index 0000000000..dafec21f40 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/function/VarargsFunctionResolverTest.java @@ -0,0 +1,81 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.expression.function; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.util.Collections; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.sql.data.type.WideningTypeRule; +import org.opensearch.sql.exception.ExpressionEvaluationException; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +@ExtendWith(MockitoExtension.class) +class VarargsFunctionResolverTest { + @Mock + private FunctionSignature exactlyMatchFS; + @Mock + private FunctionSignature bestMatchFS; + @Mock + private FunctionSignature leastMatchFS; + @Mock + private FunctionSignature notMatchFS; + @Mock + private FunctionSignature functionSignature; + @Mock + private FunctionBuilder exactlyMatchBuilder; + @Mock + private FunctionBuilder bestMatchBuilder; + @Mock + private FunctionBuilder leastMatchBuilder; + @Mock + private FunctionBuilder notMatchBuilder; + + private FunctionName functionName = FunctionName.of("test_function"); + + @Test + void resolve_function_signature_exactly_match() { + when(functionSignature.match(exactlyMatchFS)).thenReturn(WideningTypeRule.TYPE_EQUAL); + when(functionSignature.getParamTypeList()).thenReturn(ImmutableList.of(STRING, STRING, STRING)); + VarargsFunctionResolver resolver = new VarargsFunctionResolver(functionName, + ImmutableMap.of(exactlyMatchFS, exactlyMatchBuilder)); + + assertEquals(exactlyMatchBuilder, resolver.resolve(functionSignature).getValue()); + } + + @Test + void resolve_function_signature_best_match() { + when(functionSignature.match(bestMatchFS)).thenReturn(1); + when(functionSignature.match(leastMatchFS)).thenReturn(2); + when(functionSignature.getParamTypeList()).thenReturn(ImmutableList.of(STRING, STRING, STRING)); + VarargsFunctionResolver resolver = new VarargsFunctionResolver(functionName, + ImmutableMap.of(bestMatchFS, bestMatchBuilder, leastMatchFS, leastMatchBuilder)); + + assertEquals(bestMatchBuilder, resolver.resolve(functionSignature).getValue()); + } + + @Test + void resolve_function_not_match() { + when(functionSignature.match(notMatchFS)).thenReturn(WideningTypeRule.IMPOSSIBLE_WIDENING); + // accepts 1 or more arguments + when(functionSignature.getParamTypeList()).thenReturn(Collections.emptyList()); + VarargsFunctionResolver resolver = new VarargsFunctionResolver(functionName, + ImmutableMap.of(notMatchFS, notMatchBuilder)); + + assertThrows(ExpressionEvaluationException.class, () -> resolver.resolve(functionSignature)); + } +} diff --git a/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java index 515b436c82..a0fd3b8c3c 100644 --- a/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java @@ -72,6 +72,9 @@ public class TextFunctionTest extends ExpressionTestBase { private static List> CONCAT_STRING_LISTS = ImmutableList.of( ImmutableList.of("hello", "world"), ImmutableList.of("123", "5325")); + private static List> CONCAT_STRING_LISTS_WITH_MANY_STRINGS = ImmutableList.of( + ImmutableList.of("he", "llo", "wo", "rld", "!"), + ImmutableList.of("0", "123", "53", "25", "7")); interface SubstrSubstring { FunctionExpression getFunction(SubstringInfo strInfo); @@ -228,6 +231,7 @@ public void upper() { @Test void concat() { CONCAT_STRING_LISTS.forEach(this::testConcatString); + CONCAT_STRING_LISTS_WITH_MANY_STRINGS.forEach(this::testConcatMultipleString); when(nullRef.type()).thenReturn(STRING); when(missingRef.type()).thenReturn(STRING); @@ -446,6 +450,22 @@ void testConcatString(List strings, String delim) { assertEquals(expected, eval(expression).stringValue()); } + void testConcatMultipleString(List strings) { + String expected = null; + if (strings.stream().noneMatch(Objects::isNull)) { + expected = String.join("", strings); + } + + FunctionExpression expression = DSL.concat( + DSL.literal(strings.get(0)), + DSL.literal(strings.get(1)), + DSL.literal(strings.get(2)), + DSL.literal(strings.get(3)), + DSL.literal(strings.get(4))); + assertEquals(STRING, expression.type()); + assertEquals(expected, eval(expression).stringValue()); + } + void testLengthString(String str) { FunctionExpression expression = DSL.length(DSL.literal(new ExprStringValue(str))); assertEquals(INTEGER, expression.type()); diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index f433845bb3..2ce44d91e5 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2614,21 +2614,21 @@ CONCAT Description >>>>>>>>>>> -Usage: CONCAT(str1, str2) returns str1 and str strings concatenated together. +Usage: CONCAT(str1, str2, ...., str_n) adds two or more strings together. -Argument type: STRING, STRING +Argument type: STRING, STRING, ...., STRING Return type: STRING Example:: - os> SELECT CONCAT('hello', 'world') + os> SELECT CONCAT('hello', 'world'), CONCAT('hello ', 'whole ', 'world', '!'); fetched rows / total rows = 1/1 - +----------------------------+ - | CONCAT('hello', 'world') | - |----------------------------| - | helloworld | - +----------------------------+ + +----------------------------+--------------------------------------------+ + | CONCAT('hello', 'world') | CONCAT('hello ', 'whole ', 'world', '!') | + |----------------------------+--------------------------------------------| + | helloworld | hello whole world! | + +----------------------------+--------------------------------------------+ CONCAT_WS diff --git a/docs/user/ppl/functions/string.rst b/docs/user/ppl/functions/string.rst index 0503759cbd..315a46616c 100644 --- a/docs/user/ppl/functions/string.rst +++ b/docs/user/ppl/functions/string.rst @@ -14,21 +14,21 @@ CONCAT Description >>>>>>>>>>> -Usage: CONCAT(str1, str2) returns str1 and str strings concatenated together. +Usage: CONCAT(str1, str2, ...., str_n) adds two or more strings together. -Argument type: STRING, STRING +Argument type: STRING, STRING, ...., STRING Return type: STRING Example:: - os> source=people | eval `CONCAT('hello', 'world')` = CONCAT('hello', 'world') | fields `CONCAT('hello', 'world')` + os> source=people | eval `CONCAT('hello', 'world')` = CONCAT('hello', 'world'), `CONCAT('hello ', 'whole ', 'world', '!')` = CONCAT('hello ', 'whole ', 'world', '!') | fields `CONCAT('hello', 'world')`, `CONCAT('hello ', 'whole ', 'world', '!')` fetched rows / total rows = 1/1 - +----------------------------+ - | CONCAT('hello', 'world') | - |----------------------------| - | helloworld | - +----------------------------+ + +----------------------------+--------------------------------------------+ + | CONCAT('hello', 'world') | CONCAT('hello ', 'whole ', 'world', '!') | + |----------------------------+--------------------------------------------| + | helloworld | hello whole world! | + +----------------------------+--------------------------------------------+ CONCAT_WS diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/TextFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/TextFunctionIT.java index 7c48bceab0..024f190bee 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/TextFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/TextFunctionIT.java @@ -99,8 +99,8 @@ public void testLtrim() throws IOException { @Test public void testConcat() throws IOException { - verifyQuery("concat", "", ", 'there'", - "hellothere", "worldthere", "helloworldthere"); + verifyQuery("concat", "", ", 'there', 'all', '!'", + "hellothereall!", "worldthereall!", "helloworldthereall!"); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/TextFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/TextFunctionIT.java index 175cafd31e..94677354e4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/TextFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/TextFunctionIT.java @@ -108,6 +108,7 @@ public void testLtrim() throws IOException { @Test public void testConcat() throws IOException { + verifyQuery("concat('hello', 'whole', 'world', '!', '!')", "keyword", "hellowholeworld!!"); verifyQuery("concat('hello', 'world')", "keyword", "helloworld"); verifyQuery("concat('', 'hello')", "keyword", "hello"); } diff --git a/integ-test/src/test/resources/correctness/expressions/text_functions.txt b/integ-test/src/test/resources/correctness/expressions/text_functions.txt index c2fd57c330..077cc82084 100644 --- a/integ-test/src/test/resources/correctness/expressions/text_functions.txt +++ b/integ-test/src/test/resources/correctness/expressions/text_functions.txt @@ -11,4 +11,6 @@ LOCATE('world', 'helloworld') as column LOCATE('world', 'hello') as column LOCATE('world', 'helloworld', 7) as column REPLACE('helloworld', 'world', 'opensearch') as column -REPLACE('hello', 'world', 'opensearch') as column \ No newline at end of file +REPLACE('hello', 'world', 'opensearch') as column +CONCAT('hello', 'world') as column +CONCAT('hello ', 'whole ', 'world', '!') as column \ No newline at end of file