Skip to content

Commit

Permalink
Support UNSUPPORTED_INPUT_UNCATCHABLE VeloxRuntimeError in expression…
Browse files Browse the repository at this point in the history
… fuzzer (facebookincubator#10993)

Summary:
Pull Request resolved: facebookincubator#10993

We recently made cast-varchar-to-integral throw VeloxRuntimeError on unicode
inputs and made date-time functions throw VeloxRuntimeError when timestamps
are out of supported range. The expression fuzzer tests have been failing due to
these changes because VeloxRuntimeError is not allowed in fuzzer. This diff
introduces a new ErrorCode `UNSUPPORTED_INPUT_UNCATCHABLE` to VeloxRuntimeError that is
ignored in fuzzers, and makes the cast and date-time functions throw errors with
this error code.

Reviewed By: xiaoxmeng, kgpai

Differential Revision: D62612232

fbshipit-source-id: 334d615555a12c47d9b2219dca1eb382edcfabf3
  • Loading branch information
kagamiori authored and facebook-github-bot committed Sep 18, 2024
1 parent 648ae89 commit 8a6ab15
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 28 deletions.
25 changes: 25 additions & 0 deletions velox/common/base/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,20 @@ DECLARE_CHECK_FAIL_TEMPLATES(::facebook::velox::VeloxRuntimeError);
/* isRetriable */ false, \
##__VA_ARGS__)

/// Throws VeloxRuntimeError when functions receive input values out of the
/// supported range. This should only be used when we want to force TRY() to not
/// suppress the error.
#define VELOX_CHECK_UNSUPPORTED_INPUT_UNCATCHABLE(expr, ...) \
if (UNLIKELY(!(expr))) { \
_VELOX_THROW_IMPL( \
::facebook::velox::VeloxRuntimeError, \
#expr, \
::facebook::velox::error_source::kErrorSourceRuntime.c_str(), \
::facebook::velox::error_code::kUnsupportedInputUncatchable.c_str(), \
/* isRetriable */ false, \
__VA_ARGS__); \
}

// If the caller passes a custom message (4 *or more* arguments), we
// have to construct a format string from ours ("({} vs. {})") plus
// theirs by adding a space and shuffling arguments. If they don't (exactly 3
Expand Down Expand Up @@ -330,6 +344,17 @@ DECLARE_CHECK_FAIL_TEMPLATES(::facebook::velox::VeloxRuntimeError);
/* isRetriable */ false, \
##__VA_ARGS__)

/// Throws VeloxRuntimeError when functions receive input values out of the
/// supported range. This should only be used when we want to force TRY() to not
/// suppress the error.
#define VELOX_FAIL_UNSUPPORTED_INPUT_UNCATCHABLE(...) \
_VELOX_THROW( \
::facebook::velox::VeloxRuntimeError, \
::facebook::velox::error_source::kErrorSourceRuntime.c_str(), \
::facebook::velox::error_code::kUnsupportedInputUncatchable.c_str(), \
/* isRetriable */ false, \
##__VA_ARGS__)

DECLARE_CHECK_FAIL_TEMPLATES(::facebook::velox::VeloxUserError);

// For all below macros, an additional message can be passed using a
Expand Down
6 changes: 6 additions & 0 deletions velox/common/base/VeloxException.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ inline constexpr auto kFileNotFound = "FILE_NOT_FOUND"_fs;

// We do not know how to classify it yet.
inline constexpr auto kUnknown = "UNKNOWN"_fs;

// VeloxRuntimeErrors due to unsupported input values such as unicode input to
// cast-varchar-to-integer and timestamps beyond the year 2037 to datetime
// functions. This kind of errors is allowed in expression fuzzer.
inline constexpr auto kUnsupportedInputUncatchable =
"UNSUPPORTED_INPUT_UNCATCHABLE"_fs;
} // namespace error_code

class VeloxException : public std::exception {
Expand Down
4 changes: 3 additions & 1 deletion velox/expression/CastExpr-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,9 @@ void CastExpr::applyCastKernel(
ToKind == TypeKind::INTEGER || ToKind == TypeKind::BIGINT ||
ToKind == TypeKind::HUGEINT) {
if constexpr (TPolicy::throwOnUnicode) {
VELOX_CHECK(
// This is a special case where we intentionally throw
// VeloxRuntimeError to avoid it being suppressed by TRY().
VELOX_CHECK_UNSUPPORTED_INPUT_UNCATCHABLE(
functions::stringCore::isAscii(
inputRowValue.data(), inputRowValue.size()),
"Unicode characters are not supported for conversion to integer types");
Expand Down
33 changes: 19 additions & 14 deletions velox/expression/fuzzer/ExpressionFuzzerVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,17 @@ void ExpressionFuzzerVerifier::retryWithTry(
plan->type(), std::vector<core::TypedExprPtr>{plan}, "try"));
}

RowVectorPtr tryResult;
ResultOrError tryResult;

// The function throws if anything goes wrong.
// The function throws if anything goes wrong except
// UNSUPPORTED_INPUT_UNCATCHABLE errors.
try {
tryResult =
verifier_
.verify(
tryPlans,
rowVector,
resultVector ? BaseVector::copy(*resultVector) : nullptr,
false, // canThrow
columnsToWrapInLazy)
.result;
tryResult = verifier_.verify(
tryPlans,
rowVector,
resultVector ? BaseVector::copy(*resultVector) : nullptr,
false, // canThrow
columnsToWrapInLazy);
} catch (const std::exception&) {
if (options_.findMinimalSubexpression) {
test::computeMinimumSubExpression(
Expand All @@ -264,10 +262,15 @@ void ExpressionFuzzerVerifier::retryWithTry(
}
throw;
}
if (tryResult.unsupportedInputUncatchableError) {
LOG(INFO)
<< "Retry with try fails to find minimal subexpression due to UNSUPPORTED_INPUT_UNCATCHABLE error.";
return;
}

// Re-evaluate the original expression on rows that didn't produce an
// error (i.e. returned non-NULL results when evaluated with TRY).
BufferPtr noErrorIndices = extractNonNullIndices(tryResult);
BufferPtr noErrorIndices = extractNonNullIndices(tryResult.result);

if (noErrorIndices != nullptr) {
auto noErrorRowVector = wrapChildren(noErrorIndices, rowVector);
Expand Down Expand Up @@ -360,8 +363,10 @@ void ExpressionFuzzerVerifier::go() {

// If both paths threw compatible exceptions, we add a try() function to
// the expression's root and execute it again. This time the expression
// cannot throw.
if (result.exceptionPtr && options_.retryWithTry) {
// cannot throw. Expressions that throw UNSUPPORTED_INPUT_UNCATCHABLE errors
// are not supported.
if (result.exceptionPtr && options_.retryWithTry &&
!result.unsupportedInputUncatchableError) {
LOG(INFO)
<< "Both paths failed with compatible exceptions. Retrying expression using try().";
retryWithTry(plans, rowVector, resultVectors, columnsToWrapInLazy);
Expand Down
3 changes: 3 additions & 0 deletions velox/expression/fuzzer/FuzzerToolkit.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ struct SignatureTemplate {
struct ResultOrError {
RowVectorPtr result;
std::exception_ptr exceptionPtr;
/// Whether the exception is UNSUPPORTED_INPUT_UNCATCHABLE error. This flag
/// should only be set to true when exceptionPtr is not a nullptr.
bool unsupportedInputUncatchableError{false};
};

/// Sort callable function signatures.
Expand Down
35 changes: 26 additions & 9 deletions velox/expression/tests/ExpressionVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ fuzzer::ResultOrError ExpressionVerifier::verify(

// Execute with common expression eval path. Some columns of the input row
// vector will be wrapped in lazy as specified in 'columnsToWrapInLazy'.
bool unsupportedInputUncatchableError = false;
try {
exec::ExprSet exprSetCommon(
plans, execCtx_, !options_.disableConstantFolding);
Expand All @@ -133,18 +134,25 @@ fuzzer::ResultOrError ExpressionVerifier::verify(
"Copy of original input",
"Input after common");
}
} catch (const VeloxUserError&) {
if (!canThrow) {
LOG(ERROR)
<< "Common eval wasn't supposed to throw, but it did. Aborting.";
} catch (const VeloxException& e) {
if (e.errorCode() == error_code::kUnsupportedInputUncatchable) {
unsupportedInputUncatchableError = true;
} else if (!(canThrow && e.isUserError())) {
if (!canThrow) {
LOG(ERROR)
<< "Common eval wasn't supposed to throw, but it did. Aborting.";
} else if (!e.isUserError()) {
LOG(ERROR)
<< "Common eval: VeloxRuntimeErrors other than UNSUPPORTED_INPUT_UNCATCHABLE error are not allowed.";
}
persistReproInfoIfNeeded(
rowVector, columnsToWrapInLazy, copiedResult, sql, complexConstants);
throw;
}
exceptionCommonPtr = std::current_exception();
} catch (...) {
LOG(ERROR)
<< "Common eval: Exceptions other than VeloxUserError are not allowed.";
<< "Common eval: Exceptions other than VeloxUserError or VeloxRuntimeError of UNSUPPORTED_INPUT_UNCATCHABLE are not allowed.";
persistReproInfoIfNeeded(
rowVector, columnsToWrapInLazy, copiedResult, sql, complexConstants);
throw;
Expand All @@ -168,11 +176,19 @@ fuzzer::ResultOrError ExpressionVerifier::verify(
"Copy of original input",
"Input after simplified");

} catch (const VeloxUserError&) {
} catch (const VeloxException& e) {
if (!e.isUserError() &&
e.errorCode() != error_code::kUnsupportedInputUncatchable) {
LOG(ERROR)
<< "Simplified eval: VeloxRuntimeErrors other than UNSUPPORTED_INPUT_UNCATCHABLE error are not allowed.";
persistReproInfoIfNeeded(
rowVector, columnsToWrapInLazy, copiedResult, sql, complexConstants);
throw;
}
exceptionSimplifiedPtr = std::current_exception();
} catch (...) {
LOG(ERROR)
<< "Simplified eval: Exceptions other than VeloxUserError are not allowed.";
<< "Simplified eval: Exceptions other than VeloxUserError or VeloxRuntimeError with UNSUPPORTED_INPUT are not allowed.";
persistReproInfoIfNeeded(
rowVector, columnsToWrapInLazy, copiedResult, sql, complexConstants);
throw;
Expand All @@ -184,7 +200,7 @@ fuzzer::ResultOrError ExpressionVerifier::verify(
// 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};
return {nullptr, exceptionCommonPtr, unsupportedInputUncatchableError};
} else {
// Throws in case output is different.
VELOX_CHECK_EQ(commonEvalResult.size(), plans.size());
Expand Down Expand Up @@ -217,7 +233,8 @@ fuzzer::ResultOrError ExpressionVerifier::verify(

return {
VectorMaker(commonEvalResult[0]->pool()).rowVector(commonEvalResult),
nullptr};
nullptr,
unsupportedInputUncatchableError};
}

void ExpressionVerifier::persistReproInfoIfNeeded(
Expand Down
6 changes: 3 additions & 3 deletions velox/type/Timestamp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ void Timestamp::toTimezone(const tz::TimeZone& zone) {
seconds_ = zone.to_local(std::chrono::seconds(seconds_)).count();
} catch (const std::invalid_argument& e) {
// Invalid argument means we hit a conversion not supported by
// external/date. Need to throw a RuntimeError so that try() statements do
// not suppress it.
VELOX_FAIL(e.what());
// external/date. This is a special case where we intentionally throw
// VeloxRuntimeError to avoid it being suppressed by TRY().
VELOX_FAIL_UNSUPPORTED_INPUT_UNCATCHABLE(e.what());
}
}

Expand Down
4 changes: 3 additions & 1 deletion velox/type/tz/TimeZoneMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ void validateRangeImpl(time_point<TDuration> timePoint) {
auto year = year_month_day(floor<days>(timePoint)).year();

if (year < kMinYear || year > kMaxYear) {
VELOX_FAIL(
// This is a special case where we intentionally throw
// VeloxRuntimeError to avoid it being suppressed by TRY().
VELOX_FAIL_UNSUPPORTED_INPUT_UNCATCHABLE(
"Timepoint is outside of supported year range: [{}, {}], got {}",
(int)kMinYear,
(int)kMaxYear,
Expand Down

0 comments on commit 8a6ab15

Please sign in to comment.