Skip to content

Commit

Permalink
fix: IN does not handle User errors thrown during eval (#11823)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11823

IN does not handle user errors thrown during eval. This means that if the IN is wrapped with a TRY, user errors
will still get thrown rather than producing a null.

The fuzzer tests demonstrated a case of this, Velox allows creating Timestamps that are large enough that they
cannot be represented as milliseconds, there would be an overflow, in this case toMillis throws a user exception.
When evaluating IN, the argument is converted to milliseconds and compared to the set of Timestamps. When
wrapped in a TRY the IN still throws an exception rather than returning NULL.

Reviewed By: bikramSingh91

Differential Revision: D67063377

fbshipit-source-id: 76638810dd11f7532cf19b754edd2a0f01c18f67
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Dec 11, 2024
1 parent 04deeb3 commit 4830651
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 9 deletions.
21 changes: 12 additions & 9 deletions velox/functions/prestosql/InPredicate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class VectorSetInPredicate : public exec::VectorFunction {
result->clearNulls(rows);
auto* boolResult = result->asUnchecked<FlatVector<bool>>();

rows.applyToSelected([&](vector_size_t row) {
context.applyToSelectedNoThrow(rows, [&](vector_size_t row) {
if (arg->isNullAt(row)) {
boolResult->setNull(row, true);
} else {
Expand Down Expand Up @@ -543,12 +543,15 @@ class InPredicate : public exec::VectorFunction {
if (simpleArg->isNullAt(rows.begin())) {
localResult = createBoolConstantNull(rows.end(), context);
} else {
bool pass = testFunction(simpleArg->valueAt(rows.begin()));
if (!pass && passOrNull) {
localResult = createBoolConstantNull(rows.end(), context);
} else {
localResult = createBoolConstant(pass, rows.end(), context);
}
static const SelectivityVector oneRow(1, true);
context.applyToSelectedNoThrow(oneRow, [&](vector_size_t row) {
bool pass = testFunction(simpleArg->valueAt(rows.begin()));
if (!pass && passOrNull) {
localResult = createBoolConstantNull(rows.end(), context);
} else {
localResult = createBoolConstant(pass, rows.end(), context);
}
});
}

context.moveOrCopyResult(localResult, rows, result);
Expand All @@ -565,7 +568,7 @@ class InPredicate : public exec::VectorFunction {
auto* rawResults = boolResult->mutableRawValues<uint64_t>();

if (flatArg->mayHaveNulls() || passOrNull) {
rows.applyToSelected([&](auto row) {
context.applyToSelectedNoThrow(rows, [&](vector_size_t row) {
if (flatArg->isNullAt(row)) {
boolResult->setNull(row, true);
} else {
Expand All @@ -578,7 +581,7 @@ class InPredicate : public exec::VectorFunction {
}
});
} else {
rows.applyToSelected([&](auto row) {
context.applyToSelectedNoThrow(rows, [&](vector_size_t row) {
bool pass = testFunction(flatArg->valueAtFast(row));
bits::setBit(rawResults, row, pass);
});
Expand Down
33 changes: 33 additions & 0 deletions velox/functions/prestosql/tests/InPredicateTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1139,5 +1139,38 @@ TEST_F(InPredicateTest, TimestampWithTimeZone) {
testConstantValues<int64_t>(TIMESTAMP_WITH_TIME_ZONE(), valueAt);
}

TEST_F(InPredicateTest, BadTimestampsWithTry) {
// Test that errors thrown while evaluating the filter are propagated
// correctly to the TRY.
// We do this using the maximum possible timestamp value, when evaulating the
// filter it will converted to milliseconds which will cause an overflow that
// is detected and handled as a user error. This should not throw an exception
// but rather be caught by the TRY and return null.
Timestamp maxTimestamp(
std::numeric_limits<int64_t>::max() / 1000, 999'999'999);
auto flatVectorWithoutNulls = makeFlatVector<Timestamp>({maxTimestamp});
auto flatVectorWithNulls =
makeNullableFlatVector<Timestamp>({{maxTimestamp}});
auto constant = makeConstant<Timestamp>(maxTimestamp, 1);

auto inValues = makeTimestampVector({0, 1, 2});

auto expected = BaseVector::createNullConstant(BOOLEAN(), 1, pool());

auto expression = std::make_shared<core::CallTypedExpr>(
BOOLEAN(),
std::vector<core::TypedExprPtr>{makeInExpression(inValues)},
"try");

auto result = evaluate(expression, makeRowVector({flatVectorWithoutNulls}));
assertEqualVectors(expected, result);

result = evaluate(expression, makeRowVector({flatVectorWithNulls}));
assertEqualVectors(expected, result);

result = evaluate(expression, makeRowVector({constant}));
assertEqualVectors(expected, result);
}

} // namespace
} // namespace facebook::velox::functions

0 comments on commit 4830651

Please sign in to comment.