Make concat function throw when there are less than 2 arguments (face…

…bookincubator#11076)

Summary:
Pull Request resolved: facebookincubator#11076

Currently, the velox concat function handles having a single argument by returning the argument as the result. Presto throws an exception, so velox should as well.

This change was made before and had to be backed out due to failing XStream pipelines. XStream A/B test in ga and f3 have been verified to succeed this time.

Reviewed By: kevinwilfong

Differential Revision: D63271129

fbshipit-source-id: 60afd7821aa8b49707a9d6c8f0b5421a006a60b6

velox/expression/FunctionSignature.h

@@ -309,6 +309,9 @@ class FunctionSignatureBuilder {
     return *this;
   }
 
+  /// Variable arity arguments can appear only at the end of the argument list
+  /// and their types must match the type specified in the last entry of
+  /// 'argumentTypes'. Variable arity arguments can appear zero or more times.
   FunctionSignatureBuilder& variableArity() {
     variableArity_ = true;
     return *this;

velox/expression/tests/ExprCompilerTest.cpp

@@ -268,9 +268,7 @@ TEST_F(ExprCompilerTest, functionSignatureNotRegistered) {
 
   VELOX_ASSERT_THROW(
       compile(expression),
-      "Scalar function concat not registered with arguments: (BIGINT, BIGINT). "
-      "Found function registered with the following signatures:\n"
-      "((varchar,varchar...) -> varchar)");
+      "Scalar function concat not registered with arguments: (BIGINT, BIGINT)");
 }
 
 TEST_F(ExprCompilerTest, constantFromFlatVector) {

velox/functions/prestosql/StringFunctions.cpp

@@ -257,12 +257,14 @@ class ConcatFunction : public exec::VectorFunction {
         exec::FunctionSignatureBuilder()
             .returnType("varchar")
             .argumentType("varchar")
+            .argumentType("varchar")
             .variableArity("varchar")
             .build(),
         // varbinary, varbinary,.. -> varbinary
         exec::FunctionSignatureBuilder()
             .returnType("varbinary")
             .argumentType("varbinary")
+            .argumentType("varbinary")
             .variableArity("varbinary")
             .build(),
     };

velox/functions/prestosql/tests/StringFunctionsTest.cpp

@@ -844,7 +844,7 @@ TEST_F(StringFunctionsTest, concat) {
   size_t maxStringLength = 100;
 
   std::vector<std::vector<std::string>> inputTable;
-  for (int argsCount = 1; argsCount <= maxArgsCount; argsCount++) {
+  for (int argsCount = 2; argsCount <= maxArgsCount; argsCount++) {
     inputTable.clear();
 
     // Create table with argsCount columns
@@ -930,6 +930,13 @@ TEST_F(StringFunctionsTest, concat) {
 
     test::assertEqualVectors(expected, result);
   }
+
+  // Less than 2 concatenation arguments throws exception.
+  {
+    VELOX_ASSERT_THROW(
+        evaluateOnce<std::string>("concat('a')", {}),
+        "Scalar function signature is not supported: concat(VARCHAR).");
+  }
 }
 
 TEST_F(StringFunctionsTest, concatVarbinary) {