Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ private FunctionBuilder getFunctionBuilder(
if (isCastFunction(functionName) || sourceTypes.equals(targetTypes)) {
return funcBuilder;
}
if (functionName.equals(BuiltinFunctionName.CONCAT.getName())) {
return funcBuilder;
}
Comment thread
dai-chen marked this conversation as resolved.
Outdated
return castArguments(sourceTypes,
targetTypes, funcBuilder);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,15 @@ public Pair<FunctionSignature, FunctionBuilder> resolve(FunctionSignature unreso
functionSignature));
}
Map.Entry<Integer, FunctionSignature> bestMatchEntry = functionMatchQueue.peek();
if (FunctionSignature.NOT_MATCH.equals(bestMatchEntry.getKey())) {
if (isConcatFunction(unresolvedSignature)
&& (unresolvedSignature.getParamTypeList().isEmpty()
|| unresolvedSignature.getParamTypeList().size() > 9)) {
throw new ExpressionEvaluationException(
String.format("%s function expected 1-9 arguments, but got %s",
Comment thread
Yury-Fridlyand marked this conversation as resolved.
Outdated
functionName, unresolvedSignature.getParamTypeList().size()));
}
if (FunctionSignature.NOT_MATCH.equals(bestMatchEntry.getKey())
&& !isConcatFunction(unresolvedSignature)) {
throw new ExpressionEvaluationException(
String.format("%s function expected %s, but get %s", functionName,
formatFunctions(functionBundle.keySet()),
Expand All @@ -66,4 +74,8 @@ private String formatFunctions(Set<FunctionSignature> functionSignatures) {
return functionSignatures.stream().map(FunctionSignature::formatTypes)
.collect(Collectors.joining(",", "{", "}"));
}

private boolean isConcatFunction(FunctionSignature signature) {
return signature.getFunctionName().equals(BuiltinFunctionName.CONCAT.getName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
Yury-Fridlyand marked this conversation as resolved.
Outdated
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.FunctionExpression;
Expand Down Expand Up @@ -212,6 +214,56 @@ public static SerializableFunction<FunctionName, Pair<FunctionSignature, Functio
return implWithProperties((fp, arg) -> 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<FunctionName, Pair<FunctionSignature, FunctionBuilder>> impl(
SerializableVarargsFunction<ExprValue, ExprValue> 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<Expression, ExprValue> valueEnv) {
argsCount.set(arguments.size());
Comment thread
dai-chen marked this conversation as resolved.
Outdated
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.
*
Expand Down Expand Up @@ -323,13 +375,29 @@ public SerializableTriFunction<ExprValue, ExprValue, ExprValue, ExprValue> nullM
};
}

/**
* Wrapper the varargs ExprValue function with default NULL and MISSING handling.
*/
public SerializableVarargsFunction<ExprValue, ExprValue> nullMissingHandling(
SerializableVarargsFunction<ExprValue, ExprValue> function, boolean withVarargs) {
return (args) -> {
if (Arrays.stream(args).anyMatch(ExprValue::isMissing)) {
return ExprValueUtils.missingValue();
}
if (Arrays.stream(args).anyMatch(ExprValue::isNull)) {
return ExprValueUtils.nullValue();
}
return function.apply(args);
};
}

/**
* Wrapper the unary ExprValue function that is aware of FunctionProperties,
* with default NULL and MISSING handling.
*/
public static SerializableBiFunction<FunctionProperties, ExprValue, ExprValue>
nullMissingHandlingWithProperties(
SerializableBiFunction<FunctionProperties, ExprValue, ExprValue> implementation) {
SerializableBiFunction<FunctionProperties, ExprValue, ExprValue> implementation) {
return (functionProperties, v1) -> {
if (v1.isMissing()) {
return ExprValueUtils.missingValue();
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T, R> extends Serializable {
/**
* Applies this function to the given arguments.
*
* @param t the function argument
* @return the function result
*/
R apply(T... t);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
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;
Expand All @@ -23,7 +25,6 @@
import org.opensearch.sql.expression.function.SerializableBiFunction;
import org.opensearch.sql.expression.function.SerializableTriFunction;


/**
* The definition of text functions.
* 1) have the clear interface for function define.
Expand Down Expand Up @@ -141,16 +142,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));
impl(nullMissingHandling(strings ->
Comment thread
dai-chen marked this conversation as resolved.
Outdated
new ExprStringValue(Arrays.stream(strings)
.map(ExprValue::stringValue)
.collect(Collectors.joining())), true), STRING, STRING, true));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
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;
Expand Down Expand Up @@ -47,6 +50,7 @@ class DefaultFunctionResolverTest {
@Test
void resolve_function_signature_exactly_match() {
when(functionSignature.match(exactlyMatchFS)).thenReturn(WideningTypeRule.TYPE_EQUAL);
when(functionSignature.getFunctionName()).thenReturn(functionName);
DefaultFunctionResolver resolver = new DefaultFunctionResolver(functionName,
ImmutableMap.of(exactlyMatchFS, exactlyMatchBuilder));

Expand All @@ -57,6 +61,7 @@ void resolve_function_signature_exactly_match() {
void resolve_function_signature_best_match() {
when(functionSignature.match(bestMatchFS)).thenReturn(1);
when(functionSignature.match(leastMatchFS)).thenReturn(2);
when(functionSignature.getFunctionName()).thenReturn(functionName);
DefaultFunctionResolver resolver = new DefaultFunctionResolver(functionName,
ImmutableMap.of(bestMatchFS, bestMatchBuilder, leastMatchFS, leastMatchBuilder));

Expand All @@ -68,6 +73,7 @@ void resolve_function_not_match() {
when(functionSignature.match(notMatchFS)).thenReturn(WideningTypeRule.IMPOSSIBLE_WIDENING);
when(notMatchFS.formatTypes()).thenReturn("[INTEGER,INTEGER]");
when(functionSignature.formatTypes()).thenReturn("[BOOLEAN,BOOLEAN]");
when(functionSignature.getFunctionName()).thenReturn(functionName);
DefaultFunctionResolver resolver = new DefaultFunctionResolver(functionName,
ImmutableMap.of(notMatchFS, notMatchBuilder));

Expand All @@ -76,4 +82,53 @@ void resolve_function_not_match() {
assertEquals("add function expected {[INTEGER,INTEGER]}, but get [BOOLEAN,BOOLEAN]",
exception.getMessage());
}

@Test
void resolve_concat_function_signature_match() {
functionName = FunctionName.of("concat");
when(functionSignature.match(notMatchFS)).thenReturn(WideningTypeRule.IMPOSSIBLE_WIDENING);
when(functionSignature.getFunctionName()).thenReturn(functionName);
when(functionSignature.getParamTypeList()).thenReturn(ImmutableList.of(STRING));

DefaultFunctionResolver resolver = new DefaultFunctionResolver(functionName,
ImmutableMap.of(notMatchFS, notMatchBuilder));

assertEquals(notMatchBuilder, resolver.resolve(functionSignature).getValue());
}

@Test
void resolve_concat_no_args_function_signature_not_match() {
functionName = FunctionName.of("concat");
when(functionSignature.match(notMatchFS)).thenReturn(WideningTypeRule.IMPOSSIBLE_WIDENING);
when(functionSignature.getFunctionName()).thenReturn(functionName);
// Concat function with no arguments
when(functionSignature.getParamTypeList()).thenReturn(Collections.emptyList());

DefaultFunctionResolver resolver = new DefaultFunctionResolver(functionName,
ImmutableMap.of(notMatchFS, notMatchBuilder));

ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class,
() -> resolver.resolve(functionSignature));
assertEquals("concat function expected 1-9 arguments, but got 0",
exception.getMessage());
}

@Test
void resolve_concat_too_many_args_function_signature_not_match() {
functionName = FunctionName.of("concat");
when(functionSignature.match(notMatchFS)).thenReturn(WideningTypeRule.IMPOSSIBLE_WIDENING);
when(functionSignature.getFunctionName()).thenReturn(functionName);
// Concat function with more than 9 arguments
when(functionSignature.getParamTypeList()).thenReturn(ImmutableList
.of(STRING, STRING, STRING, STRING, STRING,
STRING, STRING, STRING, STRING, STRING));

DefaultFunctionResolver resolver = new DefaultFunctionResolver(functionName,
ImmutableMap.of(notMatchFS, notMatchBuilder));

ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class,
() -> resolver.resolve(functionSignature));
assertEquals("concat function expected 1-9 arguments, but got 10",
exception.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public int compareTo(ExprValue o) {
twoArgs = (v1, v2) -> ANY;
static final SerializableTriFunction<ExprValue, ExprValue, ExprValue, ExprValue>
threeArgs = (v1, v2, v3) -> ANY;
static final SerializableVarargsFunction<ExprValue, ExprValue>
varrgs = (v1) -> ANY;
@Mock
FunctionProperties mockProperties;
}
Original file line number Diff line number Diff line change
@@ -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<FunctionName, Pair<FunctionSignature, FunctionBuilder>>
getImplementationGenerator() {
return impl(varrgs, ANY_TYPE, ANY_TYPE, true);
}

@Override
List<Expression> getSampleArguments() {
return List.of(DSL.literal(ANY));
}

@Override
String getExpected_toString() {
return "sample(ANY)";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ public class TextFunctionTest extends ExpressionTestBase {
private static List<List<String>> CONCAT_STRING_LISTS = ImmutableList.of(
ImmutableList.of("hello", "world"),
ImmutableList.of("123", "5325"));
private static List<List<String>> 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);
Expand Down Expand Up @@ -228,11 +231,13 @@ 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);
assertEquals(missingValue(), eval(
DSL.concat(missingRef, DSL.literal("1"))));
// If any of the expressions is a NULL value, it returns NULL.
assertEquals(nullValue(), eval(
DSL.concat(nullRef, DSL.literal("1"))));
assertEquals(missingValue(), eval(
Expand Down Expand Up @@ -446,6 +451,22 @@ void testConcatString(List<String> strings, String delim) {
assertEquals(expected, eval(expression).stringValue());
}

void testConcatMultipleString(List<String> 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());
Expand Down
Loading