[sidecar] Fix array_sort lambda function failure when sidecar is enabled and add e2e lambda function tests

Author: pdabre12

presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/SemanticErrorCode.java

@@ -56,6 +56,7 @@ public enum SemanticErrorCode
     FUNCTION_NOT_FOUND,
     INVALID_FUNCTION_NAME,
     DUPLICATE_PARAMETER_NAME,
+    EXCEPTIONS_WHEN_RESOLVING_FUNCTIONS,
 
     ORDER_BY_MUST_BE_IN_SELECT,
     ORDER_BY_MUST_BE_IN_AGGREGATE,

presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/SemanticException.java

@@ -26,6 +26,7 @@ public class SemanticException
 {
     private final SemanticErrorCode code;
     private final Optional<NodeLocation> location;
+    private final String formattedMessage;
 
     public SemanticException(SemanticErrorCode code, String format, Object... args)
     {
@@ -44,10 +45,18 @@ public SemanticException(SemanticErrorCode code, Optional<NodeLocation> location
 
     public SemanticException(SemanticErrorCode code, Throwable cause, Optional<NodeLocation> location, String format, Object... args)
     {
-        super(formatMessage(format, location, args), cause);
+        super(cause);
 
         this.code = requireNonNull(code, "code is null");
         this.location = requireNonNull(location, "location is null");
+        requireNonNull(format, "format is null");
+        this.formattedMessage = formatMessage(format, location, args);
+    }
+
+    @Override
+    public String getMessage()
+    {
+        return formattedMessage;
     }
 
     // TODO: Should be replaced with analyzer agnostic location

presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/SignatureMatchingException.java

@@ -0,0 +1,39 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.sql.analyzer;
+
+import java.util.List;
+
+import static com.facebook.presto.sql.analyzer.SemanticErrorCode.EXCEPTIONS_WHEN_RESOLVING_FUNCTIONS;
+import static java.lang.String.format;
+
+public class SignatureMatchingException
+        extends SemanticException
+{
+    public SignatureMatchingException(
+            String prefix,
+            List<SemanticException> failedExceptions)
+    {
+        super(EXCEPTIONS_WHEN_RESOLVING_FUNCTIONS, formatMessage(prefix, failedExceptions));
+    }
+
+    private static String formatMessage(String formatString, List<SemanticException> failedExceptions)
+    {
+        StringBuilder sb = new StringBuilder(formatString).append("\n");
+        for (int i = 0; i < failedExceptions.size(); i++) {
+            sb.append(format(" Exception %d: %s%n", i + 1, failedExceptions.get(i).getMessage()));
+        }
+        return sb.toString();
+    }
+}

presto-main-base/src/main/java/com/facebook/presto/metadata/FunctionSignatureMatcher.java

@@ -19,6 +19,8 @@
 import com.facebook.presto.spi.function.FunctionKind;
 import com.facebook.presto.spi.function.Signature;
 import com.facebook.presto.spi.function.SqlFunction;
+import com.facebook.presto.sql.analyzer.SemanticException;
+import com.facebook.presto.sql.analyzer.SignatureMatchingException;
 import com.facebook.presto.sql.analyzer.TypeSignatureProvider;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -124,15 +126,28 @@ private Optional<Signature> matchFunction(Collection<? extends SqlFunction> cand
     private List<ApplicableFunction> identifyApplicableFunctions(Collection<? extends SqlFunction> candidates, List<TypeSignatureProvider> actualParameters, boolean allowCoercion)
     {
         ImmutableList.Builder<ApplicableFunction> applicableFunctions = ImmutableList.builder();
+        ImmutableList.Builder<SemanticException> semanticExceptions = ImmutableList.builder();
         for (SqlFunction function : candidates) {
             Signature declaredSignature = function.getSignature();
-            Optional<Signature> boundSignature = new SignatureBinder(functionAndTypeManager, declaredSignature, allowCoercion)
-                    .bind(actualParameters);
-            if (boundSignature.isPresent()) {
-                applicableFunctions.add(new ApplicableFunction(declaredSignature, boundSignature.get(), function.isCalledOnNullInput()));
+            try {
+                Optional<Signature> boundSignature = new SignatureBinder(functionAndTypeManager, declaredSignature, allowCoercion)
+                        .bind(actualParameters);
+                boundSignature.ifPresent(signature -> applicableFunctions.add(new ApplicableFunction(declaredSignature, signature, function.isCalledOnNullInput())));
             }
+            catch (SemanticException e) {
+                semanticExceptions.add(e);
+            }
+        }
+
+        List<ApplicableFunction> applicableFunctionsList = applicableFunctions.build();
+        List<SemanticException> semanticExceptionList = semanticExceptions.build();
+        if (applicableFunctionsList.isEmpty() && !semanticExceptionList.isEmpty()) {
+            decideAndThrow(semanticExceptionList,
+                    candidates.stream().findFirst()
+                            .map(function -> function.getSignature().getName().getObjectName())
+                            .orElse(""));
         }
-        return applicableFunctions.build();
+        return applicableFunctionsList;
     }
 
     private List<ApplicableFunction> selectMostSpecificFunctions(List<ApplicableFunction> applicableFunctions, List<TypeSignatureProvider> parameters)
@@ -287,6 +302,22 @@ private static boolean returnsNullOnGivenInputTypes(ApplicableFunction applicabl
         return true;
     }
 
+    /**
+     * Decides which exception to throw based on the number of failed attempts.
+     * If there's only one SemanticException, it throws that SemanticException directly.
+     * If there are multiple SemanticExceptions, it throws the SignatureMatchingException.
+     */
+    private static void decideAndThrow(List<SemanticException> failedExceptions, String functionName)
+            throws SemanticException
+    {
+        if (failedExceptions.size() == 1) {
+            throw failedExceptions.get(0);
+        }
+        else {
+            throw new SignatureMatchingException(format("Failed to find matching function signature for %s, matching failures: ", functionName), failedExceptions);
+        }
+    }
+
     static String constructFunctionNotFoundErrorMessage(QualifiedObjectName functionName, List<TypeSignatureProvider> parameterTypes, Collection<? extends SqlFunction> candidates)
     {
         String name = toConciseFunctionName(functionName);

presto-native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/TestNativeSidecarPlugin.java

@@ -188,10 +188,25 @@ public void testWindowFunctions()
     }
 
     @Test
-    public void testArraySort()
+    public void testLambdaFunctions()
     {
-        assertQueryFails("SELECT array_sort(quantities, (x, y) -> if (x < y, 1, if (x > y, -1, 0))) FROM orders_ex",
-                "line 1:31: Expected a lambda that takes 1 argument\\(s\\) but got 2");
+        // These function signatures are only supported in the native execution engine
+        assertQuerySucceeds("select array_sort(array[row('apples', 23), row('bananas', 12), row('grapes', 44)], x -> x[2])");
+        assertQuerySucceeds("SELECT array_sort(quantities, x -> abs(x)) FROM orders_ex");
+        assertQuerySucceeds("SELECT array_sort(quantities, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) FROM orders_ex");
+
+        assertQuery("SELECT array_sort(map_keys(map_union(quantity_by_linenumber))) FROM orders_ex");
+        assertQuery("SELECT filter(quantities, q -> q > 10) FROM orders_ex");
+        assertQuery("SELECT all_match(shuffle(quantities), x -> (x > 500.0)) FROM orders_ex");
+        assertQuery("SELECT any_match(quantities, x -> TRY(((10 / x) > 2))) FROM orders_ex");
+        assertQuery("SELECT TRY(none_match(quantities, x -> ((10 / x) > 2))) FROM orders_ex");
+        assertQuery("SELECT reduce(array[nationkey, regionkey], 103, (s, x) -> s + x, s -> s) FROM nation");
+        assertQuery("SELECT transform(array[1, 2, 3], x -> x * regionkey + nationkey) FROM nation");
+        assertQueryFails(
+                "SELECT array_sort(quantities, (x, y, z) -> if (x < y + z, cast(1 as bigint), if (x > y + z, cast(-1 as bigint), cast(0 as bigint)))) FROM orders_ex",
+                Pattern.quote("Failed to find matching function signature for array_sort, matching failures: \n" +
+                        " Exception 1: line 1:31: Expected a lambda that takes 1 argument(s) but got 3\n" +
+                        " Exception 2: line 1:31: Expected a lambda that takes 2 argument(s) but got 3\n"));
     }
 
     @Test