Fix ExpressionVerifier to ignore UNSUPPORTED_INPUT_UNCATCHABLE only t…

…hrown from one eval path (#11096)

Summary:
Pull Request resolved: #11096

Velox throws VeloxRuntimeError at a few places intentionally to fail the query
instead of generating wrong results, e.g., when timestamp is outside the supported
range. These VeloxRuntimeErrors caused failures in expression fuzzer, hence an
attempt was made to tolerate such runtime errors in fuzzer so that fuzzer can still
test the relevant functions (#10993).
However, it turns out that that attempt was not enough. The reason is that
VeloxRuntimeError doesn't work well with the existing mechanism of masking error
by default nulls. So the expression fuzzer still occasionally fails because of only one
of the common and simple evaluation path throwing the error. (If we later want to
make those VeloxRuntimeError not suppressable by TRY but suppressable by the
mask-error-by-default-null mechanism, we'll need to  extend the EvalErrors struct
with an additional buffer to track whether the error at an index can be suppressed
by Try and check that in TryExpr.)

This diff relaxes the fuzzer check to make fuzzer not fail when only one of the
common and simple eval path throw the UNSUPPORTED_INPUT_UNCATCHABLE
runtime error. This should unblock #10713.

Reviewed By: kevinwilfong

Differential Revision: D63421914

fbshipit-source-id: 54d01f7a5e7ea887018659d0eec8070fd0922b50

velox/expression/tests/ExpressionVerifier.cpp

@@ -177,8 +177,9 @@ fuzzer::ResultOrError ExpressionVerifier::verify(
         "Input after simplified");
 
   } catch (const VeloxException& e) {
-    if (!e.isUserError() &&
-        e.errorCode() != error_code::kUnsupportedInputUncatchable) {
+    if (e.errorCode() == error_code::kUnsupportedInputUncatchable) {
+      unsupportedInputUncatchableError = true;
+    } else if (!e.isUserError()) {
       LOG(ERROR)
           << "Simplified eval: VeloxRuntimeErrors other than UNSUPPORTED_INPUT_UNCATCHABLE error are not allowed.";
       persistReproInfoIfNeeded(
@@ -197,10 +198,19 @@ fuzzer::ResultOrError ExpressionVerifier::verify(
   try {
     // Compare results or exceptions (if any). Fail if anything is different.
     if (exceptionCommonPtr || exceptionSimplifiedPtr) {
-      // Throws in case exceptions are not compatible. If they are compatible,
-      // return false to signal that the expression failed.
-      fuzzer::compareExceptions(exceptionCommonPtr, exceptionSimplifiedPtr);
-      return {nullptr, exceptionCommonPtr, unsupportedInputUncatchableError};
+      // UNSUPPORTED_INPUT_UNCATCHABLE errors are VeloxRuntimeErrors that cannot
+      // be suppressed by default NULLs. So it may happen that only one of the
+      // common and simplified path throws this error. In this case, we do not
+      // compare the exceptions.
+      if (!unsupportedInputUncatchableError) {
+        // Throws in case exceptions are not compatible. If they are compatible,
+        // return false to signal that the expression failed.
+        fuzzer::compareExceptions(exceptionCommonPtr, exceptionSimplifiedPtr);
+      }
+      return {
+          nullptr,
+          exceptionCommonPtr ? exceptionCommonPtr : exceptionSimplifiedPtr,
+          unsupportedInputUncatchableError};
     } else {
       // Throws in case output is different.
       VELOX_CHECK_EQ(commonEvalResult.size(), plans.size());