Support decimal in expression fuzzer test

facebookincubator · Mar 26, 2024 · 3e340cd · 3e340cd
1 parent 4f3d32f
commit 3e340cd
Show file tree

Hide file tree

Showing 15 changed files with 483 additions and 39 deletions.
diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml
@@ -170,6 +170,7 @@ jobs:
                 --seed ${RANDOM} \
                 --enable_variadic_signatures \
                 --velox_fuzzer_enable_complex_types \
+                --velox_fuzzer_enable_decimal_type \
                 --lazy_vector_generation_ratio 0.2 \
                 --velox_fuzzer_enable_column_reuse \
                 --velox_fuzzer_enable_expression_reuse \
@@ -195,6 +196,7 @@ jobs:
                 --enable_variadic_signatures \
                 --lazy_vector_generation_ratio 0.2 \
                 --velox_fuzzer_enable_column_reuse \
+                --velox_fuzzer_enable_decimal_type \
                 --velox_fuzzer_enable_expression_reuse \
                 --max_expression_trees_per_step 2 \
                 --retry_with_try \
@@ -361,6 +363,7 @@ jobs:
           fuzzer_args: " --seed ${RANDOM} --lazy_vector_generation_ratio 0.2 \
           --duration_sec 1800 --enable_variadic_signatures \
           --velox_fuzzer_enable_complex_types \
+          --velox_fuzzer_enable_decimal_type \
           --velox_fuzzer_enable_column_reuse \
           --velox_fuzzer_enable_expression_reuse \
           --max_expression_trees_per_step 2 \
@@ -492,6 +495,7 @@ jobs:
           --assign_function_tickets  $(cat /tmp/presto_bias_functions) \
           --duration_sec 3600 --enable_variadic_signatures \
           --velox_fuzzer_enable_complex_types \
+          --velox_fuzzer_enable_decimal_type \
           --velox_fuzzer_enable_column_reuse \
           --velox_fuzzer_enable_expression_reuse \
           --max_expression_trees_per_step 2 \

diff --git a/.github/workflows/scheduled.yml b/.github/workflows/scheduled.yml
@@ -213,6 +213,7 @@ jobs:
                 --seed ${RANDOM} \
                 --enable_variadic_signatures \
                 --velox_fuzzer_enable_complex_types \
+                --velox_fuzzer_enable_decimal_type \
                 --lazy_vector_generation_ratio 0.2 \
                 --velox_fuzzer_enable_column_reuse \
                 --velox_fuzzer_enable_expression_reuse \
@@ -304,6 +305,7 @@ jobs:
                 --enable_variadic_signatures \
                 --lazy_vector_generation_ratio 0.2 \
                 --velox_fuzzer_enable_column_reuse \
+                --velox_fuzzer_enable_decimal_type \
                 --velox_fuzzer_enable_expression_reuse \
                 --max_expression_trees_per_step 2 \
                 --retry_with_try \

diff --git a/velox/docs/develop/testing/fuzzer.rst b/velox/docs/develop/testing/fuzzer.rst
@@ -160,6 +160,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.
@@ -187,6 +189,7 @@ An example set of arguments to run the expression fuzzer with all features enabl
 --enable_variadic_signatures
 --lazy_vector_generation_ratio 0.2
 --velox_fuzzer_enable_complex_types
+--velox_fuzzer_enable_decimal_type
 --velox_fuzzer_enable_expression_reuse
 --velox_fuzzer_enable_column_reuse
 --retry_with_try

diff --git a/velox/expression/ReverseSignatureBinder.cpp b/velox/expression/ReverseSignatureBinder.cpp
@@ -36,9 +36,6 @@ bool ReverseSignatureBinder::hasConstrainedIntegerVariable(
 }
 
 bool ReverseSignatureBinder::tryBind() {
-  if (hasConstrainedIntegerVariable(signature_.returnType())) {
-    return false;
-  }
   return SignatureBinderBase::tryBind(signature_.returnType(), returnType_);
 }
 

diff --git a/velox/expression/tests/ArgumentTypeFuzzerTest.cpp b/velox/expression/tests/ArgumentTypeFuzzerTest.cpp
@@ -59,6 +59,42 @@ class ArgumentTypeFuzzerTest : public testing::Test {
     }
   }
 
