Support decimal in fuzzer test

velox/docs/develop/testing/fuzzer.rst

@@ -219,6 +219,8 @@ Below are arguments that toggle certain fuzzer features in Expression Fuzzer:
 
 * ``--velox_fuzzer_enable_complex_types``: Enable testing of function signatures with complex argument or return types. Default is false.
 
+* ``--velox_fuzzer_enable_decimal_type``: Enable testing of function signatures with decimal argument or return type. Default is false.
+
 * ``--lazy_vector_generation_ratio``: Specifies the probability with which columns in the input row vector will be selected to be wrapped in lazy encoding (expressed as double from 0 to 1). Default is 0.0.
 
 * ``--velox_fuzzer_enable_column_reuse``: Enable generation of expressions where one input column can be used by multiple subexpressions. Default is false.

velox/expression/ReverseSignatureBinder.cpp

@@ -18,27 +18,7 @@
 
 namespace facebook::velox::exec {
 
-bool ReverseSignatureBinder::hasConstrainedIntegerVariable(
-    const TypeSignature& type) const {
-  if (type.parameters().empty()) {
-    auto it = variables().find(type.baseName());
-    return it != variables().end() && it->second.isIntegerParameter() &&
-        it->second.constraint() != "";
-  }
-
-  const auto& parameters = type.parameters();
-  for (const auto& parameter : parameters) {
-    if (hasConstrainedIntegerVariable(parameter)) {
-      return true;
-    }
-  }
-  return false;
-}
-
 bool ReverseSignatureBinder::tryBind() {
-  if (hasConstrainedIntegerVariable(signature_.returnType())) {
-    return false;
-  }
   tryBindSucceeded_ =
       SignatureBinderBase::tryBind(signature_.returnType(), returnType_);
   return tryBindSucceeded_;

velox/expression/ReverseSignatureBinder.h

@@ -54,10 +54,6 @@ class ReverseSignatureBinder : private SignatureBinderBase {
   }
 
  private:
-  // Return whether there is a constraint on an integer variable in type
-  // signature.
-  bool hasConstrainedIntegerVariable(const TypeSignature& type) const;
-
   const TypePtr returnType_;
 
   // True if 'tryBind' has been called and succeeded. False otherwise.

velox/expression/tests/ExpressionFuzzer.cpp

@@ -39,13 +39,6 @@ int32_t rand(FuzzerGenerator& rng) {
 
 class DecimalArgGeneratorBase : public ArgGenerator {
  public:
-  TypePtr generateReturnType(
-      const exec::FunctionSignature& /*signature*/,
-      FuzzerGenerator& /*rng*/) override {
-    auto [p, s] = inputs_.begin()->first;
-    return DECIMAL(p, s);
-  }
-
   std::vector<TypePtr> generateArgs(
       const exec::FunctionSignature& /*signature*/,
       const TypePtr& returnType,
@@ -80,7 +73,7 @@ class DecimalArgGeneratorBase : public ArgGenerator {
       }
     }
   }
-
+  
   struct Inputs {
     TypePtr a;
     TypePtr b;
@@ -163,19 +156,6 @@ class DivideArgGenerator : public DecimalArgGeneratorBase {
 
 class FloorAndRoundArgGenerator : public ArgGenerator {
  public:
-  TypePtr generateReturnType(
-      const exec::FunctionSignature& signature,
-      FuzzerGenerator& rng) override {
-    if (signature.argumentTypes().size() == 1) {
-      auto p = 1 + rand(rng) % 38;
-      return DECIMAL(p, 0);
-    } else {
-      auto p = 2 + rand(rng) % 37;
-      auto s = rand(rng) % p;
-      return DECIMAL(p, s);
-    }
-  }
-
   std::vector<TypePtr> generateArgs(
       const exec::FunctionSignature& signature,
       const TypePtr& returnType,
@@ -624,16 +604,20 @@ bool useTypeName(
 
 bool isSupportedSignature(
     const exec::FunctionSignature& signature,
-    bool enableComplexType) {
-  // Not supporting lambda functions, or functions using decimal and
-  // timestamp with time zone types.
+    bool enableComplexType,
+    bool enableDecimalType) {
+  // When enableComplexType is disabled, not supporting complex functions.
+  const bool useComplexType = useTypeName(signature, "array") ||
+      useTypeName(signature, "map") || useTypeName(signature, "row");
+  // When enableDecimalType is disabled, not supporting decimal functions. Not
+  // supporting lambda functions, or functions using timestamp with time zone
+  // types.
   return !(
       useTypeName(signature, "opaque") ||
-      // useTypeName(signature, "long_decimal") ||
-      // useTypeName(signature, "short_decimal") ||
-      // useTypeName(signature, "decimal") ||
       useTypeName(signature, "timestamp with time zone") ||
       useTypeName(signature, "interval day to second") ||
+      (!enableDecimalType && useTypeName(signature, "decimal")) ||
+      (!enableComplexType && useComplexType) ||
       (enableComplexType && useTypeName(signature, "unknown")));
 }
 
@@ -751,10 +735,14 @@ ExpressionFuzzer::ExpressionFuzzer(
     for (const auto& signature : function.second) {
       ++totalFunctionSignatures;
 
-      if (!isSupportedSignature(*signature, options_.enableComplexTypes)) {
+      if (!isSupportedSignature(
+              *signature,
+              options_.enableComplexTypes,
+              options_.enableDecimalType)) {
         continue;
       }
-      if (!(signature->variables().empty() || options_.enableComplexTypes)) {
+      if (!(signature->variables().empty() || options_.enableComplexTypes ||
+            options_.enableDecimalType)) {
         LOG(WARNING) << "Skipping unsupported signature: " << function.first
                      << signature->toString();
         continue;
@@ -793,29 +781,15 @@ ExpressionFuzzer::ExpressionFuzzer(
         ArgumentTypeFuzzer typeFuzzer{*signature, localRng};
         auto returnType = typeFuzzer.fuzzReturnType();
         bool ok = typeFuzzer.fuzzArgumentTypes(options_.maxNumVarArgs);
-        if (!ok) {
-          auto it = argGenerators_.find(function.first);
-          if (it != argGenerators_.end()) {
-            returnType = it->second->generateReturnType(*signature, localRng);
-            argTypes =
-                it->second->generateArgs(*signature, returnType, localRng);
-            VELOX_CHECK(
-                !argTypes.empty(),
-                "Failed to generate arguments for {} with return type {}",
-                function.first,
-                returnType->toString());
-          } else {
-            VELOX_FAIL("Failed to generate arguments");
-          }
-        } else {
+        if (ok) {
           argTypes = typeFuzzer.argumentTypes();
+          if (!isDeterministic(function.first, argTypes)) {
+            LOG(WARNING) << "Skipping non-deterministic function: "
+                        << function.first << signature->toString();
+            continue;
+          }
         }
       }
-      if (!isDeterministic(function.first, argTypes)) {
-        LOG(WARNING) << "Skipping non-deterministic function: "
-                     << function.first << signature->toString();
-        continue;
-      }
 
       if (!signature->variables().empty()) {
         std::unordered_set<std::string> typeVariables;
@@ -894,8 +868,10 @@ ExpressionFuzzer::ExpressionFuzzer(
   sortSignatureTemplates(signatureTemplates_);
 
   for (const auto& it : signatureTemplates_) {
-    auto& returnType = it.signature->returnType().baseName();
-    auto* returnTypeKey = &returnType;
+    const auto returnType =
+        exec::sanitizeName(it.signature->returnType().baseName());
+    const auto* returnTypeKey = &returnType;
+
     if (it.typeVariables.find(returnType) != it.typeVariables.end()) {
       // Return type is a template variable.
       returnTypeKey = &kTypeParameterName;
@@ -969,7 +945,7 @@ void ExpressionFuzzer::addToTypeToExpressionListByTicketTimes(
     const std::string& funcName) {
   int tickets = getTickets(funcName);
   for (int i = 0; i < tickets; i++) {
-    typeToExpressionList_[type].push_back(funcName);
+    typeToExpressionList_[exec::sanitizeName(type)].push_back(funcName);
   }
 }
 
@@ -1243,7 +1219,8 @@ core::TypedExprPtr ExpressionFuzzer::generateExpression(
     } else {
       expression = generateExpressionFromConcreteSignatures(
           returnType, chosenFunctionName);
-      if (!expression && options_.enableComplexTypes) {
+      if (!expression &&
+          (options_.enableComplexTypes || options_.enableDecimalType)) {
         expression = generateExpressionFromSignatureTemplate(
             returnType, chosenFunctionName);
       }

velox/expression/tests/ExpressionFuzzer.h

@@ -38,12 +38,6 @@ class ArgGenerator {
       const exec::FunctionSignature& signature,
       const TypePtr& returnType,
       FuzzerGenerator& rng) = 0;
-
-  /// Returns a randomly generated valid return type for a given signature.
-  /// TODO Remove. This API should not be needed.
-  virtual TypePtr generateReturnType(
-      const exec::FunctionSignature& signature,
-      FuzzerGenerator& rng) = 0;
 };
 
 // A tool that can be used to generate random expressions.
@@ -67,6 +61,10 @@ class ExpressionFuzzer {
     // types.
     bool enableComplexTypes = false;
 
+    // Enable testing of function signatures with decimal argument or return
+    // types.
+    bool enableDecimalType = false;
+
     // Enable generation of expressions where one input column can be used by
     // multiple subexpressions.
     bool enableColumnReuse = false;

velox/expression/tests/FuzzerRunner.cpp

@@ -121,6 +121,11 @@ DEFINE_bool(
     false,
     "Enable testing of function signatures with complex argument or return types.");
 
+DEFINE_bool(
+    velox_fuzzer_enable_decimal_type,
+    false,
+    "Enable testing of function signatures with decimal argument or return types.");
+
 DEFINE_bool(
     velox_fuzzer_enable_column_reuse,
     false,
@@ -168,6 +173,7 @@ ExpressionFuzzer::Options getExpressionFuzzerOptions(
   opts.enableVariadicSignatures = FLAGS_enable_variadic_signatures;
   opts.enableDereference = FLAGS_enable_dereference;
   opts.enableComplexTypes = FLAGS_velox_fuzzer_enable_complex_types;
+  opts.enableDecimalType = FLAGS_velox_fuzzer_enable_decimal_type;
   opts.enableColumnReuse = FLAGS_velox_fuzzer_enable_column_reuse;
   opts.enableExpressionReuse = FLAGS_velox_fuzzer_enable_expression_reuse;
   opts.functionTickets = FLAGS_assign_function_tickets;

velox/expression/tests/utils/ArgumentTypeFuzzer.cpp

@@ -157,15 +157,20 @@ bool ArgumentTypeFuzzer::fuzzArgumentTypes(uint32_t maxVariadicArgs) {
   const auto& formalArgs = signature_.argumentTypes();
   auto formalArgsCnt = formalArgs.size();
 
-  std::unordered_map<std::string, int> integerBindings;
-
   if (returnType_) {
     exec::ReverseSignatureBinder binder{signature_, returnType_};
     if (!binder.tryBind()) {
       return false;
     }
     bindings_ = binder.bindings();
     integerBindings_ = binder.integerBindings();
+    for (const auto& [name, _] : integerBindings_) {
+      // If the variable used by result type is constrained, argument types can
+      // not be generated without following the constraints.
+      if (variables().count(name) && variables().at(name).constraint() != "") {
+        return false;
+      }
+    }
   } else {
     for (const auto& [name, _] : signature_.variables()) {
       bindings_.insert({name, nullptr});

velox/vector/fuzzer/VectorFuzzer.cpp

@@ -1040,32 +1040,8 @@ const std::vector<TypePtr> defaultScalarTypes() {
   };
   return kScalarTypes;
 }
-
-std::pair<int8_t, int8_t> randPrecisionScale(
-    FuzzerGenerator& rng,
-    int8_t maxPrecision) {
-  // Generate precision in range [1, Decimal type max precision]
-  auto precision = 1 + rand<int8_t>(rng) % maxPrecision;
-  // Generate scale in range [0, precision]
-  auto scale = rand<int8_t>(rng) % (precision + 1);
-  return {precision, scale};
-}
-
 } // namespace
 
-TypePtr randDecimalType(FuzzerGenerator& rng) {
-  // Short or Long?
-  if (rand<bool>(rng)) {
-    auto [precision, scale] =
-        randPrecisionScale(rng, ShortDecimalType::kMaxPrecision);
-    return DECIMAL(precision, scale);
-  } else {
-    auto [precision, scale] =
-        randPrecisionScale(rng, LongDecimalType::kMaxPrecision);
-    return DECIMAL(precision, scale);
-  }
-}
-
 TypePtr randType(FuzzerGenerator& rng, int maxDepth) {
   return randType(rng, defaultScalarTypes(), maxDepth);
 }

velox/vector/fuzzer/VectorFuzzer.h

@@ -379,6 +379,4 @@ RowTypePtr randRowType(
     const std::vector<TypePtr>& scalarTypes,
     int maxDepth = 5);
 
-TypePtr randDecimalType(FuzzerGenerator& rng);
-
 } // namespace facebook::velox