Skip to content

Commit

Permalink
Throw VeloxRuntimeError in Reduce() on violating kMaxArraySize. (#11252)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11252

Expression can be wrapped in TRY() and current user error gets caught
and NULL is returned, which causes correctness issue.
Changing the code to throw VeloxRuntimeError to fail the query.

Reviewed By: Yuhta, amitkdutta

Differential Revision: D64352201

fbshipit-source-id: d67edb1066e7e058e72aec8f2038dd917e7a3f2c
  • Loading branch information
Sergey Pershin authored and facebook-github-bot committed Oct 15, 2024
1 parent 6051f03 commit 757a1e4
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 15 deletions.
15 changes: 6 additions & 9 deletions velox/functions/prestosql/Reduce.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,12 @@ void checkArraySizes(
return;
}
const auto size = rawSizes[indices[row]];
try {
VELOX_USER_CHECK_LT(
size,
kMaxArraySize,
"reduce lambda function doesn't support arrays with more than {} elements",
kMaxArraySize);
} catch (VeloxUserError&) {
context.setError(row, std::current_exception());
}
// We do not want this error to be suppressed by TRY(), so we simply throw.
VELOX_CHECK_LT(
size,
kMaxArraySize,
"reduce lambda function doesn't support arrays with more than {} elements",
kMaxArraySize);
});
}

Expand Down
11 changes: 5 additions & 6 deletions velox/functions/prestosql/tests/ReduceTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ TEST_F(ReduceTest, limit) {

VELOX_ASSERT_THROW(
evaluate("reduce(c0, 0, (s, x) -> s + x, s -> 1 * s)", data),
"reduce lambda function doesn't support arrays with more than 10000 elements");
"reduce lambda function doesn't support arrays with more than");

// Exclude huge arrays.
SelectivityVector rows(4);
Expand All @@ -286,11 +286,10 @@ TEST_F(ReduceTest, limit) {
makeFlatVector<int64_t>({123 * 1'000, 123 * 9'000, -1, 123 * 10});
assertEqualVectors(expected, result, rows);

// Mask errors with TRY.
result = evaluate("TRY(reduce(c0, 0, (s, x) -> s + x, s -> 1 * s))", data);
expected = makeNullableFlatVector<int64_t>(
{123 * 1'000, 123 * 9'000, std::nullopt, 123 * 10, std::nullopt});
assertEqualVectors(expected, result);
// TRY should not mask errors.
VELOX_ASSERT_THROW(
evaluate("TRY(reduce(c0, 0, (s, x) -> s + x, s -> 1 * s))", data),
"reduce lambda function doesn't support arrays with more than");
}

TEST_F(ReduceTest, rewrites) {
Expand Down

0 comments on commit 757a1e4

Please sign in to comment.