+  void testFuzzingDecimalSuccess(
+      const std::shared_ptr<exec::FunctionSignature>& signature,
+      int32_t expectedArguments,
+      std::optional<TypeKind> outputKind = std::nullopt) {
+    std::mt19937 seed{0};
+    ArgumentTypeFuzzer fuzzer{*signature, seed};
+    ASSERT_TRUE(fuzzer.fuzzArgumentTypes(kMaxVariadicArgs));
+
+    auto& argumentTypes = fuzzer.argumentTypes();
+    ASSERT_GE(argumentTypes.size(), expectedArguments);
+
+    auto& argumentSignatures = signature->argumentTypes();
+    int i;
+    for (i = 0; i < expectedArguments; ++i) {
+      ASSERT_TRUE(argumentTypes[i]->isDecimal())
+          << "at " << i
+          << ": Expected DECIMAL. Got: " << argumentTypes[i]->toString();
+    }
+
+    if (i < argumentTypes.size()) {
+      ASSERT_TRUE(signature->variableArity());
+      ASSERT_LE(
+          argumentTypes.size() - argumentSignatures.size(), kMaxVariadicArgs);
+      for (int j = i; j < argumentTypes.size(); ++j) {
+        ASSERT_TRUE(argumentTypes[j]->equivalent(*argumentTypes[i - 1]));
+      }
+    }
+
+    const auto outputType = fuzzer.fuzzReturnType();
+    if (outputKind.has_value()) {
+      ASSERT_TRUE(outputType->kind() == outputKind);
+    } else {
+      ASSERT_TRUE(outputType->isDecimal());
+    }
+  }
+
   void testFuzzingFailure(
       const std::shared_ptr<exec::FunctionSignature>& signature,
       const TypePtr& returnType) {
@@ -222,9 +258,36 @@ TEST_F(ArgumentTypeFuzzerTest, any) {
   ASSERT_TRUE(argumentTypes[0] != nullptr);
 }
 
-TEST_F(ArgumentTypeFuzzerTest, unsupported) {
-  // Constraints on the return type is not supported.
-  auto signature =
+TEST_F(ArgumentTypeFuzzerTest, decimal) {
+  auto signature = exec::FunctionSignatureBuilder()
+                       .integerVariable("a_scale")
+                       .integerVariable("a_precision")
+                       .returnType("boolean")
+                       .argumentType("decimal(a_precision, a_scale)")
+                       .argumentType("decimal(a_precision, a_scale)")
+                       .argumentType("decimal(a_precision, a_scale)")
+                       .build();
+
+  testFuzzingDecimalSuccess(signature, 3, TypeKind::BOOLEAN);
+
+  signature =
+      exec::FunctionSignatureBuilder()
+          .integerVariable("a_precision")
+          .integerVariable("a_scale")
+          .integerVariable("b_precision")
+          .integerVariable("b_scale")
+          .integerVariable(
+              "r_precision",
+              "min(38, max(a_precision - a_scale, b_precision - b_scale) + max(a_scale, b_scale) + 1)")
+          .integerVariable("r_scale", "max(a_scale, b_scale)")
+          .returnType("DECIMAL(r_precision, r_scale)")
+          .argumentType("DECIMAL(a_precision, a_scale)")
+          .argumentType("DECIMAL(b_precision, b_scale)")
+          .build();
+
+  testFuzzingDecimalSuccess(signature, 2);
+
+  signature =
       exec::FunctionSignatureBuilder()
           .integerVariable("a_scale")
           .integerVariable("b_scale")
@@ -239,7 +302,47 @@ TEST_F(ArgumentTypeFuzzerTest, unsupported) {
           .argumentType("decimal(b_precision, b_scale)")
           .build();
 
-  testFuzzingFailure(signature, DECIMAL(13, 6));
+  testFuzzingDecimalSuccess(signature, 2, TypeKind::ROW);
+
+  signature = exec::FunctionSignatureBuilder()
+                  .integerVariable("i1")
+                  .integerVariable("i2")
+                  .integerVariable("i5")
+                  .integerVariable("i6")
+                  .integerVariable(
+                      "i3", "min(38, max(i1 - i5, i2 - i6) + max(i5, i6) + 1)")
+                  .integerVariable("i7", "max(i5, i6)")
+                  .returnType("decimal(i3,i7)")
+                  .argumentType("decimal(i1,i5)")
+                  .argumentType("decimal(i2,i6)")
+                  .build();
+  testFuzzingDecimalSuccess(signature, 2);
+
+  signature = exec::FunctionSignatureBuilder()
+                  .integerVariable("i1")
+                  .integerVariable("i5")
+                  .returnType("boolean")
+                  .argumentType("decimal(i1,i5)")
+                  .argumentType("decimal(i1,i5)")
+                  .argumentType("decimal(i1,i5)")
+                  .build();
+  testFuzzingDecimalSuccess(signature, 3, TypeKind::BOOLEAN);
+
+  signature = exec::FunctionSignatureBuilder()
+                  .integerVariable("precision")
+                  .integerVariable("scale")
+                  .returnType("DECIMAL(precision, scale)")
+                  .argumentType("DECIMAL(precision, scale)")
+                  .variableArity()
+                  .build();
+  testFuzzingDecimalSuccess(signature, 1);
+
+  signature = exec::FunctionSignatureBuilder()
+                  .integerVariable("precision", "min(max(6, precision), 18)")
+                  .returnType("timestamp")
+                  .argumentType("decimal(precision, 6)")
+                  .build();
+  testFuzzingDecimalSuccess(signature, 1, TypeKind::TIMESTAMP);
 }
 
 TEST_F(ArgumentTypeFuzzerTest, lambda) {