Back out "Make concat function throw when there are less than 2 argum…

…ents" (facebookincubator#10720) Summary: Pull Request resolved: facebookincubator#10720 This diff D60536630 seems to be breaking all of xstream engine's AB tests, e.g. https://www.internalfb.com/conveyor/xstream/query_runtime/releases Reviewed By: bikramSingh91 Differential Revision: D61150783 fbshipit-source-id: 83338c9cd848e85493b5692bafc232fdf78d36b5
DanielHunte · Aug 13, 2024 · 99f990c · 99f990c
1 parent 7474bae
commit 99f990c
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 15 deletions.
diff --git a/velox/expression/FunctionSignature.h b/velox/expression/FunctionSignature.h
@@ -309,10 +309,6 @@ 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;

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

diff --git a/velox/functions/prestosql/StringFunctions.cpp b/velox/functions/prestosql/StringFunctions.cpp
@@ -258,15 +258,13 @@ class ConcatFunction : public exec::VectorFunction {
             .returnType("varchar")
             .argumentType("varchar")
             .argumentType("varchar")
-            .argumentType("varchar")
             .variableArity()
             .build(),
         // varbinary, varbinary,.. -> varbinary
         exec::FunctionSignatureBuilder()
             .returnType("varbinary")
             .argumentType("varbinary")
             .argumentType("varbinary")
-            .argumentType("varbinary")
             .variableArity()
             .build(),
     };

diff --git a/velox/functions/prestosql/tests/StringFunctionsTest.cpp b/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 = 2; argsCount <= maxArgsCount; argsCount++) {
+  for (int argsCount = 1; argsCount <= maxArgsCount; argsCount++) {
     inputTable.clear();
 
     // Create table with argsCount columns
@@ -930,13 +930,6 @@ 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) {